From 6267b4f797182a9968f36a016cbb5359e2de8b43 Mon Sep 17 00:00:00 2001 From: jgabry Date: Tue, 22 Sep 2026 12:58:17 -0600 Subject: [PATCH 1/2] Test platform branches on the platform they apply to The toolchain and WSL tests mocked os_is_windows() and os_is_wsl() to force a platform branch on every OS. Most of those mocks sat under a skip and did nothing; the rest compared strings that mocked helpers produced. Each test now runs the branch its platform takes, and the Windows and WSL CI jobs cover theirs. The mocked WSL output_dir test is removed because the two real-WSL tests after it check the same behaviour end to end. --- tests/testthat/test-install.R | 25 ++----- tests/testthat/test-model-output_dir.R | 93 -------------------------- tests/testthat/test-utils.R | 50 ++++++-------- 3 files changed, 25 insertions(+), 143 deletions(-) diff --git a/tests/testthat/test-install.R b/tests/testthat/test-install.R index 71efdb92a..b6e902394 100644 --- a/tests/testthat/test-install.R +++ b/tests/testthat/test-install.R @@ -533,11 +533,12 @@ test_that("install_cmdstan() rejects a non-logical copy_make_local", { # Windows toolchain discovery tests ---------------------------------------- test_that("toolchain_PATH_env_var() returns NULL on non-Windows", { + skip_if(os_is_windows()) + old_cache <- .cmdstanr$TOOLCHAIN_PATH on.exit(.cmdstanr$TOOLCHAIN_PATH <- old_cache) .cmdstanr$TOOLCHAIN_PATH <- NULL - local_mocked_bindings(os_is_windows = function() FALSE) expect_null(toolchain_PATH_env_var()) }) @@ -577,7 +578,6 @@ test_that("toolchain_PATH_env_var() uses RTOOLS40_HOME for R < 4.2", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.1.0"), short_path = function(path) path, repair_path = function(path) path @@ -603,7 +603,6 @@ test_that("toolchain_PATH_env_var() uses RTOOLS40_HOME for R < 4.2", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.1.0"), short_path = function(path) path, repair_path = function(path) path @@ -624,6 +623,8 @@ test_that("toolchain_PATH_env_var() uses RTOOLS40_HOME for R < 4.2", { }) test_that("toolchain_PATH_env_var() compares R versions numerically", { + skip_if(!os_is_windows()) + old_cache <- .cmdstanr$TOOLCHAIN_PATH on.exit(.cmdstanr$TOOLCHAIN_PATH <- old_cache) @@ -631,7 +632,6 @@ test_that("toolchain_PATH_env_var() compares R versions numerically", { rcmd_calls <- 0L .cmdstanr$TOOLCHAIN_PATH <- NULL local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.10.0"), .cmdstanr_rcmd = function(...) { rcmd_calls <<- rcmd_calls + 1L @@ -665,7 +665,6 @@ test_that("toolchain_PATH_env_var() uses configured R_TOOLS_SOFT", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), .cmdstanr_rcmd = function(..., stdout = FALSE) fake_soft, short_path = function(path) path, @@ -706,7 +705,6 @@ test_that("toolchain_PATH_env_var() falls back to Sys.which() when Rcmd fails", .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), short_path = function(path) path, repair_path = function(path) path @@ -743,7 +741,6 @@ test_that("toolchain_PATH_env_var() searches PATH when R_TOOLS_SOFT is empty", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), .cmdstanr_rcmd = function(..., stdout = FALSE) "", short_path = function(path) path, @@ -780,7 +777,6 @@ test_that("toolchain_PATH_env_var() returns NULL when both approaches fail", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), short_path = function(path) path, repair_path = function(path) path @@ -807,7 +803,6 @@ test_that("toolchain_PATH_env_var() returns NULL when only one tool in PATH", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), short_path = function(path) path, repair_path = function(path) path @@ -840,7 +835,6 @@ test_that("toolchain_PATH_env_var() falls back to PATH when executables missing .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), short_path = function(path) path, repair_path = function(path) path @@ -876,7 +870,6 @@ test_that("toolchain_PATH_env_var() preserves configured compiler", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), .cmdstanr_rcmd = function(..., stdout = FALSE) fake_soft, short_path = function(path) path, @@ -921,7 +914,6 @@ test_that("toolchain_PATH_env_var() preserves configured make", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), .cmdstanr_rcmd = function(..., stdout = FALSE) fake_soft, short_path = function(path) path, @@ -967,7 +959,6 @@ test_that("toolchain_PATH_env_var() rejects unsafe toolchain paths", { .cmdstanr$TOOLCHAIN_PATH <- NULL local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0"), .cmdstanr_rcmd = function(..., stdout = FALSE) fake_soft, short_path = function(path) path, @@ -994,34 +985,26 @@ test_that("is_ucrt_toolchain() returns correct values for R versions", { # is_ucrt_toolchain() is TRUE for R 4.2.x – 4.x.x on Windows local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.2.0") ) expect_true(is_ucrt_toolchain()) }) local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.4.0") ) expect_true(is_ucrt_toolchain()) }) local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("4.1.0") ) expect_false(is_ucrt_toolchain()) }) local({ local_mocked_bindings( - os_is_windows = function() TRUE, current_r_version = function() numeric_version("5.0.0") ) expect_false(is_ucrt_toolchain()) }) - local({ - local_mocked_bindings(os_is_windows = function() FALSE) - expect_false(is_ucrt_toolchain()) - }) }) diff --git a/tests/testthat/test-model-output_dir.R b/tests/testthat/test-model-output_dir.R index bac9eaac7..711388567 100644 --- a/tests/testthat/test-model-output_dir.R +++ b/tests/testthat/test-model-output_dir.R @@ -4,98 +4,6 @@ local_output_sandbox <- function(pattern = "sandbox", .local_envir = parent.fram withr::local_tempdir(pattern = pattern, .local_envir = .local_envir) } -test_that("WSL output paths stay host-native until command composition", { - # Use minimal method arguments so this test exercises path handling without - # launching CmdStan. - method_args <- list( - method = "sample", - save_metric = NULL, - validate = function(num_procs) invisible(), - compose = function(idx, args) args - ) - # Cover system and non-system Windows drives as well as a WSL UNC path. - host_dirs <- c( - "C:/output", - "D:/output", - "//wsl$/Ubuntu/home/user/output" - ) - wsl_dirs <- c( - "/mnt/c/output", - "/mnt/d/output", - "/home/user/output" - ) - as_wsl_path <- function(path = NULL, revert = FALSE) { - if (is.null(path) || revert) { - return(path) - } - path <- sub("//wsl$/Ubuntu", "", path, fixed = TRUE) - for (i in seq_along(host_dirs)) { - path <- sub(host_dirs[i], wsl_dirs[i], path, fixed = TRUE) - } - path - } - # Simulate Windows R using WSL so this boundary test runs on every platform. - with_mocked_bindings( - { - args <- lapply(host_dirs, function(output_dir) { - CmdStanArgs$new( - model_name = "model", - exe_file = "model", - proc_ids = 1, - method_args = method_args, - output_dir = output_dir, - output_basename = "model" - ) - }) - output_files <- file.path(host_dirs, "model-01.csv") - expect_equal( - vapply(args, function(x) x$output_dir, character(1)), - host_dirs - ) - expect_equal( - vapply(args, function(x) x$new_files("output"), character(1)), - output_files - ) - cmdstan_output_files <- vapply(seq_along(args), function(i) { - command_args <- args[[i]]$compose_all_args( - output_file = output_files[i] - ) - sub("file=", "", command_args[grepl("^file=", command_args)], fixed = TRUE) - }, character(1)) - expect_equal(cmdstan_output_files, file.path(wsl_dirs, "model-01.csv")) - - command_args <- args[[1]]$compose_all_args( - output_file = output_files[1], - profile_file = file.path(host_dirs[1], "model-profile-01.csv"), - latent_dynamics_file = file.path(host_dirs[1], "model-diagnostic-01.csv") - ) - expect_in("diagnostic_file=/mnt/c/output/model-diagnostic-01.csv", command_args) - expect_in("profile_file=/mnt/c/output/model-profile-01.csv", command_args) - - # Omitting output_dir must still use the faster WSL-native temp directory. - default_args <- CmdStanArgs$new( - model_name = "model", - exe_file = "model", - proc_ids = 1, - method_args = method_args, - output_basename = "model" - ) - expect_equal(default_args$output_dir, "//wsl$/Ubuntu/tmp/cmdstanr") - expect_in( - "file=/tmp/cmdstanr/model-01.csv", - default_args$compose_all_args( - output_file = default_args$new_files("output") - ) - ) - }, - os_is_wsl = function() TRUE, - wsl_safe_path = as_wsl_path, - wsl_dir_prefix = function(...) "//wsl$/Ubuntu", - wsl_tempdir = function() "/tmp/cmdstanr", - validate_cmdstan_args = function(self) invisible() - ) -}) - test_that("all fitting methods work with output_dir", { sandbox <- local_output_sandbox() for (method in c("sample", "optimize", "variational")) { @@ -171,7 +79,6 @@ test_that("all fitting methods work with output_dir", { test_that("explicit WSL output paths are usable by Windows R", { skip_if_not(os_is_wsl()) - # Unlike the mocked test above, this exercises the full Windows/WSL workflow. output_dir <- local_output_sandbox("wsl-output-dir") mod <- testing_model("logistic_profiling") utils::capture.output( diff --git a/tests/testthat/test-utils.R b/tests/testthat/test-utils.R index caa52c537..85f7755e3 100644 --- a/tests/testthat/test-utils.R +++ b/tests/testthat/test-utils.R @@ -427,35 +427,27 @@ test_that("repair_path works with multiple paths", { }) test_that("wsl_safe_path() works with multiple paths", { - with_mocked_bindings( - { - expect_equal( - wsl_safe_path( - c( - "/mnt/c/project/init-1.json", - "/mnt/d/project/init-2.json", - "relative/init-3.json" - ), - revert = TRUE - ), - c( - "C:/project/init-1.json", - "D:/project/init-2.json", - "relative/init-3.json" - ) - ) - expect_equal( - wsl_safe_path( - c( - "//wsl$/Ubuntu/tmp/init-1.json", - "//wsl$/Ubuntu/tmp/init-2.json" - ) - ), - c("/tmp/init-1.json", "/tmp/init-2.json") - ) - }, - os_is_wsl = function() TRUE, - wsl_dir_prefix = function(...) "//wsl$/Ubuntu" + skip_if_not(os_is_wsl()) + expect_equal( + wsl_safe_path( + c( + "/mnt/c/project/init-1.json", + "/mnt/d/project/init-2.json", + "relative/init-3.json" + ), + revert = TRUE + ), + c( + "C:/project/init-1.json", + "D:/project/init-2.json", + "relative/init-3.json" + ) + ) + expect_equal( + wsl_safe_path( + paste0(wsl_dir_prefix(), c("/tmp/init-1.json", "/tmp/init-2.json")) + ), + c("/tmp/init-1.json", "/tmp/init-2.json") ) }) From 26459b94dd6e3ef65319b003ed12528b444a3328 Mon Sep 17 00:00:00 2001 From: jgabry Date: Tue, 22 Sep 2026 12:58:30 -0600 Subject: [PATCH 2/2] Update Test-coverage.yaml --- .github/workflows/Test-coverage.yaml | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/.github/workflows/Test-coverage.yaml b/.github/workflows/Test-coverage.yaml index decf31c0d..ae1851317 100644 --- a/.github/workflows/Test-coverage.yaml +++ b/.github/workflows/Test-coverage.yaml @@ -14,6 +14,10 @@ permissions: contents: read id-token: write +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + jobs: test-coverage: if: "! contains(github.event.head_commit.message, '[ci skip]')" @@ -37,12 +41,6 @@ jobs: echo "CMDSTAN_PATH=${HOME}/.cmdstan" >> $GITHUB_ENV shell: bash - - uses: n1hility/cancel-previous-runs@v3 - with: - token: ${{ secrets.GITHUB_TOKEN }} - workflow: Test-coverage.yaml - if: "!startsWith(github.ref, 'refs/tags/') && github.ref != 'refs/heads/master'" - - uses: actions/checkout@v7 - uses: r-lib/actions/setup-r@v2