Skip to content

Commit 1e38fc2

Browse files
RFC: fail test.data.table() if non-test code produces warnings (#7210)
* test.data.table: catch warnings outside test code * Use explicit environment assignment instead of <<- * Thinko: length -> nrow * tests 2253*: suppressWarnings, skip if no UTF-8 * tests: remember tracebacks from warnings * Thinko: length -> nrow * suppressWarnings(Sys.setlocale(...)) On modern versions of R for Windows, Sys.setlocale() warns for non-UTF-8 locales. Also comment the previous instance of suppressWarnings(). * Apply fixes from code review * conditionMessage() instead of toString() * apply names earlier * don't check for zero-length calls in sys.calls() * be careful not to over-translate an already-translated message Co-Authored-By: Michael Chirico <[email protected]> --------- Co-authored-by: Michael Chirico <[email protected]>
1 parent cfa9f49 commit 1e38fc2

File tree

2 files changed

+52
-5
lines changed

2 files changed

+52
-5
lines changed

R/test.data.table.R

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,30 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=F
205205
}
206206
# nocov end
207207

208-
err = try(sys.source(fn, envir=env), silent=silent)
208+
env$warnings = list()
209+
err = try(
210+
withCallingHandlers(
211+
sys.source(fn, envir=env),
212+
warning=function(w) {
213+
# nocov start
214+
if (!silent && showProgress) print(w)
215+
env$warnings = c(env$warnings, list(list(
216+
"after test"=env$prevtest, warning=conditionMessage(w),
217+
calls=paste(
218+
vapply_1c(sys.calls(), function(call) {
219+
if (is.name(call[[1]])) {
220+
as.character(call[[1]])
221+
} else "..."
222+
}),
223+
collapse=" -> "
224+
)
225+
)))
226+
invokeRestart("muffleWarning")
227+
# nocov end
228+
}
229+
),
230+
silent=silent
231+
)
209232

210233
options(oldOptions)
211234
for (i in oldEnv) {
@@ -262,6 +285,21 @@ test.data.table = function(script="tests.Rraw", verbose=FALSE, pkg=".", silent=F
262285
# important to stopf() here, so that 'R CMD check' fails
263286
# nocov end
264287
}
288+
if (length(env$warnings)) {
289+
# nocov start
290+
warnings = rbindlist(env$warnings)
291+
catf(
292+
ngettext(nrow(warnings),
293+
"Caught %d warning outside the test() calls:\n",
294+
"Caught %d warnings outside the test() calls:\n"
295+
),
296+
nrow(warnings),
297+
domain=NA
298+
)
299+
print(warnings, nrows = nrow(warnings))
300+
stopf("Tests succeeded, but non-test code caused warnings. Search %s for tests shown above.", names(fn))
301+
# nocov end
302+
}
265303

266304
# There aren't any errors, so we can use up 11 lines for the timings table
267305
time = nTest = RSS = NULL # to avoid 'no visible binding' note

inst/tests/tests.Rraw

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16996,7 +16996,8 @@ if (.Platform$OS.type=="windows") local({
1699616996
LC_TIME = "Chinese (Simplified)_China.936"
1699716997
)
1699816998
x_old = Map(Sys.getlocale, names(x))
16999-
invisible(Map(Sys.setlocale, names(x), x))
16999+
# setlocale() warns for non-UTF-8 locales in modern versions of R, #7210
17000+
suppressWarnings(invisible(Map(Sys.setlocale, names(x), x)))
1700017001
on.exit(Map(Sys.setlocale, names(x_old), x_old))
1700117002
# triggered segfault here in #4402, Windows-only under translation.
1700217003
# test that the argument order changes correctly (the 'item 2' moves to the beginning of the message)
@@ -18615,12 +18616,18 @@ test(2252.2, dt[, let(b=2L)], error = "\\[ was called on a data.table.*not data.
1861518616
rm(.datatable.aware)
1861618617

1861718618
# tests for trunc.char handling wide characters # 5096
18618-
local({
18619+
local(if (l10n_info()$`UTF-8` || {
1861918620
lc_ctype = Sys.getlocale('LC_CTYPE')
18621+
lc_wantctype = 'en_US.UTF-8'
1862018622
# Japanese multibyte characters require utf8. As of 2025, we're likely to be already running in a UTF-8 locale, but if not, try this setlocale() call as a last chance.
1862118623
# Unfortunately, there is no guaranteed, portable way of switching to UTF-8 US English.
18622-
if (!l10n_info()$`UTF-8`) Sys.setlocale('LC_CTYPE', "en_US.UTF-8")
18623-
on.exit(Sys.setlocale('LC_CTYPE', lc_ctype))
18624+
# Avoid the warning upon possible failure, #7210.
18625+
lc_newctype = suppressWarnings(Sys.setlocale('LC_CTYPE', lc_wantctype))
18626+
if (identical(lc_newctype, lc_wantctype)) {
18627+
on.exit(Sys.setlocale('LC_CTYPE', lc_ctype))
18628+
TRUE
18629+
} else FALSE
18630+
}) {
1862418631
accented_a = "\u0061\u0301"
1862518632
ja_ichi = "\u4E00"
1862618633
ja_ni = "\u4E8C"
@@ -18670,6 +18677,8 @@ local({
1867018677
paste0(c(ja_ko, ja_n, accented_a), dots, collapse=" ")))
1867118678
# test for data.table with NA, #6441
1867218679
test(2253.20, options=list(datatable.prettyprint.char = 1L), data.table(a = c("abc", NA)), output=" a\n1: a...\n2: <NA>")
18680+
} else {
18681+
cat("Tests 2253* skipped because they need a UTF-8 locale.\n")
1867318682
})
1867418683

1867518684
# allow 1-D matrix in j for consistency, #783

0 commit comments

Comments
 (0)