Repository navigation
Mark slow test methods with @requires_resource('cpu') #108416
Description
Activity
No, they are different. The purpose is different, this issue is about speeding up manual testing, although it can help in CI testing too. I only mark the slowest methods, allowing fast tests to run. #108388 is much larger and more complex issue, this issue only intersects with a small part of it..
This doesn't feel right. Tagging these as 'cpu' is usually untrue. Many of these tests are not resource intensive for anything but 'wall clock time'.
Critically, it would means we have no easy reliable way of running this vast swath of our testsuite across multiple platforms when doing development if our Github CI does not use whatever resource is being applied so that the tests become skipped by default. If a dev has to do something special to identify and make tests potentially relevant change run across all platforms, they simply not going to get that determination right or remember a large portion of the time and the tests will not be run until after merge when it is too late. Allowing regressions to be merged is a net negative.
Tangentially related: We should probably have all github actions CI configurations in release branches set to use
-uallno matter what.Reacted by Erlend E. AaslandI think we want a new resource class for this purpose when latency is the main reason it is being disabled rather than actual CPU consumption. I suggest perhaps calling it
'walltime'?We could make a new
-ucategory setting for github CI use called-ufastand configure specific github actions we want to respond faster to specify that. We could then control which other settings are turned on or off via that within regrtest itself. Leaving walltime enabled by default so that devs run those tests themselves by default. It'd be good to have at least one non-required GH actions CI check not using the "fast" run, or even doing a "-uall,-network,-largefile" run, as that still provides visibility on the PR.Even better if it is possible for the github merge queue automerge to wait for that not otherwise required CI check even though the merge button blocker does not? I don't know if github allows that level of config... @ambv ?
(brainstorming)
Reacted by Erlend E. AaslandRe-did using
time.process_time()instead oftime.perf_counter(), and then usingsum(os.times()[:4]), because some tests run CPU-heavy subprocesses. Now it includes about 46 tests.I prepared also a separate patch for marking long running tests which do not spend much CPU time, but simply sleep, with a new resource "walltime".
- added a commit that references this issue
on Sep 2, 2023 - added a commit that references this issue
on Sep 3, 2023 See also #108828 which will help to find all tests which can be skipped for particular reason.
- added a commit that references this issue
on Sep 5, 2023
Has this already been discussed elsewhere?
This is a minor feature, which does not need previous discussion elsewhere
Links to previous discussion of this feature:
No response
Proposal:
The proposed PR marks all test methods which have duration longer than 3 seconds with
@test.support.requires_resource('cpu')decorator.The purpose is to reduce manual testing time. It happens that all tests in a file are ran in fraction of a second, but few tests take a long time to run. When you work on some large feature or bugfix you need to run corresponding tests multiple times. You can exclude the slowest tests manually, but you should know what of them are culprits. When they are marked as CPU-hungry, you can just not enable the "cpu" resource.
For example, all
test_mathtakes over 1.5 minutes to finish. But when excludetest_sumprod_stress, it takes only 3 seconds.Linked PRs