Use fixtures to invoke xunit-style fixtures #4091

nicoddemus · 2018-10-08T22:44:03Z

WIP for #3094.

Unfortunately I'm not able to get this to work with yield tests, their expectations about when setup_* functions are called break when we move the setup_* call to fixtures.

Consider this test (simplified from test_runner_xunit.py::test_func_generator_setup):

import sys

def setup_module(mod):
    mod.x = []

def test_one():
    assert x == []
    def check():
        assert True
    yield check

This is what happens on master: during collection, setup_module is called (by Node.setup()), then test_one is called to generate the internal yield test function check. For this reason, the assert x == [] line at the beginning of test_one works.

What this PR does is to inject an autouse module-scoped fixture which calls setup_module. The problem is that this changes the semantics of yield-tests: test_one is still called during collection to generate the check test, but setup_module has not been called yet because it will be called by a fixture later during the runtest phase, so the line assert x == [] fails.

I don't see a way around this, so I'm afraid we will have to post-pone this for pytest 4.1, when we finally remove yield-tests.

RonnyPfannschmidt · 2018-10-09T20:32:11Z

src/_pytest/python.py

-            teardown_module = _get_xunit_setup_teardown(self.obj, "teardown_module")
-        if teardown_module is not None:
-            self.addfinalizer(teardown_module)
+    # def setup(self):


please remove the comments, dead code should go, we have history so we don't have to keep zombies 😈

Oh definitely. 👍

src/_pytest/python.py

RonnyPfannschmidt

would it be possible to use fixture factory parsing to add the fixtures as needed?

codecov · 2019-01-08T22:21:21Z

Codecov Report

Merging #4091 into features will decrease coverage by 0.36%.
The diff coverage is 95%.

@@             Coverage Diff              @@
##           features    #4091      +/-   ##
============================================
- Coverage     95.75%   95.38%   -0.37%     
============================================
  Files           111      111              
  Lines         24678    24737      +59     
  Branches       2446     2455       +9     
============================================
- Hits          23630    23595      -35     
- Misses          740      820      +80     
- Partials        308      322      +14

Flag	Coverage Δ
#docs	`?`
#doctesting	`?`
#linting	`?`
#linux	`94.01% <95%> (-1.57%)`	⬇️
#nobyte	`?`
#numpy	`41.31% <73.27%> (-51.89%)`	⬇️
#pexpect	`41.31% <73.27%> (-0.83%)`	⬇️
#py27	`92.61% <95%> (-1.19%)`	⬇️
#py34	`?`
#py35	`?`
#py36	`?`
#py37	`92.15% <94.16%> (-1.78%)`	⬇️
#trial	`41.31% <73.27%> (-51.89%)`	⬇️
#windows	`93.29% <94.16%> (-0.64%)`	⬇️
#xdist	`93.72% <94.16%> (-0.07%)`	⬇️

Impacted Files	Coverage Δ
testing/python/collect.py	`99.36% <ø> (-0.01%)`	⬇️
testing/python/fixture.py	`99.08% <100%> (ø)`	⬆️
src/_pytest/unittest.py	`94.08% <89.28%> (-0.2%)`	⬇️
src/_pytest/python.py	`92.33% <96.59%> (-0.67%)`	⬇️
src/_pytest/pytester.py	`82.31% <0%> (-5.14%)`	⬇️
src/_pytest/assertion/util.py	`93.03% <0%> (-4.51%)`	⬇️
testing/test_pdb.py	`95.13% <0%> (-3.9%)`	⬇️
src/_pytest/compat.py	`93.93% <0%> (-3.04%)`	⬇️
src/_pytest/doctest.py	`94.46% <0%> (-2.38%)`	⬇️
testing/python/integration.py	`89.36% <0%> (-2.13%)`	⬇️
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0da5531...0f918b1. Read the comment docs.

nicoddemus · 2019-01-08T22:55:12Z

Cool, now all tests pass under the new implementation.

TODO:

Remove dead code
Add specific tests
Some docs

nicoddemus · 2019-01-09T22:32:21Z

Finally ready for review 🎉

Fix pytest-dev#3094, Fix pytest-dev#517

s0undt3ch · 2019-01-12T20:11:03Z

Sorry for the question on PR, but here it goes...

