From 195a18fd27feb6048521b48e9a2bc21aeeed97c5 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sun, 16 Oct 2022 14:22:58 +0200 Subject: [PATCH 1/3] ensure tests that require serial processing are also processed after all other tests have been ran --- R/testing.R | 6 +++ tests/testthat.R | 7 +++- tests/testthat/test-cache-high-level-api.R | 10 ++++- ...-cache-interaction-roxygen-code-examples.R | 12 ------ tests/testthat/test-cache-with-r-cache.R | 31 -------------- tests/testthat/tests-cache-require-serial.R | 42 +++++++++++++++++++ 6 files changed, 63 insertions(+), 45 deletions(-) create mode 100644 tests/testthat/tests-cache-require-serial.R diff --git a/R/testing.R b/R/testing.R index 4149dfea8..0c47fe8c4 100644 --- a/R/testing.R +++ b/R/testing.R @@ -377,3 +377,9 @@ test_transformers_drop <- function(transformers) { } }) } + + +skip_during_parallel <- function() { + is_parallel <- as.logical(as.logical(toupper(Sys.getenv("STYLER_TEST_IS_TRULY_PARALLEL", TRUE)))) + testthat::skip_if(is_parallel) +} diff --git a/tests/testthat.R b/tests/testthat.R index 08ba76957..7c7b8c20d 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -1,4 +1,9 @@ library(testthat) library(styler) +test_check("styler") # checks multiple files, in parallel -test_check("styler") +# checks file one by one, not parallel +Sys.setenv(STYLER_TEST_IS_TRULY_PARALLEL = FALSE) +test_file("testthat/test-cache-high-level-api.R") +test_file("testthat/test-cache-interaction-roxygen-code-examples.R") +test_file("testthat/tests-cache-require-serial.R") diff --git a/tests/testthat/test-cache-high-level-api.R b/tests/testthat/test-cache-high-level-api.R index 9150e87ac..e645f8bc8 100644 --- a/tests/testthat/test-cache-high-level-api.R +++ b/tests/testthat/test-cache-high-level-api.R @@ -2,6 +2,7 @@ test_that("activated cache brings speedup on style_file() API", { local_test_setup() skip_on_cran() skip_on_covr() + skip_during_parallel() n <- n_times_faster_with_cache( test_path("reference-objects/caching.R"), test_path("reference-objects/caching.R"), @@ -27,6 +28,7 @@ text <- c( test_that("activated cache brings speedup on style_text() API on character vector", { skip_on_cran() skip_on_covr() + skip_during_parallel() n <- n_times_faster_with_cache( text, text, fun = style_text @@ -37,6 +39,7 @@ test_that("activated cache brings speedup on style_text() API on character vecto test_that("activated cache brings speedup on style_text() API on character scalar", { skip_on_cran() skip_on_covr() + skip_during_parallel() text2 <- paste0(text, collapse = "\n") n <- n_times_faster_with_cache( @@ -56,6 +59,7 @@ test_that("trailing line breaks are ignored for caching", { expect_equal(cache_info(format = "tabular")$n, 3) skip_on_cran() skip_on_covr() + skip_during_parallel() n <- n_times_faster_with_cache(text1, text2) expect_gt(n, 55) }) @@ -69,6 +73,7 @@ test_that("trailing line breaks are ignored for caching in one scalar", { expect_equal(cache_info(format = "tabular")$n, 3) skip_on_cran() skip_on_covr() + skip_during_parallel() n <- n_times_faster_with_cache(text1, text2) expect_gt(n, 55) }) @@ -85,6 +90,7 @@ test_that("trailing line breaks are ignored for caching in one scalar", { expect_equal(cache_info(format = "tabular")$n, 3) skip_on_cran() skip_on_covr() + skip_during_parallel() n <- n_times_faster_with_cache(text1, text2) expect_gt(n, 55) }) @@ -92,7 +98,7 @@ test_that("trailing line breaks are ignored for caching in one scalar", { test_that("speedup higher when cached roxygen example code is multiple expressions", { skip_on_cran() skip_on_covr() - + skip_during_parallel() text_long <- c( "#' Roxygen", "#' Comment", @@ -132,6 +138,7 @@ test_that("speedup higher when cached roxygen example code is multiple expressio test_that("no speedup when tranformer changes", { skip_on_cran() skip_on_covr() + skip_during_parallel() local_test_setup() t1 <- tidyverse_style() first <- system.time(style_text(text, transformers = t1)) @@ -145,6 +152,7 @@ test_that("unactivated cache does not bring speedup", { skip_on_cran() skip_on_covr() local_test_setup() + skip_during_parallel() first <- system.time(style_file(test_path("reference-objects/caching.R"))) second <- system.time(style_file(test_path("reference-objects/caching.R"))) expect_false(first["elapsed"] / 4 > second["elapsed"]) diff --git a/tests/testthat/test-cache-interaction-roxygen-code-examples.R b/tests/testthat/test-cache-interaction-roxygen-code-examples.R index bfbce2cd3..19c55e835 100644 --- a/tests/testthat/test-cache-interaction-roxygen-code-examples.R +++ b/tests/testthat/test-cache-interaction-roxygen-code-examples.R @@ -42,18 +42,6 @@ test_that("roxzgen code examples are written to cache as both individual express ) }) - -test_that("roxzgen code examples are written to cache as whole expressions bring speedgain", { - skip_on_cran() - local_test_setup(cache = TRUE) - text <- readLines(test_path("cache-with-r-cache/roxygen-cache-1.R")) - first <- system.time(styled <- style_text(text)) - # don't use full cache, only roxygen cache - styled[1] <- "#' This is a nother text" - second <- system.time(style_text(styled)) - expect_gt(first["elapsed"], 4 * second["elapsed"]) -}) - test_that("cache is deactivated at end of caching related testthat file", { expect_false(cache_is_activated()) }) diff --git a/tests/testthat/test-cache-with-r-cache.R b/tests/testthat/test-cache-with-r-cache.R index 3d5399157..318121d50 100644 --- a/tests/testthat/test-cache-with-r-cache.R +++ b/tests/testthat/test-cache-with-r-cache.R @@ -25,37 +25,6 @@ test_that("Cache management works", { expect_error(cache_clear("testthat", ask = FALSE), NA) }) -test_that("top-level test: Caches top-level expressions efficiently on style_text()", { - local_test_setup(cache = TRUE) - text <- test_path("cache-with-r-cache/mlflow-1-in.R") %>% - readLines() - benchmark <- system.time(text_styled <- as.character(style_text(text))) - expect_equal(text, text_styled) - full_cached_benchmark <- system.time(text_styled2 <- as.character(style_text(text_styled))) - expect_equal(text, text_styled2) - - # modify one function declaration - text_styled[2] <- gsub(")", " )", text_styled[2], fixed = TRUE) - partially_cached_benchmark <- system.time( - text_cached_partially <- as.character(style_text(text_styled)) - ) - expect_equal(text, text_cached_partially) - cache_deactivate() - not_cached_benchmark <- system.time( - text_not_cached <- as.character(style_text(text_styled)) - ) - expect_equal(text, text_not_cached) - - skip_on_cran() - skip_on_covr() - expect_lt( - partially_cached_benchmark["elapsed"] * 2.4, - not_cached_benchmark["elapsed"] - ) - expect_lt(full_cached_benchmark["elapsed"] * 45, benchmark["elapsed"]) -}) - - test_that("cached expressions are displayed propperly", { skip_if(getRversion() < "4.2") diff --git a/tests/testthat/tests-cache-require-serial.R b/tests/testthat/tests-cache-require-serial.R new file mode 100644 index 000000000..fcedbcd16 --- /dev/null +++ b/tests/testthat/tests-cache-require-serial.R @@ -0,0 +1,42 @@ + +test_that("top-level test: Caches top-level expressions efficiently on style_text()", { + local_test_setup(cache = TRUE) + text <- test_path("cache-with-r-cache/mlflow-1-in.R") %>% + readLines() + benchmark <- system.time(text_styled <- as.character(style_text(text))) + expect_equal(text, text_styled) + full_cached_benchmark <- system.time(text_styled2 <- as.character(style_text(text_styled))) + expect_equal(text, text_styled2) + + # modify one function declaration + text_styled[2] <- gsub(")", " )", text_styled[2], fixed = TRUE) + partially_cached_benchmark <- system.time( + text_cached_partially <- as.character(style_text(text_styled)) + ) + expect_equal(text, text_cached_partially) + cache_deactivate() + not_cached_benchmark <- system.time( + text_not_cached <- as.character(style_text(text_styled)) + ) + expect_equal(text, text_not_cached) + + skip_on_cran() + skip_on_covr() + expect_lt( + partially_cached_benchmark["elapsed"] * 2.4, + not_cached_benchmark["elapsed"] + ) + expect_lt(full_cached_benchmark["elapsed"] * 45, benchmark["elapsed"]) +}) + + +test_that("roxzgen code examples are written to cache as whole expressions bring speedgain", { + skip_on_cran() + local_test_setup(cache = TRUE) + text <- readLines(test_path("cache-with-r-cache/roxygen-cache-1.R")) + first <- system.time(styled <- style_text(text)) + # don't use full cache, only roxygen cache + styled[1] <- "#' This is a nother text" + second <- system.time(style_text(styled)) + expect_gt(first["elapsed"], 4 * second["elapsed"]) +}) From 8895482344973e55cc3a53aca09f9ad2d65b17c9 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sun, 16 Oct 2022 15:56:40 +0200 Subject: [PATCH 2/3] avoid re-running --- tests/testthat.R | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/testthat.R b/tests/testthat.R index 7c7b8c20d..823ab1776 100644 --- a/tests/testthat.R +++ b/tests/testthat.R @@ -5,5 +5,4 @@ test_check("styler") # checks multiple files, in parallel # checks file one by one, not parallel Sys.setenv(STYLER_TEST_IS_TRULY_PARALLEL = FALSE) test_file("testthat/test-cache-high-level-api.R") -test_file("testthat/test-cache-interaction-roxygen-code-examples.R") test_file("testthat/tests-cache-require-serial.R") From ddba306c0a92155d91065f5565e02e249421bfe8 Mon Sep 17 00:00:00 2001 From: Lorenz Walthert Date: Sun, 16 Oct 2022 16:06:34 +0200 Subject: [PATCH 3/3] refactor --- R/testing.R | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/R/testing.R b/R/testing.R index 0c47fe8c4..c0f839609 100644 --- a/R/testing.R +++ b/R/testing.R @@ -380,6 +380,8 @@ test_transformers_drop <- function(transformers) { skip_during_parallel <- function() { - is_parallel <- as.logical(as.logical(toupper(Sys.getenv("STYLER_TEST_IS_TRULY_PARALLEL", TRUE)))) - testthat::skip_if(is_parallel) + Sys.getenv("STYLER_TEST_IS_TRULY_PARALLEL", TRUE) %>% + toupper() %>% + as.logical() %>% + testthat::skip_if() }