diff --git a/DESCRIPTION b/DESCRIPTION index 0884b9f..f73982c 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: conbenchcoms Title: An API wrapper for conbench communications -Version: 0.0.8 +Version: 0.0.9 Authors@R: c( person("Jonathan", "Keane", , "jon@voltrondata.com", role = c("aut", "ctb"), comment = c(ORCID = "0000-0001-7087-9776")), diff --git a/NAMESPACE b/NAMESPACE index 90e60e1..008942b 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -2,7 +2,8 @@ export(benchmark_results) export(benchmarks) -export(compare) +export(compare_results) +export(compare_runs) export(conbench_perform) export(conbench_request) export(hardware) diff --git a/NEWS.md b/NEWS.md index 7cb412b..8641c0f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +# conbenchcoms 0.0.9 +* Remove `compare` function +* Add `compare_results` function (similar to old `compare` but only works for comparing benchmark results) +* Add `compare_runs` function, which returns a tibble, paginating until all matching data is returned, which works only with Conbench servers of at least version [ad06af9](https://github.com/conbench/conbench/commit/ad06af9) + # conbenchcoms 0.0.8 * Remove `simplifyVector` and `flatten` arguments from `history` * `history` now always returns a `tibble` diff --git a/R/compare.R b/R/compare.R index 0486229..a013dc0 100644 --- a/R/compare.R +++ b/R/compare.R @@ -1,9 +1,8 @@ -#' Compare runs, benchmarks, batches, or commits +#' Compare benchmark results #' -#' @param type the type of comparison to make (one of: runs, benchmarks, batches, commits) -#' @param baseline the baseline sha of the entity to compare -#' @param contender the contender sha of the entity to compare -#' @param zscore_threshold the zscore threshold to mark regressions and improvements. +#' @param baseline the ID of the baseline result to compare +#' @param contender the ID of the contender result to compare +#' @param zscore_threshold the zscore threshold to mark regressions and improvements. #' Default is defined at the Conbench api level. #' @param pairwise_percent_threshold the pairwise_percent_threshold to mark regressions and improvements. #' Default is defined at the Conbench api level. @@ -11,20 +10,18 @@ #' #' @return the JSON response #' @export -compare <- function(type = c("runs", "benchmarks", "batches", "commits"), - baseline, - contender, - zscore_threshold = NULL, - pairwise_percent_threshold = NULL, - ...) { - type <- match.arg(type) +compare_results <- function(baseline, + contender, + zscore_threshold = NULL, + pairwise_percent_threshold = NULL, + ...) { stopifnot("zscore_threshold must be numeric" = is.numeric(zscore_threshold) || is.null(zscore_threshold)) stopifnot("pairwise_percent_threshold must be numeric" = is.numeric(pairwise_percent_threshold) || is.null(pairwise_percent_threshold)) req <- req_url_path_append( conbench_request(), "compare", - type, + "benchmark-results", paste0(baseline, "...", contender) ) @@ -40,3 +37,66 @@ compare <- function(type = c("runs", "benchmarks", "batches", "commits"), resp_body_json(resp, ...) } + +#' Compare runs +#' +#' @param baseline the ID of the baseline run to compare +#' @param contender the ID of the contender run to compare +#' @param zscore_threshold the zscore threshold to mark regressions and improvements. +#' Default is defined at the Conbench api level. +#' @param pairwise_percent_threshold the pairwise_percent_threshold to mark regressions and improvements. +#' Default is defined at the Conbench api level. +#' +#' @return a tibble of run comparisons +#' @export +compare_runs <- function(baseline, + contender, + zscore_threshold = NULL, + pairwise_percent_threshold = NULL) { + stopifnot("zscore_threshold must be numeric" = is.numeric(zscore_threshold) || is.null(zscore_threshold)) + stopifnot("pairwise_percent_threshold must be numeric" = is.numeric(pairwise_percent_threshold) || is.null(pairwise_percent_threshold)) + + req <- req_url_path_append( + conbench_request(), + "compare", + "runs", + paste0(baseline, "...", contender) + ) + req <- req_url_query(req, page_size = 500) + if (!is.null(zscore_threshold)) { + req <- req_url_query(req, threshold_z = zscore_threshold) + } + if (!is.null(pairwise_percent_threshold)) { + req <- req_url_query(req, threshold = pairwise_percent_threshold) + } + + resp <- conbench_perform(req) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) + data <- dplyr::as_tibble(json[["data"]]) + + while (!is.null(json[["metadata"]][["next_page_cursor"]])) { + req <- req_url_path_append( + conbench_request(), + "compare", + "runs", + paste0(baseline, "...", contender) + ) + req <- req_url_query( + req, + page_size = 500, + cursor = json[["metadata"]][["next_page_cursor"]] + ) + if (!is.null(zscore_threshold)) { + req <- req_url_query(req, threshold_z = zscore_threshold) + } + if (!is.null(pairwise_percent_threshold)) { + req <- req_url_query(req, threshold = pairwise_percent_threshold) + } + + resp <- conbench_perform(req) + json <- resp_body_json(resp, simplifyVector = TRUE, flatten = TRUE) + data <- dplyr::bind_rows(data, dplyr::as_tibble(json[["data"]])) + } + + data +} diff --git a/man/compare.Rd b/man/compare_results.Rd similarity index 70% rename from man/compare.Rd rename to man/compare_results.Rd index 8ada6bb..23f202e 100644 --- a/man/compare.Rd +++ b/man/compare_results.Rd @@ -1,11 +1,10 @@ % Generated by roxygen2: do not edit by hand % Please edit documentation in R/compare.R -\name{compare} -\alias{compare} -\title{Compare runs, benchmarks, batches, or commits} +\name{compare_results} +\alias{compare_results} +\title{Compare benchmark results} \usage{ -compare( - type = c("runs", "benchmarks", "batches", "commits"), +compare_results( baseline, contender, zscore_threshold = NULL, @@ -14,11 +13,9 @@ compare( ) } \arguments{ -\item{type}{the type of comparison to make (one of: runs, benchmarks, batches, commits)} +\item{baseline}{the ID of the baseline result to compare} -\item{baseline}{the baseline sha of the entity to compare} - -\item{contender}{the contender sha of the entity to compare} +\item{contender}{the ID of the contender result to compare} \item{zscore_threshold}{the zscore threshold to mark regressions and improvements. Default is defined at the Conbench api level.} @@ -40,5 +37,5 @@ booleans, numbers, and strings) be caused to atomic vectors?} the JSON response } \description{ -Compare runs, benchmarks, batches, or commits +Compare benchmark results } diff --git a/man/compare_runs.Rd b/man/compare_runs.Rd new file mode 100644 index 0000000..0833b92 --- /dev/null +++ b/man/compare_runs.Rd @@ -0,0 +1,30 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/compare.R +\name{compare_runs} +\alias{compare_runs} +\title{Compare runs} +\usage{ +compare_runs( + baseline, + contender, + zscore_threshold = NULL, + pairwise_percent_threshold = NULL +) +} +\arguments{ +\item{baseline}{the ID of the baseline run to compare} + +\item{contender}{the ID of the contender run to compare} + +\item{zscore_threshold}{the zscore threshold to mark regressions and improvements. +Default is defined at the Conbench api level.} + +\item{pairwise_percent_threshold}{the pairwise_percent_threshold to mark regressions and improvements. +Default is defined at the Conbench api level.} +} +\value{ +a tibble of run comparisons +} +\description{ +Compare runs +} diff --git a/tests/testthat/resp-class/localhost/api/compare/runs/10aded...0af-81f373.json b/tests/testthat/resp-class/localhost/api/compare/runs/10aded...0af-81f373.json deleted file mode 100644 index 39771fb..0000000 --- a/tests/testthat/resp-class/localhost/api/compare/runs/10aded...0af-81f373.json +++ /dev/null @@ -1,18 +0,0 @@ -[ - { - "analysis": { - "lookback_z_score": { - "improvement_indicated": false, - "regression_indicated": false, - "z_score": -0.5, - "z_threshold": 11.1 - }, - "pairwise": { - "improvement_indicated": false, - "percent_change": -26, - "percent_threshold": 20.2, - "regression_indicated": true - } - } - } -] diff --git a/tests/testthat/resp-class/localhost/api/compare/runs/ee1...f00d-dd5b56.json b/tests/testthat/resp-class/localhost/api/compare/runs/ee1...f00d-dd5b56.json deleted file mode 100644 index 0de234f..0000000 --- a/tests/testthat/resp-class/localhost/api/compare/runs/ee1...f00d-dd5b56.json +++ /dev/null @@ -1,18 +0,0 @@ -[ - { - "analysis": { - "lookback_z_score": { - "improvement_indicated": false, - "regression_indicated": false, - "z_score": -0.5, - "z_threshold": 10 - }, - "pairwise": { - "improvement_indicated": false, - "percent_change": -26, - "percent_threshold": 20, - "regression_indicated": true - } - } - } -] diff --git a/tests/testthat/resp-class/localhost/api/benchmark-results-461b95.json b/tests/testthat/resp/localhost/api/benchmark-results-461b95.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/benchmark-results-461b95.json rename to tests/testthat/resp/localhost/api/benchmark-results-461b95.json diff --git a/tests/testthat/resp-class/localhost/api/benchmark-results-5948df.json b/tests/testthat/resp/localhost/api/benchmark-results-5948df.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/benchmark-results-5948df.json rename to tests/testthat/resp/localhost/api/benchmark-results-5948df.json diff --git a/tests/testthat/resp-class/localhost/api/benchmark-results-74052b.json b/tests/testthat/resp/localhost/api/benchmark-results-74052b.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/benchmark-results-74052b.json rename to tests/testthat/resp/localhost/api/benchmark-results-74052b.json diff --git a/tests/testthat/resp-class/localhost/api/benchmark-results-9473a1.json b/tests/testthat/resp/localhost/api/benchmark-results-9473a1.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/benchmark-results-9473a1.json rename to tests/testthat/resp/localhost/api/benchmark-results-9473a1.json diff --git a/tests/testthat/resp-class/localhost/api/benchmark-results-bc1336.json b/tests/testthat/resp/localhost/api/benchmark-results-bc1336.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/benchmark-results-bc1336.json rename to tests/testthat/resp/localhost/api/benchmark-results-bc1336.json diff --git a/tests/testthat/resp/localhost/api/compare/benchmark-results/10aded...0af-81f373.json b/tests/testthat/resp/localhost/api/compare/benchmark-results/10aded...0af-81f373.json new file mode 100644 index 0000000..540f114 --- /dev/null +++ b/tests/testthat/resp/localhost/api/compare/benchmark-results/10aded...0af-81f373.json @@ -0,0 +1,16 @@ +{ + "analysis": { + "lookback_z_score": { + "improvement_indicated": false, + "regression_indicated": false, + "z_score": -0.5, + "z_threshold": 11.1 + }, + "pairwise": { + "improvement_indicated": false, + "percent_change": -26, + "percent_threshold": 20.2, + "regression_indicated": true + } + } +} diff --git a/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-3bead7.json b/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-3bead7.json new file mode 100644 index 0000000..4b4d520 --- /dev/null +++ b/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-3bead7.json @@ -0,0 +1,23 @@ +{ + "data": [ + { + "analysis": { + "lookback_z_score": { + "improvement_indicated": false, + "regression_indicated": false, + "z_score": -0.5, + "z_threshold": 11.1 + }, + "pairwise": { + "improvement_indicated": false, + "percent_change": -26, + "percent_threshold": 20.2, + "regression_indicated": true + } + } + } + ], + "metadata": { + "next_page_cursor": "curse" + } +} diff --git a/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-b7f6c4.json b/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-b7f6c4.json new file mode 100644 index 0000000..4c9243e --- /dev/null +++ b/tests/testthat/resp/localhost/api/compare/runs/10aded...0af-b7f6c4.json @@ -0,0 +1,23 @@ +{ + "data": [ + { + "analysis": { + "lookback_z_score": { + "improvement_indicated": false, + "regression_indicated": false, + "z_score": -0.5, + "z_threshold": 11.1 + }, + "pairwise": { + "improvement_indicated": false, + "percent_change": -26, + "percent_threshold": 20.2, + "regression_indicated": true + } + } + } + ], + "metadata": { + "next_page_cursor": null + } +} diff --git a/tests/testthat/resp-class/localhost/api/hardware.json b/tests/testthat/resp/localhost/api/hardware.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/hardware.json rename to tests/testthat/resp/localhost/api/hardware.json diff --git a/tests/testthat/resp-class/localhost/api/history/hist-id-981e53.json b/tests/testthat/resp/localhost/api/history/hist-id-981e53.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/history/hist-id-981e53.json rename to tests/testthat/resp/localhost/api/history/hist-id-981e53.json diff --git a/tests/testthat/resp-class/localhost/api/info.json b/tests/testthat/resp/localhost/api/info.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/info.json rename to tests/testthat/resp/localhost/api/info.json diff --git a/tests/testthat/resp-class/localhost/api/runs-ebe91f.json b/tests/testthat/resp/localhost/api/runs-ebe91f.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/runs-ebe91f.json rename to tests/testthat/resp/localhost/api/runs-ebe91f.json diff --git a/tests/testthat/resp-class/localhost/api/runs-f23dbb.json b/tests/testthat/resp/localhost/api/runs-f23dbb.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/runs-f23dbb.json rename to tests/testthat/resp/localhost/api/runs-f23dbb.json diff --git a/tests/testthat/resp-class/localhost/api/runs-f323bc.json b/tests/testthat/resp/localhost/api/runs-f323bc.json similarity index 100% rename from tests/testthat/resp-class/localhost/api/runs-f323bc.json rename to tests/testthat/resp/localhost/api/runs-f323bc.json diff --git a/tests/testthat/test-benchmark-results.R b/tests/testthat/test-benchmark-results.R index 47910dc..589b431 100644 --- a/tests/testthat/test-benchmark-results.R +++ b/tests/testthat/test-benchmark-results.R @@ -12,7 +12,7 @@ with_mock_dir(test_path("logged-in"), { }) }) -with_mock_dir(test_path("resp-class"), { +with_mock_dir(test_path("resp"), { run_id_1 <- "5a1ad" run_id_2 <- "5eaf00d" batch_id_1 <- "abba0123" diff --git a/tests/testthat/test-compare.R b/tests/testthat/test-compare.R index a955869..fc5a0a2 100644 --- a/tests/testthat/test-compare.R +++ b/tests/testthat/test-compare.R @@ -1,70 +1,78 @@ with_mock_dir(test_path("logged-in"), { test_that("compare()", { expect_GET( - compare("runs", "C0C0A", "BA11E7"), - "http://localhost/api/compare/runs/C0C0A...BA11E7" + compare_results("C0C0A", "BA11E7"), + "http://localhost/api/compare/benchmark-results/C0C0A...BA11E7" ) expect_GET( - compare("benchmarks", "C0C0A", "BA11E7"), - "http://localhost/api/compare/benchmarks/C0C0A...BA11E7" + compare_runs("C0C0A", "BA11E7"), + "http://localhost/api/compare/runs/C0C0A...BA11E7?page_size=500" ) }) }) -with_mock_dir(test_path("resp-class"), { - test_that("zscore and pairwise thresholds return correct values", { - z_thres <- 10 - percent_threshold <- 20 +with_mock_dir(test_path("resp"), { + test_that("compare_results returns the correct json", { + z_thres <- 11.1 + percent_threshold <- 20.2 - resp <- compare( - type = "runs", - "ee1", - "f00d", + resp <- compare_results( + "10aded", + "0af", zscore_threshold = z_thres, - pairwise_percent_threshold = z_thres, - simplifyVector = TRUE + pairwise_percent_threshold = z_thres ) - returned_z_thres <- unique(resp[["analysis"]][["lookback_z_score"]][["z_threshold"]]) - expect_equal(returned_z_thres[!is.na(returned_z_thres)], z_thres) - returned_percent_thres <- unique(resp[["analysis"]][["pairwise"]][["percent_threshold"]]) - expect_equal(returned_percent_thres[!is.na(returned_percent_thres)], percent_threshold) + expect_type(resp, "list") + + returned_z_thres <- resp[["analysis"]][["lookback_z_score"]][["z_threshold"]] + expect_equal(returned_z_thres, z_thres) + returned_percent_thres <- resp[["analysis"]][["pairwise"]][["percent_threshold"]] + expect_equal(returned_percent_thres, percent_threshold) }) - test_that("zscore and pairwise thresholds can accept decimals", { + test_that("compare_runs returns the correct tibble", { z_thres <- 11.1 percent_threshold <- 20.2 - resp <- compare( - type = "runs", + resp <- compare_runs( "10aded", "0af", zscore_threshold = z_thres, - pairwise_percent_threshold = z_thres, - simplifyVector = TRUE + pairwise_percent_threshold = z_thres ) - returned_z_thres <- unique(resp[["analysis"]][["lookback_z_score"]][["z_threshold"]]) - expect_equal(returned_z_thres[!is.na(returned_z_thres)], z_thres) - returned_percent_thres <- unique(resp[["analysis"]][["pairwise"]][["percent_threshold"]]) - expect_equal(returned_percent_thres[!is.na(returned_percent_thres)], percent_threshold) + expect_s3_class(resp, "tbl_df") + + returned_z_thres <- unique(resp[["analysis.lookback_z_score.z_threshold"]]) + expect_equal(returned_z_thres, z_thres) + returned_percent_thres <- unique(resp[["analysis.pairwise.percent_threshold"]]) + expect_equal(returned_percent_thres, percent_threshold) }) test_that("zscore and pairwise thresholds fail with non-numeric values", { - expect_error(compare( - type = "runs", + expect_error(compare_results( + "10aded", + "0af", + zscore_threshold = "foo" + ), "zscore_threshold must be numeric") + + expect_error(compare_results( + "10aded", + "0af", + pairwise_percent_threshold = "bar" + ), "pairwise_percent_threshold must be numeric") + + expect_error(compare_runs( "10aded", "0af", - zscore_threshold = "foo", - simplifyVector = TRUE + zscore_threshold = "foo" ), "zscore_threshold must be numeric") - expect_error(compare( - type = "runs", + expect_error(compare_runs( "10aded", "0af", - pairwise_percent_threshold = "bar", - simplifyVector = TRUE + pairwise_percent_threshold = "bar" ), "pairwise_percent_threshold must be numeric") }) }) diff --git a/tests/testthat/test-hardware.R b/tests/testthat/test-hardware.R index 67f9a50..1bfd809 100644 --- a/tests/testthat/test-hardware.R +++ b/tests/testthat/test-hardware.R @@ -13,7 +13,7 @@ with_mock_dir(test_path("logged-in"), { }) -with_mock_dir(test_path("resp-class"), { +with_mock_dir(test_path("resp"), { test_that("hardware() returns a data.frame", { the_hardware <- hardware() expect_s3_class( diff --git a/tests/testthat/test-history.R b/tests/testthat/test-history.R index b0dd115..3553239 100644 --- a/tests/testthat/test-history.R +++ b/tests/testthat/test-history.R @@ -8,7 +8,7 @@ with_mock_dir(test_path("logged-in"), { }) -with_mock_dir(test_path("resp-class"), { +with_mock_dir(test_path("resp"), { the_id <- "hist-id" test_that("history() returns a tibble", { the_history <- history(benchmark_id = the_id) diff --git a/tests/testthat/test-info.R b/tests/testthat/test-info.R index 180a563..42806b6 100644 --- a/tests/testthat/test-info.R +++ b/tests/testthat/test-info.R @@ -13,7 +13,7 @@ with_mock_dir(test_path("logged-in"), { }) -with_mock_dir(test_path("resp-class"), { +with_mock_dir(test_path("resp"), { test_that("info() returns a data.frame", { the_info <- info() expect_s3_class( diff --git a/tests/testthat/test-runs.R b/tests/testthat/test-runs.R index cb99d51..e4764a0 100644 --- a/tests/testthat/test-runs.R +++ b/tests/testthat/test-runs.R @@ -7,7 +7,7 @@ with_mock_dir(test_path("logged-in"), { }) }) -with_mock_dir(test_path("resp-class"), { +with_mock_dir(test_path("resp"), { sha_1 <- "babb1ed" sha_2 <- "5eaf00d" test_that("runs() returns a tibble", {