Skip to content

Commit 11d3993

Browse files
408: skip problematic tests on R-devel non-standard clang linux systems (#415)
[skip vbump] * remove --as-cran from Rstudio project options * start with skipping helpers * rename accordingly * review and add comments * make existing skip on CRAN more specific * additional new skippings based on cran check results * one more * polish news, bump up version * [skip actions] Restyle files * trigger checks * move skipping code to package so we can use it in vignette * update collate * [skip actions] Restyle files * prefix with mmrm in vignette because not exported * use expect_equal for rank deficient ST test * higher tolerance for residual tests * skip 2 tests conditionally --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 67c3665 commit 11d3993

File tree

12 files changed

+262
-40
lines changed

12 files changed

+262
-40
lines changed

DESCRIPTION

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
Type: Package
22
Package: mmrm
33
Title: Mixed Models for Repeated Measures
4-
Version: 0.3.7.9010
4+
Version: 0.3.8
55
Authors@R: c(
66
person("Daniel", "Sabanes Bove", , "daniel.sabanes_bove@roche.com", role = c("aut", "cre")),
77
person("Julia", "Dedic", , "julia.dedic@roche.com", role = "aut"),
@@ -114,6 +114,7 @@ Collate:
114114
'utils-nse.R'
115115
'utils-formula.R'
116116
'satterthwaite.R'
117+
'skipping.R'
117118
'tidiers.R'
118119
'testing.R'
119120
'tmb-methods.R'

NEWS.md

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,22 @@
1-
# mmrm 0.3.7.9010
1+
# mmrm 0.3.8
22

33
### New Features
44

55
- `Anova` is implemented for `mmrm` models and available upon loading the `car` package. It supports type II and III hypothesis testing.
6-
- The argument `start` for `mmrm_control()` is updated to allow function/numeric input for better
7-
choices of initial values.
6+
- The argument `start` for `mmrm_control()` is updated to allow better choices of initial values.
87
- `confint` on `mmrm` models will give t-based confidence intervals now, instead of the normal approximation.
98

10-
### Miscellaneous
9+
### Bug Fixes
1110

12-
- In documentation of `mmrm_control()`, the allowed vcov definition is corrected to "Empirical-Jackknife" (CR3), and "Empirical-Bias-Reduced" (CR2).
13-
- Fix a compiler warning related to missing format specification in error message function call.
14-
- If an empty contrast matrix is provided to `df_md`, it will return statistics with `NA` values.
11+
- Previously if the first optimizer failed, the best successful fit among the remaining optimizers was not returned correctly. This is fixed now.
1512

16-
### Bug Fixes
13+
### Miscellaneous
1714

18-
- Previously if the first optimizer fails, the best successful fit among the rest optimizer
19-
will be returned. However the index of it was incorrect, and this issue is fixed now.
15+
- In documentation of `mmrm_control()`, the allowed `vcov` definition is corrected to "Empirical-Jackknife" (CR3), and "Empirical-Bias-Reduced" (CR2).
16+
- Fixed a compiler warning related to missing format specification.
17+
- If an empty contrast matrix is provided to `df_md`, it will return statistics with `NA` values.
2018

21-
# mmrm 0.3.6
19+
# mmrm 0.3.7
2220

2321
### New Features
2422

R/skipping.R

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
# Internal functions used for skipping tests or examples.
2+
3+
# Predicate whether currently running R version is under development.
4+
is_r_devel <- function() {
5+
grepl("devel", R.version$status)
6+
}
7+
8+
# Predicate whether currently running on a Linux operating system.
9+
is_linux <- function() {
10+
tolower(Sys.info()[["sysname"]]) == "linux"
11+
}
12+
13+
# Predicate whether currently running on R compiled with clang.
14+
is_using_clang <- function() {
15+
grepl("clang", R_compiled_by()["C"])
16+
}
17+
18+
# A `data.frame` giving default clang versions for each OS version of the
19+
# Fedora Linux distribution.
20+
# Source: https://packages.fedoraproject.org/pkgs/clang/clang/
21+
# See Updates section for older Fedora versions.
22+
fedora_clang_defaults <- data.frame(
23+
os = as.integer(c(36, 37, 38, 39, 40)),
24+
clang = as.integer(c(14, 15, 16, 17, 17))
25+
)
26+
27+
# A `data.frame` giving default clang versions for each OS version of the
28+
# Debian Linux distribution.
29+
# Source: https://packages.debian.org/search?keywords=clang
30+
debian_clang_defaults <- data.frame(
31+
os = c("bullseye", "bookworm", "trixie"),
32+
clang = as.integer(c(11, 14, 16))
33+
)
34+
35+
# Parse the major clang version as integer (e.g. 17) from
36+
# the full clang string (e.g. "Debian clang version 17.0.6 (3)")
37+
parse_clang_major <- function(clang_string) {
38+
assert_string(clang_string, pattern = "clang")
39+
clang_version <- gsub(pattern = "[^0-9.]", replacement = "", x = clang_string)
40+
as.integer(gsub(pattern = "([0-9]+).+", replacement = "\\1", x = clang_version))
41+
}
42+
43+
# Predicate whether a non-standard clang version is used, specifically
44+
# a higher than default clang version. Assumes that clang is used, otherwise fails.
45+
# If not Fedora or Debian of the known versions are used, always returns `FALSE`.
46+
is_non_standard_clang <- function(os_string,
47+
clang_major_version) {
48+
assert_string(os_string)
49+
assert_int(clang_major_version)
50+
if (grepl("Fedora", os_string)) {
51+
os_version <- as.integer(gsub(pattern = "[^0-9]", replacement = "", x = os_string))
52+
assert_int(os_version)
53+
which_os <- match(os_version, fedora_clang_defaults$os)
54+
if (is.na(which_os)) {
55+
return(FALSE)
56+
}
57+
clang_major_version > fedora_clang_defaults$clang[which_os]
58+
} else if (grepl("Debian", os_string)) {
59+
os_codename <- gsub(pattern = "Debian GNU/Linux ([a-z]+)/*[a-z]*", replacement = "\\1", x = os_string)
60+
assert_string(os_codename)
61+
which_os <- match(os_codename, debian_clang_defaults$os)
62+
if (is.na(which_os)) {
63+
return(FALSE)
64+
}
65+
clang_major_version > debian_clang_defaults$clang[which_os]
66+
} else {
67+
FALSE
68+
}
69+
}
70+
71+
# Predicate whether an R-devel version is running on Linux Fedora or
72+
# Debian with a non-standard clang compiler.
73+
is_r_devel_linux_clang <- function() {
74+
is_r_devel() &&
75+
is_linux() &&
76+
is_using_clang() &&
77+
is_non_standard_clang(
78+
os_string = utils::osVersion,
79+
clang_major_version = parse_clang_major(R_compiled_by()["C"])
80+
)
81+
}

mmrm.Rproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,5 +19,5 @@ BuildType: Package
1919
PackageUseDevtools: Yes
2020
PackageInstallArgs: --no-multiarch --with-keep.source
2121
PackageBuildArgs: --no-build-vignettes
22-
PackageCheckArgs: --as-cran --no-vignettes
22+
PackageCheckArgs: --no-vignettes
2323
PackageRoxygenize: rd,collate,namespace

tests/testthat/helper-skipping.R

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# Custom skipping function, specializing testthat::skip.
2+
skip_if_r_devel_linux_clang <- function() {
3+
do_skip <- is_r_devel_linux_clang()
4+
if (do_skip) {
5+
skip("On R-devel Linux system with non-standard clang")
6+
} else {
7+
invisible()
8+
}
9+
}

tests/testthat/test-between-within.R

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ test_that("h_df_1d_bw works as expected for singular fits", {
121121
# h_df_md_bw ----
122122

123123
test_that("h_df_md_bw works as expected - between effect", {
124+
skip_if_r_devel_linux_clang()
124125
object <- get_mmrm()
125126
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
126127
contrast[1, 2] <- contrast[2, 3] <- 1
@@ -133,6 +134,7 @@ test_that("h_df_md_bw works as expected - between effect", {
133134
})
134135

135136
test_that("h_df_md_bw works as expected - within effect", {
137+
skip_if_r_devel_linux_clang()
136138
object <- get_mmrm()
137139
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
138140
contrast[1, 6] <- contrast[2, 7] <- 1
@@ -145,6 +147,7 @@ test_that("h_df_md_bw works as expected - within effect", {
145147
})
146148

147149
test_that("h_df_md_bw works as expected - both effects", {
150+
skip_if_r_devel_linux_clang()
148151
object <- get_mmrm()
149152
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
150153
contrast[1, 2] <- contrast[2, 3] <- contrast[1, 6] <- contrast[2, 7] <- 1
@@ -157,6 +160,7 @@ test_that("h_df_md_bw works as expected - both effects", {
157160
})
158161

159162
test_that("h_df_md_bw works as expected for rank deficient model", {
163+
skip_if_r_devel_linux_clang()
160164
object <- get_mmrm_rank_deficient()
161165
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
162166
contrast[1, 2] <- contrast[2, 3] <- 1

tests/testthat/test-emmeans.R

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ test_that("emm_basis method works also for rank deficient fit", {
8282
# emmeans ----
8383

8484
test_that("emmeans works as expected", {
85+
skip_if_r_devel_linux_clang()
8586
skip_if_not_installed("emmeans", minimum_version = "1.6")
8687

8788
fit <- get_mmrm()
@@ -115,6 +116,7 @@ test_that("emmeans works as expected for transformed variables and fixed effect
115116
})
116117

117118
test_that("emmeans gives values close to what is expected", {
119+
skip_if_r_devel_linux_clang()
118120
skip_if_not_installed("emmeans", minimum_version = "1.6")
119121

120122
fit <- get_mmrm()
@@ -136,6 +138,7 @@ test_that("emmeans gives values close to what is expected", {
136138
})
137139

138140
test_that("emmeans works as expected also for rank deficient fit when singular coefficients are not involved", {
141+
skip_if_r_devel_linux_clang()
139142
skip_if_not_installed("emmeans", minimum_version = "1.6")
140143

141144
fit <- get_mmrm_rank_deficient()

tests/testthat/test-fit.R

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -203,27 +203,19 @@ test_that("h_summarize_all_fits works as expected", {
203203
all_fits <- list(mod_fit, mod_fit2)
204204
result <- expect_silent(h_summarize_all_fits(all_fits))
205205

206-
# NOTE:
207-
# Test currently fails on R-devel on Fedora with clang for the first optimizer
208-
# mmrm v0.1.5 @ r-devel-linux-x86_64-fedora-clang
209-
# `actual$log_liks`: -2155.280410081641 -1693.224938122514
210-
# `expected$log_liks`: -1693.224935585730 -1693.224938122510
211-
#
212-
# As this failure only appears on R-devel on Fedora with clang, we are
213-
# temporarily skipping this test on CRAN until we can reproduce the behavior.
214-
skip_on_cran()
206+
skip_if_r_devel_linux_clang()
215207

216208
expected <- list(
217209
warnings = list(NULL, NULL),
218210
messages = list(NULL, NULL),
219211
log_liks = c(-1693.22493558573, -1693.22493812251),
220212
converged = c(TRUE, TRUE)
221213
)
222-
223214
expect_equal(result, expected)
224215
})
225216

226217
test_that("h_summarize_all_fits works when some list elements are try-error objects", {
218+
skip_if_r_devel_linux_clang()
227219
mod_fit <- get_mmrm()
228220
mod_fit2 <- try(stop("bla"), silent = TRUE)
229221
mod_fit3 <- get_mmrm()

tests/testthat/test-satterthwaite.R

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ test_that("h_df_md_from_1d works as expected", {
134134
# h_df_md_sat ----
135135

136136
test_that("h_df_md_sat works as expected", {
137+
skip_if_r_devel_linux_clang()
137138
object <- get_mmrm()
138139
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
139140
contrast[1, 2] <- contrast[2, 3] <- 1
@@ -156,11 +157,12 @@ test_that("h_df_md_sat works as expected with a non-full rank contrast matrix",
156157
})
157158

158159
test_that("h_df_md_sat works as expected for rank deficient model", {
160+
skip_if_r_devel_linux_clang()
159161
object <- get_mmrm_rank_deficient()
160162
contrast <- matrix(data = 0, nrow = 2, ncol = length(component(object, "beta_est")))
161163
contrast[1, 2] <- contrast[2, 3] <- 1
162164
result <- expect_silent(h_df_md_sat(object, contrast))
163165
object2 <- get_mmrm()
164166
expected <- expect_silent(h_df_md_sat(object2, contrast))
165-
expect_identical(result, expected)
167+
expect_equal(result, expected, tolerance = 1e-4)
166168
})

tests/testthat/test-skipping.R

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# is_r_devel ----
2+
3+
test_that("is_r_devel works as expected", {
4+
expect_flag(is_r_devel())
5+
})
6+
7+
# is_linux ----
8+
9+
test_that("is_linux works as expected", {
10+
expect_flag(is_linux())
11+
})
12+
13+
# is_using_clang ----
14+
15+
test_that("is_using_clang works as expected", {
16+
expect_flag(is_using_clang())
17+
})
18+
19+
# parse_clang_major ----
20+
21+
test_that("parse_clang_major works as expected", {
22+
result <- parse_clang_major("Debian clang version 17.0.6 (3)")
23+
expected <- 17L
24+
expect_identical(result, expected)
25+
})
26+
27+
# is_non_standard_clang ----
28+
29+
test_that("is_non_standard_clang works as expected for Fedora", {
30+
os_string <- "Fedora Linux 36 (Workstation Edition)"
31+
expect_false(is_non_standard_clang(os_string, clang_major_version = 14L))
32+
expect_false(is_non_standard_clang(os_string, clang_major_version = 13L))
33+
expect_true(is_non_standard_clang(os_string, clang_major_version = 15L))
34+
})
35+
36+
test_that("is_non_standard_clang returns FALSE for non-listed Fedora versions", {
37+
os_string <- "Fedora Linux 12"
38+
expect_false(is_non_standard_clang(os_string, clang_major_version = 14L))
39+
expect_false(is_non_standard_clang(os_string, clang_major_version = 13L))
40+
expect_false(is_non_standard_clang(os_string, clang_major_version = 15L))
41+
})
42+
43+
test_that("is_non_standard_clang works as expected for Debian", {
44+
os_string <- "Debian GNU/Linux trixie/sid"
45+
expect_false(is_non_standard_clang(os_string, clang_major_version = 16L))
46+
expect_false(is_non_standard_clang(os_string, clang_major_version = 15L))
47+
expect_true(is_non_standard_clang(os_string, clang_major_version = 17L))
48+
})
49+
50+
test_that("is_non_standard_clang returns FALSE for non-listed Debian versions", {
51+
os_string <- "Debian GNU/Linux bla"
52+
expect_false(is_non_standard_clang(os_string, clang_major_version = 14L))
53+
expect_false(is_non_standard_clang(os_string, clang_major_version = 13L))
54+
expect_false(is_non_standard_clang(os_string, clang_major_version = 15L))
55+
})
56+
57+
test_that("is_non_standard_clang returns FALSE for other Linux distributions", {
58+
os_string <- "Solaris"
59+
expect_false(is_non_standard_clang(os_string, clang_major_version = 14L))
60+
expect_false(is_non_standard_clang(os_string, clang_major_version = 13L))
61+
expect_false(is_non_standard_clang(os_string, clang_major_version = 15L))
62+
})
63+
64+
# is_r_devel_linux_clang ----
65+
66+
test_that("is_r_devel_linux_clang works as expected", {
67+
expect_flag(is_r_devel_linux_clang())
68+
})

0 commit comments

Comments
 (0)