Skip to content
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

pytest_plugin: disable flood protection for example tests #2630

Merged
merged 1 commit into from
Oct 20, 2024

Conversation

dgw
Copy link
Member

@dgw dgw commented Oct 13, 2024

Description

This patch feels almost too simple to work, but it does.

For quite a while, we've had three tests from the rand plugin that take several seconds each. I had a Sopelunk with the Git version of pytest-profiling (because they haven't released in about 5 years and the last package is broken on modern Pythons) and discovered that about 98% of the time spent on the five tests selected by pytest -k rand.py was in time.sleep().

Drilling down, I saw that was being called from bot.say(), which led pretty quickly to the real culprit: Flood protection.

There's no server to flood during tests, so we can just turn that protection off. With this change, those five tests from rand went from ~12 seconds on my test system down to about 0.25 sec.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make lint and make test)
  • I have tested the functionality of the things this change touches

For quite a while, we've had three tests from the `rand` plugin that
take several seconds each. I had a Sopelunk with the Git version [1] of
`pytest-profiling` [2] and discovered that about 98% of the time spent
on the five tests selected by `pytest -k rand.py` was in `time.sleep()`.

Drilling down, I saw that was being called from `bot.say()`, which led
pretty quickly to the real culprit: Flood protection.

There's no server to flood during tests, so we can just turn that
protection off. With this change, those five tests from `rand` went from
about 12 seconds on my test system down to about 0.25 sec.

[1]: The last actual release of `pytest-profiling` is from 2019, and it
     doesn't work on modern Python. (I started analyzing this under
     Python 3.13, while verifying fixes for new warnings there.) All of
     the necessary updates already exist in the Git repo, though; they
     just haven't published a new release in five years.

[2]: https://github.com/man-group/pytest-plugins/tree/master/pytest-profiling
@dgw dgw added Build Bugfix Generally, PRs that reference (and fix) one or more issue(s) Housekeeping Code cleanup, removal of deprecated stuff, etc. labels Oct 13, 2024
@dgw dgw added this to the 8.0.1 milestone Oct 13, 2024
@dgw dgw requested a review from a team October 13, 2024 18:36
@dgw
Copy link
Member Author

dgw commented Oct 13, 2024

Super ironic that the CI run for this PR had some of the longest job times this repo's ever seen… 3/6 jobs went over 2m, where normally they are all done in about 90 seconds.

Still, I promise this made local tests on my machine a lot faster. GHA hosted runner performance might be too inconsistent to draw any conclusions from, especially based on only a single run.

Current master on my Z1 Extreme-based PC, on 15W mode, in WSL (not exactly ideal performance conditions):

==== 1390 passed, 8 xfailed, 1 warning in 50.19s ====

After git checkout -b test-2630 && git merge fix-slow-repeated-tests:

==== 1390 passed, 8 xfailed, 1 warning in 32.11s ====

Again, can't draw too much of a conclusion from single runs, but the difference is definitely noticeable.

@dgw dgw merged commit 7150e02 into master Oct 20, 2024
17 checks passed
@dgw dgw deleted the fix-slow-repeated-tests branch October 20, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s) Build Housekeeping Code cleanup, removal of deprecated stuff, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants