-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Improve performance of ~3 of the slowest tests #4090
Conversation
I don't recall why, and looking at the test itself no clear reason jumps to me... if things are passing, I say let's keep your changes. 😁 |
testing/code/test_excinfo.py
Outdated
@@ -1361,40 +1371,30 @@ def a(x): | |||
def b(x): | |||
return a(numpy_like()) | |||
|
|||
try: | |||
with pytest.raises(RecursionError) as excinfo: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RecursionError
is py35+ (or py36+) only; IIRC you need to use RuntimeError
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dang, I always forget this 😭
Codecov Report
@@ Coverage Diff @@
## master #4090 +/- ##
=========================================
- Coverage 94.47% 94.4% -0.08%
=========================================
Files 109 109
Lines 23813 23811 -2
Branches 2349 2349
=========================================
- Hits 22498 22478 -20
- Misses 1007 1021 +14
- Partials 308 312 +4
Continue to review full report at Codecov.
|
Noice! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fabulous enhancements 👍
cherry picked a few from
pytest testing/{subdir} --durations=5
and wanted to see what I could do to make them faster.this saves ~11 seconds off of a test run (which isn't much but it's something!)
interesting to note that
runpytest_subprocess
adds about 500ms per invocation. (I guess we should avoid it if possible? @nicoddemus do you know if there's a reason why it was used for #3124?)