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

py36+ tests are definition ordered #7848

Closed
wants to merge 1 commit into from
Closed

py36+ tests are definition ordered #7848

wants to merge 1 commit into from

Conversation

asottile
Copy link
Member

@asottile asottile commented Oct 3, 2020

Resolves #5196
See #7808

@asottile
Copy link
Member Author

asottile commented Oct 3, 2020

an interesting side-effect of this is base classes appear after the subclass which is perhaps a little strange -- I can see if I can adjust that behaviour if it's undesirable (I would expect pytest to run base-class tests first, though it makes sense from an mro() perspective the way it is currently implemented) 🤔

@bluetech
Copy link
Member

bluetech commented Oct 3, 2020

So IIUC the previous behavior was to execute tests in the order in which they physically appear (the file path and the line number within the file). So tests from base classes would execute before or after depending on where the base class is in the file (nice IMO) or how its filename sorts (doesn't make sense IMO).

Certainly definition order makes more sense semantically, but I do indeed think that tests from base classes should run first, that's how I expect it anyway. I was going to say "just traverse the MRO in reverse" but since the code pokes at __dict__s directly that would be tricky.

@asottile
Copy link
Member Author

asottile commented Oct 3, 2020

I don't think the mro thing would be that complicated -- I'll see what I can do (I agree the base classes should run first)

Base automatically changed from master to main March 9, 2021 20:40
@bluetech
Copy link
Member

Looking at collection timings for pandas (182330 tests), this sort call features prominently. Particularly the slowdown comes from sort -> sort_key -> PyObjMixin.reportinfo() -> getfslineno(obj) -> findsource(obj) (only for objs without __code__, mostly test classes) -> inspect.findsource(obj). And apparently since Python 3.9, inspect.findsource() uses ast instead of regexes to find the line number of classes, which I suspect makes it even slower (didn't verify this).

Anyway, if we can get this PR to work it will be nice (maybe I'll try it myself).

@asottile
Copy link
Member Author

ah yeah I haven't had time to come back to this -- I think all that needs to happen is to consider the mro when ordering tests -- not sure when I'll have some time to look at it so feel free to carry the patch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use simple definition order in Python>3.6
2 participants