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 #3854 #3861

Merged
merged 13 commits into from
Aug 25, 2018
2 changes: 2 additions & 0 deletions changelog/3796.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed an issue where teardown of fixtures of consecutive sub-packages were executed once, at the end of the outer
package.
1 change: 1 addition & 0 deletions changelog/3854.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes double collection of tests within packages when the filename starts with a capital letter.
3 changes: 1 addition & 2 deletions src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import functools
import inspect
import os
import sys
import warnings
from collections import OrderedDict, deque, defaultdict
Expand Down Expand Up @@ -93,7 +92,7 @@ def get_scope_package(node, fixturedef):

cls = pytest.Package
current = node
fixture_package_name = os.path.join(fixturedef.baseid, "__init__.py")
fixture_package_name = "%s/%s" % (fixturedef.baseid, "__init__.py")
while current and (
type(current) is not cls or fixture_package_name != current.nodeid
):
Expand Down
19 changes: 15 additions & 4 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -590,17 +590,28 @@ def collect(self):
self.session.config.pluginmanager._duplicatepaths.remove(path)

this_path = self.fspath.dirpath()
pkg_prefix = None
pkg_prefixes = set()
for path in this_path.visit(rec=self._recurse, bf=True, sort=True):
# we will visit our own __init__.py file, in which case we skip it
skip = False
if path.basename == "__init__.py" and path.dirpath() == this_path:
continue
if pkg_prefix and pkg_prefix in path.parts():

for pkg_prefix in pkg_prefixes:
if (
pkg_prefix in path.parts()
and pkg_prefix.join("__init__.py") != path
):
skip = True

if skip:
continue

if path.isdir() and path.join("__init__.py").check(file=1):
pkg_prefixes.add(path)

for x in self._collectfile(path):
yield x
if isinstance(x, Package):
pkg_prefix = path.dirpath()


def _get_xunit_setup_teardown(holder, attr_name, param_obj=None):
Expand Down
35 changes: 35 additions & 0 deletions testing/python/collect.py
Original file line number Diff line number Diff line change
Expand Up @@ -1624,3 +1624,38 @@ def test_package_with_modules(testdir):
root.chdir()
result = testdir.runpytest("-v", "-s")
result.assert_outcomes(passed=2)


def test_package_ordering(testdir):
"""
.
└── root
├── Test_root.py
├── __init__.py
├── sub1
│ ├── Test_sub1.py
│ └── __init__.py
└── sub2
└── test
└── test_sub2.py

"""
testdir.makeini(
"""
[pytest]
python_files=*.py
"""
)
root = testdir.mkpydir("root")
sub1 = root.mkdir("sub1")
sub1.ensure("__init__.py")
sub2 = root.mkdir("sub2")
sub2_test = sub2.mkdir("sub2")

root.join("Test_root.py").write("def test_1(): pass")
sub1.join("Test_sub1.py").write("def test_2(): pass")
sub2_test.join("test_sub2.py").write("def test_3(): pass")

# Execute from .
result = testdir.runpytest("-v", "-s")
result.assert_outcomes(passed=3)
69 changes: 69 additions & 0 deletions testing/python/fixture.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
import textwrap

import pytest
Expand Down Expand Up @@ -3977,3 +3978,71 @@ def test_func(self, f2, f1, m2):
items, _ = testdir.inline_genitems()
request = FixtureRequest(items[0])
assert request.fixturenames == "s1 p1 m1 m2 c1 f2 f1".split()

def test_multiple_packages(self, testdir):
"""Complex test involving multiple package fixtures. Make sure teardowns
are executed in order.
.
└── root
├── __init__.py
├── sub1
│ ├── __init__.py
│ ├── conftest.py
│ └── test_1.py
└── sub2
├── __init__.py
├── conftest.py
└── test_2.py
"""
root = testdir.mkdir("root")
root.join("__init__.py").write("values = []")
sub1 = root.mkdir("sub1")
sub1.ensure("__init__.py")
sub1.join("conftest.py").write(
textwrap.dedent(
"""\
import pytest
from .. import values
@pytest.fixture(scope="package")
def fix():
values.append("pre-sub1")
yield values
assert values.pop() == "pre-sub1"
"""
)
)
sub1.join("test_1.py").write(
textwrap.dedent(
"""\
from .. import values
def test_1(fix):
assert values == ["pre-sub1"]
"""
)
)
sub2 = root.mkdir("sub2")
sub2.ensure("__init__.py")
sub2.join("conftest.py").write(
textwrap.dedent(
"""\
Copy link
Member

Choose a reason for hiding this comment

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

To avoid this weird line break by black I usually put the triple strings into a variable:

contents = dedent("""\
    ...
""")
sub2.join("conftest.py").write(contents)

But really up to you, just in case this bothers you. 😉

Copy link
Member

Choose a reason for hiding this comment

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

black is going to reformat that to:

contents = dedent(
    """\
    ...
"""
    )

:rip:

Copy link
Member

Choose a reason for hiding this comment

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

Oh. 😞

Copy link
Member

Choose a reason for hiding this comment

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

you can do this:

x = """\
    ...
"""
print(dedent(x))

but then. meh.

import pytest
from .. import values
@pytest.fixture(scope="package")
def fix():
values.append("pre-sub2")
yield values
assert values.pop() == "pre-sub2"
"""
)
)
sub2.join("test_2.py").write(
textwrap.dedent(
"""\
from .. import values
def test_2(fix):
assert values == ["pre-sub2"]
"""
)
)
reprec = testdir.inline_run()
reprec.assertoutcome(passed=2)