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

fix bug with nonskipped first test in package #5831

Merged
merged 3 commits into from Nov 6, 2019

Conversation

zefirior
Copy link

@zefirior zefirior commented Sep 8, 2019

Issue #5830
Fix fail skipping the first test in package marked as skip

@RonnyPfannschmidt
Copy link
Member

great job at sorting this one aout and creating a working first implementation

at first glance it would still create duplicate markers on the module

the problem here is that the package object really should be the parent of the tests collected by the module
so other tests would also inherit the packages mark

however as things are, we have a package and a module, and the module has the markers

so while the collection node tree structure is as it is now,
we need to introduce a _PackageModule that has ALLOW_OWN_MARKERS = False on top of your correction to ensure the module of __init__.py wont get duplicate marker objects

@zefirior
Copy link
Author

zefirior commented Sep 11, 2019

@RonnyPfannschmidt, If I understand correctly, you talked about the case when __init__.py is a test module. This module should not have its own markers, because the module inherits the markers from the parent package

@blueyed
Copy link
Contributor

blueyed commented Oct 20, 2019

@zefirior
Thanks for this. I've went ahead and rebased it to get CI results (pytest-CI was missing). Old HEAD: 6e5538b.

@RonnyPfannschmidt
Please see the comment above.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @zefirior!

@RonnyPfannschmidt could you give the final word here? thanks!

@zefirior
Copy link
Author

Thanks for review! I will be happy to help you in the future.

@asottile
Copy link
Member

just a heads up, I'm looking to revert this in #6197 (at least temporarily) as it caused some regressions

vinaycalastry pushed a commit to vinaycalastry/pytest that referenced this pull request Dec 11, 2019
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.

None yet

5 participants