-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Optimize/revisit determine_setup #4367
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
Conversation
@@ -121,28 +121,34 @@ def determine_setup(inifile, args, rootdir_cmd_arg=None, config=None): | |||
break | |||
except KeyError: | |||
inicfg = None | |||
rootdir = get_common_ancestor(dirs) | |||
if rootdir_cmd_arg is None: |
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.
Note: only the "is None" branch appears to be covered here: https://codecov.io/gh/pytest-dev/pytest/commit/247bdb2c0f5ba64161de7f24e958919dfe0e02ea#D1-124
157f875
to
ccd5e16
Compare
Build failure is unrelated (test_fail_and_continue_with_stepwise), #4366. |
This allows to make use of it when determining the rootdir etc.
ccd5e16
to
ba457f5
Compare
Codecov Report
@@ Coverage Diff @@
## features #4367 +/- ##
============================================
- Coverage 95.85% 95.79% -0.06%
============================================
Files 111 111
Lines 24950 25233 +283
Branches 2439 2486 +47
============================================
+ Hits 23915 24172 +257
- Misses 737 765 +28
+ Partials 298 296 -2
Continue to review full report at Codecov.
|
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.
at initial skimming this looks correct and the test results indicate it is
good job at aking that one a bit quicker - could you post the difference it makes before we merge
This was mostly for readability / saneness IIRC.
Well, it is not 100% covered and we had things already where tests were passing, but they broke things then. But since this is for features and relatively safe, I'm going to merge this already. |
Taken out of #4337.