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

Wrong fixture/conftest being used #2836

Closed
tom-dalton-fanduel opened this Issue Oct 13, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@tom-dalton-fanduel
Contributor

tom-dalton-fanduel commented Oct 13, 2017

I discovered a weird issue when running int tests on our Jenkins node at work, which I cannot reproduce inside a dev/vagrant VM using the same versions. The behaviour described below is only on the Jenkins node, but I don't really know where to go next in working out why it's happening. The original errors were in a way more complex setup, but I've managed to reduce this to a fairly minimal example. I doubt anyone else will be able to repro this in isolation, but i'm hoping someone can help me with where to look next to work out what's going on.

Minimal example file structure:

integration_tests/init.py (empty)

integration_tests/aa/init.py (empty)

integration_tests/aa/conftest.py

import pytest
@pytest.fixture
def url():
    return "1"

integration_tests/aa/test_x.py

def test_url(url):
    assert url == "1"

integration_tests/aa_foo/init.py (empty)

integration_tests/aa_foo/conftest.py

import pytest
@pytest.fixture
def url():
    return "2"

integration_tests/aa_foo/test_x.py

def test_url(url):
    assert url == "2"

So we have 2 packages, each containing a conftest defining a fixture url and each containing a test.

07:51:50 py.test integration_tests/
07:51:50 ============================= test session starts ==============================
07:51:50 platform linux -- Python 3.5.1, pytest-3.2.3, py-1.4.34, pluggy-0.4.0
07:51:50 rootdir: /var/lib/jenkins/workspace/adminweb-int-test, inifile:
07:51:50 collected 2 items
07:51:50 
07:51:50 integration_tests/aa/test_x.py .
07:51:50 integration_tests/aa_foo/test_x.py F
07:51:50 
07:51:50 =================================== FAILURES ===================================
07:51:50 ___________________________________ test_url ___________________________________
07:51:50 
07:51:50 url = '1'
07:51:50 
07:51:50     def test_url(url):
07:51:50 >       assert url == "2"
07:51:50 E       AssertionError: assert '1' == '2'
07:51:50 E         - 1
07:51:50 E         + 2
07:51:50 
07:51:50 integration_tests/aa_foo/test_x.py:3: AssertionError
07:51:50 ====================== 1 failed, 1 passed in 0.04 seconds ======================

What we see is that when aa_foo/test_x.py executes, it's using the wrong url fixture - it's using the fixture from aa rather than aa_foo. From investigation, it seems to be somehow related to aa and aa_foo sharing a common root name (aa). Longer versions of this name (originally these dirs were named create and create_with_stuff shows the same issue. I shortened the name repeatedly, and aa is the sortest name that shows the issue - if I change the package names to a and a_foo, it starts working again.

As said at the start, it works fine in a local development (vagrant) VM. Both cases (broken jenkins and working dev VM) are running Ubuntu 14.04 and python 3.5, and I've also created a new virtual env that only contains/installs py.test 3.2.3 to rule out other packages affecting this.

Are there any known bugs/issues in pytest that can cause this sort of behaviour? The original issue was found in a complex nest of parameterised fixtures defined at different levels, and I had originally assumed the bug was in that nest, but distilling it down to this simple example rules all the complexity out. It seems like pytest is simply using the wrong fixture.

Any help or guidance anyone can provide getting to the bottom of this would be very much appreciated!

@obestwalter

This comment has been minimized.

Member

obestwalter commented Oct 13, 2017

Maybe you get more information if you run:

pytest --trace-config

in your different environments and see if and how things behave differently?

@nicoddemus

This comment has been minimized.

Member

nicoddemus commented Oct 16, 2017

FWIW I also can't reproduce this. Just to make sure, the minimal example you posted here also fails on Jenkins?

Besides @obestwalter's advice, I would also take a look at the environment variables.

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 19, 2017

Thanks both. The minimal example given above fails on Jenkins but not elsewhere. I added --trace-config and the bit of the output that looks most useful comes right before the tests are run:

01:59:26 PLUGIN registered: <module 'integration_tests.aa_foo.conftest' (<_pytest.assertion.rewrite.AssertionRewritingHook object at 0x2ba28f88f1d0>)>
01:59:26 PLUGIN registered: <module 'integration_tests.aa.conftest' (<_pytest.assertion.rewrite.AssertionRewritingHook object at 0x2ba28f88f1d0>)>
01:59:26 collected 2 items
01:59:26 
01:59:26 integration_tests/aa/test_x.py .
01:59:26 integration_tests/aa_foo/test_x.py F

So that suggests it does recognise/collect both conftest modules, yet it's still using the fixture from the aa package against the aa_foo test. I'll try to do some more debugging but any further pointers to where I might need to be looking would be appreciated :-)

I'll also do a bit more digging into the env vars (Jenkins provides a lot of them, but a very quick skim doesn't show anything obviously askew, and all our other (quite a few of them) python/pytest jobs are working fine fortunately.

@tom-dalton-fanduel tom-dalton-fanduel changed the title from Wong fixture/conftest being used to Wrong fixture/conftest being used Oct 20, 2017

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 20, 2017

