-
Notifications
You must be signed in to change notification settings - Fork 187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check if parallel testing reduces check time in CI #2297
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
On * checking tests ...
Running ‘testthat.R’ [67s/69s]
[68s/69s] OK In this PR for the same config: * checking tests ...
Running ‘testthat.R’ [85s/46s]
[85s/46s] OK I don't understand why there are two timings, and which one we should care about. The |
Part of the issue is that I think there's a maximum number of jobs that can be running at once -- speeding up individual jobs helps some, but I'd really like to reduce the number of jobs if some jobs are redundant. |
Agreed. But what do you think about turning on the parallel testing for |
if we could have it run in serial by default and parallel according to environment variable on GHA only (not CRAN) I think that's preferable. parallelism is often a trojan horse for headaches if we don't get a huge benefit |
Will setting something like this in if (Sys.getenv("NOT_CRAN", "true") == "true") {
Sys.setenv("TESTTHAT_PARALLEL" = "FALSE")
} |
Filed an issue upstream about the error we see in the vigilant workflow: r-lib/testthat#1912 |
I suspect the first is CPU time and the second is elapsed (wall) time. |
The reduction in test time from parallel testing has reduced significantly since the last time we checked; probably because our test suite itself has somehow got faster 👀
* checking tests ...
Running ‘testthat.R’ [28s/29s]
[29s/29s] OK
* checking tests ...
Running ‘testthat.R’ [49s/24s]
[49s/24s] OK I am no longer sure if it is still worth it to turn on parallel testing, or if we'd wait for our test suite to get even bigger before we could reap the benefits of parallel testing. |
@MichaelChirico, @AshesITR WDYT? Should we keep this PR open for a few more months or a year, rebase once in a while, and check for time saving benefits? |
I'm fine with either way. It looks like most of the work is done already and rebasing from time to time isn't difficult. What do you think is best? |
I think it should be fine (and costless) to keep this pending for a while. |
Related to #2295