Allow running a subset of tests by pattern #6040
Conversation
|
@ben-schwen PTAL if this suits your needs |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6040 +/- ##
==========================================
+ Coverage 97.51% 97.53% +0.01%
==========================================
Files 80 80
Lines 14920 14916 -4
==========================================
- Hits 14549 14548 -1
+ Misses 371 368 -3 ☔ View full report in Codecov by Sentry. |
Basically yes. However, I just noticed that it has problems when the test uses |
Could you elaborate? I just tried |
I tried Running 3 of 6832 tests matching '2025'
Error in parse(n = -1, file = file, srcfile = NULL, keep.source = FALSE) :
184:1: unexpected '}'
183: test(2025.01, fread(testDir("issue_3400_fread.txt"), skip=1, header=TRUE), data.table(A=INT(1,3,4), B=INT(2,2,5), C=INT(3,1,6)))
184: }
^but maybe thats because these tests use the test_no + 0.1 instead of fixed numbers? |
|
Maybe another more simple approach: |
that was done in 0b683fb as elaborated in PR description. it doesn't work well with our test setup :\ |
the problem with 2025 is here: Some but not all of the suite have literal '2025' in the name, we wind up with that dangling We can fix this by making more changes in #6041, but the shortcomings of the static approach remain. |
MichaelChirico
force-pushed
the
test-pattern
branch
from
b3ea13a to
df1cf69
Compare
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @MichaelChirico and the rest of your teammates on |
MichaelChirico
force-pushed
the
test-pattern
branch
from
df1cf69 to
579593a
Compare
|
@ben-schwen restacked this onto #6041, with the latest change there The fundamental issue remains; as a workaround, I added a step to at least check the subsetted script parses, if not we back up to running the full suite instead of the cryptic error. |
Related to #6040 -- it will be good to keep to a pattern where `test()` always has a numeric literal in the `num=` argument, even if it's a dynamic test where the base number is incremented by a variable amount. Doing so will make the `testPattern=` argument to `test.data.table()` more usable. We can add a linter for this (#5908) to prevent regression later. <details> Linter to find these: ```r l=make_linter_from_xpath( "//SYMBOL_FUNCTION_CALL[text() = 'test']/parent::expr/following-sibling::expr[1][SYMBOL or expr[1]/SYMBOL]", "xxx") lint("inst/tests/tests.Rraw", l()) ``` </details>
|
This doesn't affect CI or |
2 participants
Closes #6039.
The simple version 0b683fb doesn't work right because of our non-hermetic tests. A version with just
try({x,y}, silent=TRUE)worked but spewed a ton of output. In any case, it still takes ~30 seconds to run the suite on my computer fortestPattern='1111'-- again because our tests are so non-hermetic and we wind up needing to evaluate thex&yparts for every test because of this.Why is this the case? think of tests like:
We need to evaluate
y<-2in order fortest(2)to pass, and moreover we needyforaoutside of tests so even early return oftest()doesn't help.