Chore/add tests - #120
Conversation
wvjgsuhp
left a comment
wvjgsuhp
left a comment
There was a problem hiding this comment.
I didn't check data-raw machinery. Felt bad typing this, but going ahead anyway.
test-speed.R growing large started to itch me, so ty for the scratch.
| # Direct callers bypass `.verify_grid_factors()`, so fall back to the name | ||
| # patterns below rather than indexing a malformed `grid_factors`. | ||
| named_dims <- is.list(grid_factors) && | ||
| all(c("dim1", "dim2") %in% names(grid_factors)) && | ||
| all(lengths(grid_factors[c("dim1", "dim2")]) == 1) |
There was a problem hiding this comment.
This feels overkill unless we are expecting calls from outside speed.
There was a problem hiding this comment.
Nah that's fair. I think in a lot of these cases, if people play silly games, they win silly errors.
|
|
||
| # Checked for every call shape, since `infer_row_col()` below uses it whether | ||
| # or not `optimise` was supplied. | ||
| .verify_grid_factors(grid_factors) |
There was a problem hiding this comment.
Should be buried under the prior .verify...s.
| # `speed()` rejects a per-level `grid_factors`, so this arm is only | ||
| # reachable by calling `create_speed_input()` directly. |
There was a problem hiding this comment.
I think we should still keep grid_factors flexible between optimizing levels.
E.g., maybe there are experimental designs depending on row/col and time/height.
I can see that such designs gonna be very complex and rare tho.
There was a problem hiding this comment.
Should we do class(test_data) = c('design', class(test_data)) instead of calling speed?
Maybe keep only 1 test for speed call.
There was a problem hiding this comment.
Yeah I like this idea. The plotting machinery doesn't need the design to run (except for one case to make sure it works).
There was a problem hiding this comment.
Would devtools::build_vignettes() suffice so we don't have to explicitly test these?
2 participants
Main objective of this PR is:
Fixes #108. Should also fix #116.
Summary
Speeds up the test suite and
R CMD check, and closes the remaining coverage gaps.Vignettes - major change
.qmd.origsources, soR CMD build/checkonly render markdown rather than re-running every optimisation twice per platform. A scheduled workflow still executes the real sources across all five OS/R combinations.Tests
test-speed.Rinto five files to take advantage of parallelism intestthat.Other house keeping
grid_factorsvalidation — a malformed value previously failed with "missing value where TRUE/FALSE needed"..onAttachversion check behind an interactive check, aSPEED_NO_VERSION_CHECKenv var, and a 2s timeout.Locally: test suite 177s → 123s,
R CMD check922s → 221s, coverage 99.82% → 100%.