test: improve robustness of test_shielded_cancel_sleep_time - #1264
Conversation
49775e8 to
0683119
Compare
agronholm
left a comment
There was a problem hiding this comment.
Change the timings if you like, but adding constants for this where they're only used in one place is just pointless churn. Drop the changelog entry, as this is an internal change. The comment also adds nothing useful, so drop that too.
0683119 to
821f4bd
Compare
I would say just opposite, that hard coding random numbers into your code is crazy for the future maintenance, but as you wish. |
The `test_shielded_cancel_sleep_time` test was failing on certain architectures (notably ppc64le in the OBS server) because the CPU time consumed during the test window occasionally exceeded the wall-clock duration, leading to false-positive assertions of "CPU spinning". The test is designed to ensure that cancelling a shielded task does not cause the event loop to enter a tight loop (spinning), consuming 100% CPU. However, on high-overhead systems, the combination of context switching and event loop management can push CPU usage slightly above the 1:1 ratio of wall-clock time. Fixes: agronholm#1263
821f4bd to
a1d4d4a
Compare
|
Adding constants can be useful if the values are used in more than one location and need to be changed in lock-step. Otherwise it's just extra fluff, especially in test code. |
|
Your force-pushing (don't do that please) didn't remove the single-use constant. |
Changes
Fixes: #1263
The
test_shielded_cancel_sleep_timetest was failing on certain architectures (notably ppc64le in OBS) because the CPU time consumed during the test window occasionally exceeded the wall-clock duration, leading to false-positive assertions of "CPU spinning".The test is designed to ensure that cancelling a shielded task does not cause the event loop to enter a tight loop (spinning), consuming 100% CPU. However, on high-overhead systems, the combination of context switching and event loop management can push CPU usage slightly above the 1:1 ratio of wall-clock time.
Checklist
If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):
tests/) which would fail without your patch NOT APPLICABLEdocs/), in case of behavior changes or newfeatures NOT APPLICABLE
docs/versionhistory.rst).If this is a trivial change, like a typo fix or a code reformatting, then you can ignore
these instructions.
Updating the changelog
If there are no entries after the last release, use
**UNRELEASED**as the version.If, say, your patch fixes issue #123, the entry should look like this:
If there's no issue linked, just link to your pull request instead by updating the
changelog after you've created the PR.