So I've been doing a bit more digging, adding a bunch of debugging into pytest to see what's going on. I've found a couple of odd things, but I don't know if this is expected behaviour - it feels like it isn't. The crux of the error/odd behaviour seems to be is that at the point that the aa_foo test is collected.

When aa/test_x/test_url is collected, the fixture info passed to the Function contains only one fixture, the url fixture from the aa/conftest.py. That's the case for me both locally and on Jenkins, and that seems totally normal.

However, when the aa_foo/test_x/test_url file is collected, the fixtureinfo data passed to the pytest Function (which is ultimately later used to run the test) contains both url fixtures from both conftests. When the test is run, the code in FixtureRequest._getnextfixturedef) basically seems to take the last fixture. On my local vagrant vm, the fixtures in the latter case are ordered [aa, aa_foo] and so the aa_foo fixture is used and the test passes. On Jenkins the fixtures are ordered the other way round, and the wrong fixture is used, the test then fails.

It seemed weird to me that the second test function was being built with both fixtures being passed to it. Unless I misunderstand the scoping/visiblity rules of fixtures, in the setup we have, the contests of aa package's conftest should not be visible to the sibling package aa_foo. So I renamed the packages abc and def and sure enough the problem went away again. But here's the difference:

Original (aa and aa_foo):

collecting 0 items                                                                                                                                                                                                   * PyCollector._genfunctions
* FixtureManager.getfixtureinfo
fixtureinfo:
<_pytest.fixtures.FuncFixtureInfo object at 0x7f0780850da0>
{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/aa' >,)}
collecting 1 item                                                                                                                                                                                                    * PyCollector._genfunctions
* FixtureManager.getfixtureinfo
fixtureinfo:
<_pytest.fixtures.FuncFixtureInfo object at 0x7f078083f588>
{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/aa' >, <FixtureDef name='url' scope='function' baseid='integration_tests/aa_foo' >)}
collected 2 items

Renamed packages to abc and def:

collecting 0 items                                                                                                                                                                                                   * PyCollector._genfunctions
* FixtureManager.getfixtureinfo
fixtureinfo:
<_pytest.fixtures.FuncFixtureInfo object at 0x7faab4ce6b00>
{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/abc' >,)}
collecting 1 item                                                                                                                                                                                                    * PyCollector._genfunctions
* FixtureManager.getfixtureinfo
fixtureinfo:
<_pytest.fixtures.FuncFixtureInfo object at 0x7faab4ceff98>
{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/def' >,)}
collected 2 items

The key difference is the contents of the fixtureinfo (specifically the .name2fixturedefs attrib) passed to Function:

{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/aa' >, <FixtureDef name='url' scope='function' baseid='integration_tests/aa_foo' >)}
vs
{'url': (<FixtureDef name='url' scope='function' baseid='integration_tests/def' >,)}

E.g. in the case where the packages share this common root name, for some reason the fixtures from the first package become valid for use in the second package. Is this intended behaviour? It certainly seems to be odd. For example, it means that the following setup works, but I would expect it to fail:

Example 2:

integration_tests/init.py (empty)
integration_tests/aa/init.py (empty)
integration_tests/aa/conftest.py

import pytest
@pytest.fixture
def url():
    return "1"

integration_tests/aa/test_x.py

def test_url(url):
    assert url == "1"

integration_tests/aa_foo/init.py (empty)
NB No conftest here!
integration_tests/aa_foo/test_x.py

def test_url(url):
    assert url == "1"

I would expect aa_foo/test_x/test_url to fail, stating there was no url fixture. But in fact it passes. Changing the names of the packages to abc and def, and then the second test fails as I expected.

So I think the fact it works locally and not on Jenkins is just symptom. The real issue appears to be is that pytest allows test functions to use fixtures they shouldn't be able to see at all.

  • It would be good if someone can confirm they see this same behaviour with the example 2 code above?
  • It would be good to know if this is actually unexpected behaviour, or if there is a good reason why this is happening?

In the meantime I'm going to keep digging into how/why both fixtures get passed to the second function when the names share the short common root, in the hope it gets clearer if this is expected or not.

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 23, 2017

I think I've found the cause of my woes, but I'll need someone to confirm if this is expected behaviour or a bug. I suspect (hope) it's the latter!

The problem seems to come down to this method: https://github.com/pytest-dev/pytest/blob/master/_pytest/fixtures.py#L1133

which:

  • takes a list of possible fixtures and a node (test file) where the fixture name is used
  • returns the list of fixtures that the node can 'see'

The way it decides if the node can see the fixture is a simple nodeid.startswith(fixturedef.baseid).

This means that in the case we have a test node like integration_tests/aa_foo/test_x.py, then it (I think incorrectly) thinks that the fixture from integration_tests/aa is a match - even though in fact this is a sibling package, rather than a parent package.

I'll make a minimal example repo to show the behaviour.

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 23, 2017

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 23, 2017

I've created PR #2862 with what seems to be a working fix for this issue...

@bilderbuchi

This comment has been minimized.

Contributor

bilderbuchi commented Oct 24, 2017

Great bughunting story, it was actually a joy following the narrative. :D

@tom-dalton-fanduel

This comment has been minimized.

Contributor

tom-dalton-fanduel commented Oct 24, 2017

Closing - Fixed by #2862

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment