-
-
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
Add package scoped fixtures #2283 #3389
Conversation
note that while adding that we should sort the relation to its also not in any way clear how things should cache and order for package nesting |
I've yet to review the PR, but I say if a package-scoped fixture is requested by the tests in |
@nicoddemus but that means we still need "synthetic" scope for every level of package and match fixtures to that in order to correctly set it up and in order to correctly order tests else you have trouble when doing fixture ordering and for parameterization in different levels |
@jonozzz does this PR only supports |
You can definitely have something like this in a
But if you'd set it to |
… Note this is still broken due to pytest-dev#3358.
Example: package1.subpackage1 package1.subpackage2 package1's setup/teardown were executed again when exiting subpackage1 and entering subpackage2.
Example: Given this hierarchy: p1.s1.s2.s3 I want to run pytest p1/s1/s2/s3/foo.py If there are any package fixtures defined at p1..s2 levels, they should also be executed.
…t to the initial argument(s). Address pytest-dev#3358 by caching nodes in a session dict.
While working on this I noticed another issue with |
@@ -1448,6 +1448,39 @@ def test_x(one): | |||
reprec = testdir.inline_run("..") | |||
reprec.assertoutcome(passed=2) | |||
|
|||
def test_package_xunit_fixture(self, testdir): |
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.
Could you also add another test with a package-scoped fixture? I also would like to see a more complex test with all fixture scopes involved.
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.
Added one and changed a few of the existing fixture tests to use package fixtures.
Changed existing complex tests to use package fixtures.
currently its still absolutely unclear to me how this interacts with deeper nesting and how this interacts with parameterization take as an example the following structure
when does what fixture run, how will tests be ordered |
It's very similar to "def fix -> (scope=session, params=[1,2,3])" except that the teardown part of this fixture (after yield) is executed once "exiting" the package not at the end of the session, as a scope=session would. For your example this is how the tests are ordered:
|
thansk for working out the example, i believe for better understanding i need a printout of setup-only as well, thanks for going the effort to make that listing, if possible i'd like to see that file committed as an example (im planning to use example files in test runs more) and would like to build on it |
Where do you want the examples to be checked in? I couldn't find an existing dir. Here's the output of the --setup-only:
|
the failure on the xdist call happens because of a breaking behaviour change introduced by the package scope the package objects import modules in the file tree even if there are no test modules below, |
Right and this was introduced recently and it's unrelated to the one-liner that I just checked in. Now if we look at tox.ini, for
Breaking change: |
@jonozzz while that assessment is entirely correct wrt introducing that failure as configuration, i propose trying to prevent a Package's collection from importing the package itself until a test module below it is actually imported how hard do you think that will be ? |
_pytest/nodes.py
Outdated
@@ -116,6 +116,7 @@ def ihook(self): | |||
Function = _CompatProperty("Function") | |||
File = _CompatProperty("File") | |||
Item = _CompatProperty("Item") | |||
Package = _CompatProperty("Package") |
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.
i missed his earlier, but those properties are scheduled for removal, i'd like to avoid adding more
Hey @jonozzz, I just wanted to let you know that we will not merge this until we get 3.6 out (which should be soon), it is not that we forgot or don't care about this PR. We want to avoid introducing a number of large changes in minor releases, to avoid potential breakage. For 3.6 we already have a large scale mark refactoring in the works. Thanks for the patience! 👍 |
No worries and thanks for the update. The code could use some cleanup though, especially in |
@RonnyPfannschmidt and @jonozzz, I think it is time for us to resume working on this. 👍 |
Sorry for the late reply- I was on vacation until today. Is there anything that needs to be added/changed, besides merging the changes? I just noticed there are some conflicts now. |
due to other development and introducing black, there turn up a lot of conflicts, we should resolve those |
im not sure if i can taek a look personally merging t this week, i would like to introduce this marked as "experimental" since i do fear that we end up finding a strange edge-case once this has been available to a wider audience i would like to note that the implementation is great work, as this touches nodes, scopes and introduces a new type of node, i fear that it may trigger a underlying issue |
Took the liberty of fixing the conflicts and pushing the changes 👍 (you have some time on your hands when you are on the phone trying to cancel your internet service... 1h and counting now 😠). |
win 👍 |
In response to #2283, adding package scoped fixtures, that follow python-nose's implementation.
In this case teardown_module() will be executed when all tests in test_foo.py are done.