I'm moving an existing unittest code base to pytest.
The initial goal is to make the full test suite just work under PyTest(yes its our fault that it just doesn't work off the bat).
We originally created a custom script to run our tests which setup a global object to store some important things about the test run. I didn't knew PyTest at the time or we'd have gone a different route 😃.
Anyway, on this migration to PyTest, we "patch" said object using fixtures(while we can't get rid of unittest altogether).
The problem I faced was that setUpClass gets called before any fixtures are evaluated, so I had to hack around that(sad but nothing impossible).

This PR suggests you're trying to move all that setup to fixtures which I assume should not require us to hack around the old behavior.

Anyway, the question(s):

4.1 is out meaning yield tests are gone. Any plans to get this in any time soon?
Will this allow us to override the setup fixture so that we force other fixtures to evaluate before the test class is setup?

Again, sorry for the question in the PR and for the long explanation around it.

nicoddemus · 2019-01-12T20:15:18Z

The problem I faced was that setUpClass gets called before any fixtures are evaluated, so I had to hack around that(sad but nothing impossible).

This is exactly the problem this PR tries to solve. 😁

With it, setUpClass is basically a class-scoped fixture, instead of being called as first thing during setup.

4.1 is out meaning yield tests are gone. Any plans to get this in any time soon?

Probably quite soon after this gets merged. I don't understand though why you mention yield tests in this question? Doesn't seem related.

Will this allow us to override the setup fixture so that we force other fixtures to evaluate before the test class is setup?

Which "setup fixture" you mean? Could you please clarify?

s0undt3ch · 2019-01-12T20:56:22Z

With it, setUpClass is basically a class-scoped fixture, instead of being called as first thing during setup.

Superb!

I don't understand though why you mention yield tests in this question? Doesn't seem related.

Looked like a blocker for the inclusion of this on 4.1

Which "setup fixture" you mean? Could you please clarify?

Read your code and your reply. Its no longer a question. Session scoped fixtures are evaluated before class based fixes.

🎉

Can't wait for this to get in the wild!!!

Thank You, and the rest of the PyTest contributors for making test writing something more joyful.

nicoddemus · 2019-01-13T14:36:12Z

@s0undt3ch oh I see, thanks. I thought you were talking about your own code, not about this PR. 😅

nicoddemus · 2019-01-15T23:09:49Z

@RonnyPfannschmidt @asottile @blueyed gentle ping. 😁

After this, #4280, and #4511 gets merged, I believe we are ready for a 4.2 release. 👍

nicoddemus · 2019-01-22T22:00:06Z

@asottile @blueyed, would you guys like more time to review this or can we merge it?

asottile · 2019-01-22T22:09:23Z

oh, I trust @RonnyPfannschmidt's judgement on this one and so I removed it from my "to review" queue 😆 -- let me know if you want another pair of eyes on it

s0undt3ch · 2019-01-23T21:24:48Z

🎉
Thanks! Can't wait for this to get released!

nicoddemus · 2019-01-23T21:25:32Z

It should be soon, likely next week or the next. 👍

It looks like pytest-dev/pytest#4091 broke the setUpTestData method, meaning all fixture data was being inserted for each test method. This is not a fix. If pytest doesn't provide a fix, we'll have to look at using it's session fixtures, or moving tests to full pytest functions – this would be good in the long term (it provides nicer debugging and less boilerplate), but would be a huge job.

nicoddemus added this to the 4.1 milestone Oct 8, 2018

nicoddemus requested a review from RonnyPfannschmidt October 8, 2018 22:44

This was referenced Oct 8, 2018

setup_* executed before all fixtures #517

Closed

Use fixtures to implement xUnit setup/teardown #3094

Closed

RonnyPfannschmidt reviewed Oct 9, 2018

View reviewed changes

src/_pytest/python.py Show resolved Hide resolved

RonnyPfannschmidt reviewed Oct 9, 2018

View reviewed changes

nicoddemus changed the title ~~WIP: setup_* methods as fixtures (delayed until 4.1)~~ setup_* methods as fixtures (delayed until 4.1) Oct 24, 2018

nicoddemus changed the title ~~setup_* methods as fixtures (delayed until 4.1)~~ WIP: setup_* methods as fixtures (delayed until 4.1) Oct 24, 2018

nicoddemus mentioned this pull request Nov 14, 2018

Release 4.0.0 #4388

Merged

nicoddemus modified the milestones: 4.1, 4.2 Dec 14, 2018

nicoddemus changed the title ~~WIP: setup_* methods as fixtures (delayed until 4.1)~~ WIP: setup_* methods as fixtures Dec 14, 2018

nicoddemus force-pushed the setup-methods-as-fixtures-3094 branch from 80e890b to 5ec4e42 Compare January 8, 2019 22:20

nicoddemus force-pushed the setup-methods-as-fixtures-3094 branch from 5ec4e42 to 954de66 Compare January 8, 2019 22:37

nicoddemus force-pushed the setup-methods-as-fixtures-3094 branch from 954de66 to 7d53b5b Compare January 9, 2019 22:31

nicoddemus changed the title ~~WIP: setup_* methods as fixtures~~ Use fixtures to invoke xunit-style fixtures Jan 9, 2019

RonnyPfannschmidt mentioned this pull request Jan 10, 2019

report.nodeid is not proper in pytest_runtest_logreport() hook for setup/teardown #4629

Closed

xunit-style functions and methods are invoked by autouse fixtures

0f918b1

Fix pytest-dev#3094, Fix pytest-dev#517

nicoddemus force-pushed the setup-methods-as-fixtures-3094 branch from 7d53b5b to 0f918b1 Compare January 10, 2019 14:10

RonnyPfannschmidt approved these changes Jan 16, 2019

View reviewed changes

nicoddemus merged commit daf3911 into pytest-dev:features Jan 23, 2019

nicoddemus deleted the setup-methods-as-fixtures-3094 branch January 23, 2019 21:23

okuta mentioned this pull request Feb 25, 2019

Very slow test setup in pytest 4.2.0+ #4824

Closed

nicoddemus mentioned this pull request Mar 1, 2019

Function-scoped fixture run before session-scoped with unittest.TestCase #4143

Closed

rsyring mentioned this pull request May 19, 2019

Support usage of other fixtures in xunit style methods: setup() and setup_class() #5289

Closed

stanislavlevin mentioned this pull request Oct 12, 2019

Dependencies between xunit-style setups and the other fixtures #5949

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use fixtures to invoke xunit-style fixtures #4091

Use fixtures to invoke xunit-style fixtures #4091

nicoddemus commented Oct 8, 2018

RonnyPfannschmidt Oct 9, 2018

nicoddemus Oct 9, 2018

RonnyPfannschmidt left a comment

codecov bot commented Jan 8, 2019 •

edited

Loading

nicoddemus commented Jan 8, 2019 •

edited

Loading

nicoddemus commented Jan 9, 2019

s0undt3ch commented Jan 12, 2019

nicoddemus commented Jan 12, 2019

s0undt3ch commented Jan 12, 2019 •

edited

Loading

nicoddemus commented Jan 13, 2019

nicoddemus commented Jan 15, 2019

nicoddemus commented Jan 22, 2019

asottile commented Jan 22, 2019

s0undt3ch commented Jan 23, 2019

nicoddemus commented Jan 23, 2019

Use fixtures to invoke xunit-style fixtures #4091

Use fixtures to invoke xunit-style fixtures #4091

Conversation

nicoddemus commented Oct 8, 2018

RonnyPfannschmidt Oct 9, 2018

Choose a reason for hiding this comment

nicoddemus Oct 9, 2018

Choose a reason for hiding this comment

RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 8, 2019 • edited Loading

Codecov Report

nicoddemus commented Jan 8, 2019 • edited Loading

nicoddemus commented Jan 9, 2019

s0undt3ch commented Jan 12, 2019

nicoddemus commented Jan 12, 2019

s0undt3ch commented Jan 12, 2019 • edited Loading

nicoddemus commented Jan 13, 2019

nicoddemus commented Jan 15, 2019

nicoddemus commented Jan 22, 2019

asottile commented Jan 22, 2019

s0undt3ch commented Jan 23, 2019

nicoddemus commented Jan 23, 2019

codecov bot commented Jan 8, 2019 •

edited

Loading

nicoddemus commented Jan 8, 2019 •

edited

Loading

s0undt3ch commented Jan 12, 2019 •

edited

Loading