gh-108416: Mark slow test methods with @requires_resource('cpu') #108421
Conversation
serhiy-storchaka
commented
Aug 24, 2023
•
serhiy-storchaka
commented
- Issue: Mark slow test methods with @requires_resource('cpu') #108416
serhiy-storchaka
added
tests
serhiy-storchaka
requested review from
rhettinger,
gpshead,
a team,
vsajip and
iritkatriel
as code owners
serhiy-storchaka
removed request for
a team,
gpshead,
iritkatriel,
rhettinger and
vsajip
|
How did you decide if a function is "slow" or not? |
|
I ran all the tests and then sorted them by execution time. |
|
Results from macOS 13.5 on M1 Max: from 5 minutes 11 seconds down to 2 minutes 25 seconds. I would prefer if there was some indication in the summary of how many tests were skipped due to not enabling various resources. We can add this in a separate PR but I consider this important. Right now it's easy to miss the fact that you ran fewer tests. It says "428 tests OK" in both cases. -m test -j12 -ucpu (with slow tests)-m test -j12 -u-cpu (without slow tests) |
|
see the issue, I don't like doing this. A goal of avoiding running regression tests is not the right goal. |
Is it the time of running all tests? It is impressive, I did not expect such difference. This PR adds
I'll do this. Currently the summary only shows if the test file was skipped completely, but not if a TestCase class or separate method was skipped. Although I afraid that the output can be very verbose, because normally many tests are skipped for different reasons. But it would be useful to have such option. |
|
Sync and merge past the parallelization refactorings that Victor merged before measuring again. I don't think we should be abusing the 'cpu' resource for this. Many of these are not CPU hogs so that becomes misleading. Some are important to have run so that we don't break everything. (the AST test that parses the stdlib for example) Should I go through all 86 tests and give a thumbs up / thumbs down to them being annotated? I'm against merging this as it is... but I think we can get something in sensible shape. (deeper thoughts recorded on the issue) |
|
Some tests ( |
Please log a message in verbose mode, to help debugging. It's surprising when a test pass and then fail on the same machine. |
They print full lists of tested files in verbose mode. |
It would help. There are now half as many of them. I have reviewed each case several times and can justify them. But there is always the possibility of missing something and my criteria could be wrong. |
|
|
It's a test_math test. |
|
I'm a bit worried that you might forget to un-skip tests in the future, like So I wrote PR #108793 to write a more detailed report on tests statistics. To remind that some tests are skipped (for good or bad reasons). |
|
These tests are not skipped. I almost always run tests with On other hand, bigmem tests are disabled by default, and run very infrequently. Some of them require over 200GB of memory, so they are never run. |
|
Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12. |
pythonGH-108421) Only mark tests which spend significant system or user time, by itself or in subprocesses. (cherry picked from commit f3ba0a7) Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-108798 is a backport of this pull request to the 3.12 branch. |
|
Sorry, @serhiy-storchaka, I could not cleanly backport this to |
…e('cpu') (pythonGH-108421)
Only mark tests which spend significant system or user time,
by itself or in subprocesses..
(cherry picked from commit f3ba0a7)
Co-authored-by: Serhiy Storchaka <[email protected]>
|
GH-108799 is a backport of this pull request to the 3.11 branch. |
|
