Use pullBase to compare benchmarks against correct branch

punchagan · Apr 18, 2023 · e78519b · e78519b
1 parent 686a0bf
commit e78519b
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 47 deletions.
diff --git a/frontend/src/App.res b/frontend/src/App.res
@@ -16,14 +16,14 @@ let makeGetBenchmarksVariables = (
   ~repoId,
   ~branch=?,
   ~pullNumber=?,
+  ~pullBase=?,
   ~worker,
   ~benchmarkName=?,
   ~startDate,
   ~endDate,
 ): GetBenchmarks.t_variables => {
   open BenchmarkDataHelpers
   let isMaster = Belt.Option.isNone(pullNumber)
-  let defaultBranch = Sidebar.defaultBranch
   let (worker, dockerImage) = switch worker {
   | None => (None, None)
   | Some((worker, dockerImage)) => (Some(worker), Some(dockerImage))
@@ -32,8 +32,8 @@ let makeGetBenchmarksVariables = (
   {
     repoId,
     branch,
-    defaultBranch,
     pullNumber,
+    pullBase,
     isMaster,
     worker,
     dockerImage,
@@ -193,7 +193,7 @@ module Benchmark = {
 
 module BenchmarkView = {
   @react.component
-  let make = (~repoId, ~branch=?, ~pullNumber=?, ~worker, ~benchmarkName=?, ~startDate, ~endDate) => {
+  let make = (~repoId, ~branch=?, ~pullNumber=?, ~pullBase=?, ~worker, ~benchmarkName=?, ~startDate, ~endDate) => {
     let ({ReScriptUrql.Hooks.response: response}, _) = {
       let startDate = Js.Date.toISOString(startDate)->Js.Json.string
       let endDate = Js.Date.toISOString(endDate)->Js.Json.string
@@ -203,6 +203,7 @@ module BenchmarkView = {
           ~repoId,
           ~branch?,
           ~pullNumber?,
+          ~pullBase?,
           ~worker,
           ~benchmarkName?,
           ~startDate,
@@ -284,7 +285,7 @@ module ErrorView = {
 
 module RepoView = {
   @react.component
-  let make = (~repoId=?, ~branch=?, ~pullNumber=?, ~benchmarkName=?, ~worker) => {
+  let make = (~repoId=?, ~branch=?, ~pullNumber=?, ~pullBase=?, ~benchmarkName=?, ~worker) => {
     let ({ReScriptUrql.Hooks.response: response}, _) = {
       ReScriptUrql.Hooks.useQuery(~query=module(GetAllRepos), ())
     }
@@ -293,16 +294,16 @@ module RepoView = {
     let onSelectDateRange = (startDate, endDate) => setDateRange(_ => (startDate, endDate))
 
     let setWorker = worker => {
-      switch (repoId, pullNumber) {
-      | (Some(repoId), None) =>
-        AppRouter.Repo({repoId: repoId, benchmarkName: benchmarkName, worker: worker})->AppRouter.go
-      | (Some(repoId), Some(pullNumber)) =>
+      switch (repoId, pullNumber, pullBase) {
+      | (Some(repoId), Some(pullNumber), Some(pullBase)) =>
         AppRouter.RepoPull({
           repoId: repoId,
           pullNumber: pullNumber,
+          pullBase: pullBase,
           benchmarkName: benchmarkName,
           worker: worker,
         })->AppRouter.go
+      | (Some(repoId), _, _) => AppRouter.Repo({repoId, benchmarkName, worker})->AppRouter.go
       | _ => ()
       }
     }
@@ -348,27 +349,28 @@ module RepoView = {
         | Some(repoId) if !(repoIds->BeltHelpers.Array.contains(repoId)) =>
           <ErrorView msg={"No such repository: " ++ repoId} repoIds onSelectRepoId />
         | Some(repoId) =>
-          let branchBreadcrumb = branch->Belt.Option.getWithDefault(Sidebar.defaultBranch)
+          let pullBase = Belt.Option.getWithDefault(pullBase, "")
           let breadcrumbs =
             <Row sx=[Sx.w.auto, Sx.text.noUnderline] alignY=#center>
-              <Text weight=#semibold> "/" </Text>
-              {
-                let href = AppRouter.RepoBranch({
-                  repoId: repoId,
-                  branch: branchBreadcrumb,
-                  benchmarkName: None,
-                  worker: worker,
-                })->AppRouter.path
-                <Link href text={branchBreadcrumb} />
-              }
               {pullNumber->Rx.onSome(pullNumber => {
                 let href = AppRouter.RepoPull({
                   repoId: repoId,
                   pullNumber: pullNumber,
+                  pullBase: pullBase,
                   benchmarkName: None,
                   worker: worker,
                 })->AppRouter.path
                 <>
+                <Text weight=#semibold> "/" </Text>
+                {
+                  let href = AppRouter.RepoBranch({
+                    repoId: repoId,
+                    branch: pullBase,
+                    benchmarkName: None,
+                    worker: worker,
+                  })->AppRouter.path
+                  <Link href text={pullBase} />
+                }
                   <Text weight=#semibold> "/" </Text>
                   <Link href icon=Icon.branch text={string_of_int(pullNumber)} />
                 </>
@@ -385,6 +387,7 @@ module RepoView = {
                   AppRouter.RepoPull({
                     repoId: repoId,
                     pullNumber: pullNumber,
+                    pullBase: pullBase,
                     benchmarkName: Some(benchmarkName),
                     worker: worker,
                   })
@@ -407,7 +410,7 @@ module RepoView = {
                 {githubLink}
                 <Litepicker startDate endDate sx=[Sx.w.xl5] onSelect={onSelectDateRange} />
               </Topbar>
-              <BenchmarkView repoId worker ?branch ?pullNumber ?benchmarkName startDate endDate />
+              <BenchmarkView repoId worker ?branch ?pullNumber pullBase ?benchmarkName startDate endDate />
             </Column>
           </>
         }}
@@ -424,8 +427,8 @@ let make = () => {
     <div className={Sx.make([Sx.container, Sx.d.flex])}> <ErrorView msg={reason} /> </div>
   | Ok(Main) => <RepoView worker={None} />
   | Ok(Repo({repoId, benchmarkName, worker})) => <RepoView repoId ?benchmarkName worker />
-  | Ok(RepoPull({repoId, pullNumber, benchmarkName, worker})) =>
-    <RepoView repoId pullNumber ?benchmarkName worker />
+  | Ok(RepoPull({repoId, pullNumber, pullBase, benchmarkName, worker})) =>
+    <RepoView repoId pullNumber pullBase ?benchmarkName worker />
   | Ok(RepoBranch({repoId, branch, benchmarkName, worker})) =>
     <RepoView repoId branch ?benchmarkName worker />
   }

diff --git a/frontend/src/AppRouter.res b/frontend/src/AppRouter.res
@@ -3,7 +3,13 @@ type worker = option<(string, string)>
 type route =
   | Main
   | Repo({repoId: string, benchmarkName: option<string>, worker: worker})
-  | RepoPull({repoId: string, pullNumber: int, benchmarkName: option<string>, worker: worker})
+  | RepoPull({
+      repoId: string,
+      pullNumber: int,
+      pullBase: string,
+      benchmarkName: option<string>,
+      worker: worker,
+    })
   | RepoBranch({repoId: string, branch: string, benchmarkName: option<string>, worker: worker})
 
 type error = {
@@ -64,28 +70,30 @@ let route = (url: RescriptReactRouter.url) => {
         worker,
       }),
     )
-  | list{orgName, repoName, "pull", pullNumberStr} =>
+  | list{orgName, repoName, "pull", pullNumberStr, "base", pullBase} =>
     switch Belt.Int.fromString(pullNumberStr) {
     | Some(pullNumber) =>
       Ok(
         RepoPull({
           repoId: orgName ++ "/" ++ repoName,
-          pullNumber: pullNumber,
+          pullNumber,
+          pullBase,
           benchmarkName: None,
-          worker
+          worker,
         }),
       )
     | None => Error({path: url.path, reason: "Invalid pull number: " ++ pullNumberStr})
     }
-  | list{orgName, repoName, "pull", pullNumberStr, "benchmark", benchmarkName} =>
+  | list{orgName, repoName, "pull", pullNumberStr, "base", pullBase, "benchmark", benchmarkName} =>
     switch Belt.Int.fromString(pullNumberStr) {
     | Some(pullNumber) =>
       Ok(
         RepoPull({
           repoId: orgName ++ "/" ++ repoName,
-          pullNumber: pullNumber,
+          pullNumber,
+          pullBase,
           benchmarkName: Some(benchmarkName),
-          worker
+          worker,
         }),
       )
     | None => Error({path: url.path, reason: "Invalid pull number: " ++ pullNumberStr})
@@ -107,13 +115,21 @@ let path = route =>
   | Repo({repoId, benchmarkName: None, worker}) => "/" ++ repoId ++ workerParams(worker)
   | Repo({repoId, benchmarkName: Some(benchmarkName), worker}) =>
     "/" ++ repoId ++ "/benchmark/" ++ benchmarkName ++ workerParams(worker)
-  | RepoPull({repoId, pullNumber, benchmarkName: None, worker}) =>
-    "/" ++ repoId ++ "/pull/" ++ Belt.Int.toString(pullNumber) ++ workerParams(worker)
-  | RepoPull({repoId, pullNumber, benchmarkName: Some(benchmarkName), worker}) =>
+  | RepoPull({repoId, pullNumber, pullBase, benchmarkName: None, worker}) =>
     "/" ++
     repoId ++
     "/pull/" ++
     Belt.Int.toString(pullNumber) ++
+    "/base/" ++
+    pullBase ++
+    workerParams(worker)
+  | RepoPull({repoId, pullNumber, pullBase, benchmarkName: Some(benchmarkName), worker}) =>
+    "/" ++
+    repoId ++
+    "/pull/" ++
+    Belt.Int.toString(pullNumber) ++
+    "/base/" ++
+    pullBase ++
     "/benchmark/" ++
     benchmarkName ++
     workerParams(worker)

diff --git a/frontend/src/BenchmarkQueryHelpers.res b/frontend/src/BenchmarkQueryHelpers.res
@@ -16,8 +16,8 @@ fragment BenchmarkMetrics on benchmarks {
 module GetBenchmarks = %graphql(`
 query ($repoId: String!,
        $pullNumber: Int,
+       $pullBase: String,
        $branch: String,
-       $defaultBranch: String!,
        $isMaster: Boolean!,
        $worker: String,
        $dockerImage: String,
@@ -40,7 +40,7 @@ query ($repoId: String!,
   }
   comparisonBenchmarks:
     benchmarks(where: {_and: [{pull_number: {_is_null: true}},
-                              {branch: {_eq: $defaultBranch}},
+                              {branch: {_eq: $pullBase}},
                               {repo_id: {_eq: $repoId}},
                               {worker: {_eq: $worker}},
                               {docker_image: {_eq: $dockerImage}},

diff --git a/frontend/src/Sidebar.res b/frontend/src/Sidebar.res
@@ -11,6 +11,7 @@ module SidebarMenuData = %graphql(`
 query ($repoId: String!) {
   pullsMenuData: benchmark_metadata(distinct_on: [pull_number], where: {_and: [{repo_id: {_eq: $repoId}}, {pull_number: {_is_null: false}}]}, order_by: [{pull_number: desc}]) {
     pull_number
+    pull_base
     is_open_pr
     branch
     pr_title
@@ -29,7 +30,7 @@ module PullsList = {
   let make = (~repoId, ~pullNumberInfos, ~selectedPull=?, ~selectedBenchmarkName=?, ~worker) => {
     pullNumberInfos
     ->Belt.Array.mapWithIndex((i, pullNumberInfo) => {
-      let (pullNumber, prTitle) = pullNumberInfo
+      let (pullNumber, pullBase, prTitle) = pullNumberInfo
 
       <Row key={string_of_int(i)}>
         <a
@@ -51,6 +52,7 @@ module PullsList = {
           href={AppRouter.RepoPull({
             repoId: repoId,
             pullNumber: pullNumber,
+            pullBase: pullBase,
             benchmarkName: selectedBenchmarkName,
             worker: worker,
           })->AppRouter.path}
@@ -72,17 +74,17 @@ module PullsMenu = {
     ~worker,
   ) => {
     let openPullNumberInfos = pullsMenuData->Belt.Array.keepMap(obj =>
-      switch obj.pull_number {
-      | Some(pullNumber) if obj.is_open_pr =>
-        Some(pullNumber, Belt.Option.getWithDefault(obj.pr_title, ""))
+      switch (obj.pull_number, obj.pull_base) {
+      | (Some(pullNumber), Some(pullBase)) if obj.is_open_pr =>
+        Some(pullNumber, pullBase, Belt.Option.getWithDefault(obj.pr_title, ""))
       | _ => None
       }
     )
 
     let closedPullNumberInfos = pullsMenuData->Belt.Array.keepMap(obj =>
-      switch obj.pull_number {
-      | Some(pullNumber) if !obj.is_open_pr =>
-        Some(pullNumber, Belt.Option.getWithDefault(obj.pr_title, ""))
+      switch (obj.pull_number, obj.pull_base) {
+      | (Some(pullNumber), Some(pullBase)) if !obj.is_open_pr =>
+        Some(pullNumber, pullBase, Belt.Option.getWithDefault(obj.pr_title, ""))
       | _ => None
       }
     )
@@ -118,22 +120,24 @@ module BenchmarksMenu = {
     ~repoId,
     ~benchmarksMenuData: array<SidebarMenuData.t_benchmarksMenuData>,
     ~selectedPull=?,
+    ~selectedPullBase=?,
     ~selectedBenchmarkName=?,
     ~worker,
   ) => {
     benchmarksMenuData
     ->Belt.Array.mapWithIndex((i, {benchmark_name: benchmarkName}) => {
-      let benchmarkRoute = switch selectedPull {
-      | None =>
-        AppRouter.Repo({
+      let benchmarkRoute = switch (selectedPull, selectedPullBase) {
+      | (Some(pullNumber), Some(pullBase)) =>
+        AppRouter.RepoPull({
           repoId: repoId,
+          pullNumber: pullNumber,
+          pullBase: pullBase,
           benchmarkName: Some(benchmarkName),
           worker: worker,
         })
-      | Some(pullNumber) =>
-        AppRouter.RepoPull({
+      | _ =>
+        AppRouter.Repo({
           repoId: repoId,
-          pullNumber: pullNumber,
           benchmarkName: Some(benchmarkName),
           worker: worker,
         })