From 20aeb7064d5e4b87cf024a4437af0659f109d744 Mon Sep 17 00:00:00 2001 From: Puneeth Chaganti Date: Thu, 13 Apr 2023 13:23:34 +0530 Subject: [PATCH] Use pullBase to compare benchmarks against correct branch --- frontend/src/App.res | 47 ++++++++++++++------------ frontend/src/AppRouter.res | 36 ++++++++++++++------ frontend/src/BenchmarkQueryHelpers.res | 4 +-- frontend/src/Sidebar.res | 30 +++++++++------- 4 files changed, 70 insertions(+), 47 deletions(-) diff --git a/frontend/src/App.res b/frontend/src/App.res index f3df278b..97b90fbf 100644 --- a/frontend/src/App.res +++ b/frontend/src/App.res @@ -16,6 +16,7 @@ let makeGetBenchmarksVariables = ( ~repoId, ~branch=?, ~pullNumber=?, + ~pullBase=?, ~worker, ~benchmarkName=?, ~startDate, @@ -23,7 +24,6 @@ let makeGetBenchmarksVariables = ( ): 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)) => | Some(repoId) => - let branchBreadcrumb = branch->Belt.Option.getWithDefault(Sidebar.defaultBranch) + let pullBase = Belt.Option.getWithDefault(pullBase, "") let breadcrumbs = - "/" - { - let href = AppRouter.RepoBranch({ - repoId: repoId, - branch: branchBreadcrumb, - benchmarkName: None, - worker: worker, - })->AppRouter.path - - } {pullNumber->Rx.onSome(pullNumber => { let href = AppRouter.RepoPull({ repoId: repoId, pullNumber: pullNumber, + pullBase: pullBase, benchmarkName: None, worker: worker, })->AppRouter.path <> + "/" + { + let href = AppRouter.RepoBranch({ + repoId: repoId, + branch: pullBase, + benchmarkName: None, + worker: worker, + })->AppRouter.path + + } "/" @@ -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} - + }} @@ -424,8 +427,8 @@ let make = () => {
| Ok(Main) => | Ok(Repo({repoId, benchmarkName, worker})) => - | Ok(RepoPull({repoId, pullNumber, benchmarkName, worker})) => - + | Ok(RepoPull({repoId, pullNumber, pullBase, benchmarkName, worker})) => + | Ok(RepoBranch({repoId, branch, benchmarkName, worker})) => } diff --git a/frontend/src/AppRouter.res b/frontend/src/AppRouter.res index 4a4d3a31..9a51b6a8 100644 --- 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, worker: worker}) - | RepoPull({repoId: string, pullNumber: int, benchmarkName: option, worker: worker}) + | RepoPull({ + repoId: string, + pullNumber: int, + pullBase: string, + benchmarkName: option, + worker: worker, + }) | RepoBranch({repoId: string, branch: string, benchmarkName: option, 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 index 741c60af..49d1fedc 100644 --- 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 index c33a3eda..ec29d158 100644 --- 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 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, ~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, })