Skip to content

Commit e5cd770

Browse files
authored
✨ Fix JunitReporter for warnings outside tests ✨ (#2158)
Unprompted, claude also added code to capture warning backtraces, which seems useful. Fixes #1913
1 parent a9aecb8 commit e5cd770

File tree

6 files changed

+41
-11
lines changed

6 files changed

+41
-11
lines changed

.claude/settings.local.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
{
22
"permissions": {
33
"allow": [
4-
"Bash(find:*)"
4+
"Bash(find:*)",
5+
"Bash(R:*)"
56
],
67
"deny": []
7-
}
8+
},
9+
"$schema": "https://json.schemastore.org/claude-code-settings.json"
810
}

CLAUDE.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@ testthat is R's most popular unit testing framework, used by thousands of CRAN p
88

99
## Key Development Commands
1010

11-
### Testing
12-
- `devtools::test()` or `Ctrl/Cmd+Shift+T` in RStudio - Run all tests
13-
- `devtools::test_file("tests/testthat/test-filename.R")` - Run tests in a specific file
14-
- `testthat::test_local()` - Run tests for local source package
15-
- `testthat::test_package("testthat")` - Run tests for installed package
16-
- `R CMD check` - Full package check including tests
11+
General advice:
12+
* When running R from the console, always run it with `--quiet --vanilla`
13+
* Always run `air format .` after generating code
14+
15+
### Development tools
1716

18-
### Building and Installation
19-
- `devtools::load_all()` or `Ctrl/Cmd+Shift+L` - Load package for development
17+
- `devtools::test()` - Run all tests
18+
- `devtools::test_file("tests/testthat/test-filename.R")` - Run tests in a specific file
19+
- `devtools::load_all()` - Load package for development
2020
- `devtools::document()` - Generate documentation
2121
- `devtools::check()` - Run R CMD check
2222
- `devtools::install()` - Install package locally

NEWS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# testthat (development version)
22

3+
* `JunitReporter()` no longer fails with `"no applicable method for xml_add_child"` for warnings outside of tests (#1913). Additionally, warnings now save their backtraces.
34
* `JunitReporter()` strips ANSI escapes in more placese (#1852, #2032).
45
* `try_again()` is now publicised. The first argument is now the number of retries, not tries (#2050).
56
* `vignette("custom-expectations)` has been overhauled to make it much clearer how to create high-quality expectations (#2113, #2132, #2072).

R/reporter-junit.R

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,11 @@ JunitReporter <- R6::R6Class(
108108
time <- self$elapsed_time()
109109
self$suite_time <- self$suite_time + time
110110

111+
# If no context was started (e.g., warnings outside tests), create a default one
112+
if (is.null(self$suite)) {
113+
self$start_context(context %||% "(unknown)")
114+
}
115+
111116
# XML node for test case
112117
name <- test %||% "(unnamed)"
113118
testcase <- xml2::xml_add_child(
@@ -147,6 +152,9 @@ JunitReporter <- R6::R6Class(
147152
} else if (expectation_skip(result)) {
148153
xml2::xml_add_child(testcase, "skipped", message = first_line(result))
149154
self$skipped <- self$skipped + 1
155+
} else if (expectation_warning(result)) {
156+
warning_node <- xml2::xml_add_child(testcase, "system-out")
157+
xml2::xml_text(warning_node) <- cli::ansi_strip(format(result))
150158
}
151159
},
152160

tests/testthat/_snaps/reporter-junit.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,12 @@
4545
</testcase>
4646
</testsuite>
4747
<testsuite name="Warnings" timestamp="1999:12:31 23:59:59" hostname="nodename" tests="2" skipped="1" failures="0" errors="0" time="0">
48-
<testcase time="0" classname="Warnings" name="warnings_get_backtraces"/>
48+
<testcase time="0" classname="Warnings" name="warnings_get_backtraces">
49+
<system-out>def
50+
Backtrace:
51+
x
52+
1. \-f()</system-out>
53+
</testcase>
4954
<testcase time="0" classname="Warnings" name="warnings_get_backtraces">
5055
<skipped message="Reason: empty test ('reporters/tests.R:44:1')"/>
5156
</testcase>

tests/testthat/test-reporter-junit.R

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,3 +27,17 @@ test_that("ANSI escapes are stripped from all user text in XML", {
2727

2828
expect_no_error(xml2::read_xml(tmp))
2929
})
30+
31+
test_that("warnings outside context don't cause xml_add_child errors", {
32+
skip_if_not_installed("xml2")
33+
34+
tmp <- withr::local_tempfile(fileext = ".xml")
35+
reporter <- JunitReporterMock$new(file = tmp)
36+
reporter$start_reporter()
37+
38+
# This would previously fail with "no applicable method for 'xml_add_child'"
39+
expect_no_error({
40+
reporter$add_result(NULL, "test", new_expectation("warning", "test"))
41+
})
42+
reporter$end_reporter()
43+
})

0 commit comments

Comments
 (0)