From 4bc6ecb8a50e30b04d83e141fd55e5c3c80faeb8 Mon Sep 17 00:00:00 2001 From: Xuan Luong Date: Thu, 5 Oct 2017 20:14:45 -0400 Subject: [PATCH 01/27] Add mention of xpass in skip/xfail documentation --- changelog/1997.doc | 1 + doc/en/skipping.rst | 12 ++++++++---- 2 files changed, 9 insertions(+), 4 deletions(-) create mode 100644 changelog/1997.doc diff --git a/changelog/1997.doc b/changelog/1997.doc new file mode 100644 index 00000000000..0fa110dc9ab --- /dev/null +++ b/changelog/1997.doc @@ -0,0 +1 @@ +Explicitly mention ``xpass`` in the documentation of ``xfail``. diff --git a/doc/en/skipping.rst b/doc/en/skipping.rst index 1504c251c74..f8cfb73ca4d 100644 --- a/doc/en/skipping.rst +++ b/doc/en/skipping.rst @@ -16,13 +16,17 @@ resource which is not available at the moment (for example a database). A **xfail** means that you expect a test to fail for some reason. A common example is a test for a feature not yet implemented, or a bug not yet fixed. +When a test passes despite being expected to fail (marked with ``pytest.mark.xfail``), +it's an **xpass** and will be reported in the test summary. ``pytest`` counts and lists *skip* and *xfail* tests separately. Detailed information about skipped/xfailed tests is not shown by default to avoid cluttering the output. You can use the ``-r`` option to see details corresponding to the "short" letters shown in the test progress:: - pytest -rxs # show extra info on skips and xfails + pytest -rxXs # show extra info on xfailed, xpassed, and skipped tests + +More details on the ``-r`` option can be found by running ``pytest -h``. (See :ref:`how to change command line options defaults`) @@ -311,12 +315,12 @@ Running it with the report-on-xfail option gives this output:: platform linux -- Python 3.x.y, pytest-3.x.y, py-1.x.y, pluggy-0.x.y rootdir: $REGENDOC_TMPDIR/example, inifile: collected 7 items - + xfail_demo.py xxxxxxx ======= short test summary info ======== XFAIL xfail_demo.py::test_hello XFAIL xfail_demo.py::test_hello2 - reason: [NOTRUN] + reason: [NOTRUN] XFAIL xfail_demo.py::test_hello3 condition: hasattr(os, 'sep') XFAIL xfail_demo.py::test_hello4 @@ -326,7 +330,7 @@ Running it with the report-on-xfail option gives this output:: XFAIL xfail_demo.py::test_hello6 reason: reason XFAIL xfail_demo.py::test_hello7 - + ======= 7 xfailed in 0.12 seconds ======== .. _`skip/xfail with parametrize`: From c24ffa3b4c777d5ec009eb3fa2a63a3e593ab8d9 Mon Sep 17 00:00:00 2001 From: Leonard Lausen Date: Sun, 8 Oct 2017 12:23:26 +0900 Subject: [PATCH 02/27] Fix pytest.parametrize when argnames are specified as kwarg --- _pytest/fixtures.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index f57031e1ad0..8d5e0c4b31c 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1037,9 +1037,13 @@ def pytest_generate_tests(self, metafunc): if faclist: fixturedef = faclist[-1] if fixturedef.params is not None: - func_params = getattr(getattr(metafunc.function, 'parametrize', None), 'args', [[None]]) + parametrize_func = getattr(metafunc.function, 'parametrize', None) + func_params = getattr(parametrize_func, 'args', [[None]]) # skip directly parametrized arguments - argnames = func_params[0] + if "argnames" in parametrize_func.kwargs: + argnames = parametrize_func.kwargs["argnames"] + else: + argnames = func_params[0] if not isinstance(argnames, (tuple, list)): argnames = [x.strip() for x in argnames.split(",") if x.strip()] if argname not in func_params and argname not in argnames: From 48b5c13f73f0bda248fd2d4f13ea054b0e7b31b9 Mon Sep 17 00:00:00 2001 From: Leonard Lausen Date: Sun, 8 Oct 2017 12:33:02 +0900 Subject: [PATCH 03/27] Add changelog for #2819 --- changelog/2819.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/2819.bugfix diff --git a/changelog/2819.bugfix b/changelog/2819.bugfix new file mode 100644 index 00000000000..303903cf745 --- /dev/null +++ b/changelog/2819.bugfix @@ -0,0 +1 @@ +Fix issue with @pytest.parametrize if argnames was specified as kwarg. \ No newline at end of file From e89abe6a40747a8ba592ea722264d99f7a0ed5e0 Mon Sep 17 00:00:00 2001 From: Leonard Lausen Date: Mon, 9 Oct 2017 00:37:27 +0900 Subject: [PATCH 04/27] Defensive fallback in case of kwargs not being present --- _pytest/fixtures.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 8d5e0c4b31c..e1c2d05f432 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1039,8 +1039,9 @@ def pytest_generate_tests(self, metafunc): if fixturedef.params is not None: parametrize_func = getattr(metafunc.function, 'parametrize', None) func_params = getattr(parametrize_func, 'args', [[None]]) + func_kwargs = getattr(parametrize_func, 'kwargs', {}) # skip directly parametrized arguments - if "argnames" in parametrize_func.kwargs: + if "argnames" in func_kwargs: argnames = parametrize_func.kwargs["argnames"] else: argnames = func_params[0] From e86ba41a320f2dd4b28eb1f96eddf77069f46950 Mon Sep 17 00:00:00 2001 From: Leonard Lausen Date: Mon, 9 Oct 2017 01:02:54 +0900 Subject: [PATCH 05/27] Add testcase for #2819 --- testing/test_mark.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/testing/test_mark.py b/testing/test_mark.py index ae070f3a0f9..dc51bbac0f2 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -342,6 +342,24 @@ def test_func(foo, bar): ]) +def test_parametrized_with_kwargs(testdir): + """Test collect parametrized func with wrong number of args.""" + py_file = testdir.makepyfile(""" + import pytest + + @pytest.fixture(params=[1,2]) + def a(request): + return request.param + + @pytest.mark.parametrize(argnames='b', argvalues=[1, 2]) + def test_func(a, b): + pass + """) + + result = testdir.runpytest(py_file) + assert(result.ret == 0) + + class TestFunctional(object): def test_mark_per_function(self, testdir): From cfdebb3ba4c4fa56d6481495f1e01bb5867b2113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D0=A1=D0=B5=D0=BC=D1=91=D0=BD=20=D0=9C=D0=B0=D1=80=D1=8C?= =?UTF-8?q?=D1=8F=D1=81=D0=B8=D0=BD?= Date: Mon, 16 Oct 2017 01:55:30 +0300 Subject: [PATCH 06/27] Fix typo in parametrization doc --- doc/en/example/parametrize.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/en/example/parametrize.rst b/doc/en/example/parametrize.rst index b72e8e6de3a..ffeb5a951be 100644 --- a/doc/en/example/parametrize.rst +++ b/doc/en/example/parametrize.rst @@ -350,7 +350,7 @@ Parametrizing test methods through per-class configuration .. _`unittest parametrizer`: https://github.com/testing-cabal/unittest-ext/blob/master/params.py -Here is an example ``pytest_generate_function`` function implementing a +Here is an example ``pytest_generate_tests`` function implementing a parametrization scheme similar to Michael Foord's `unittest parametrizer`_ but in a lot less code:: From a4fd5cdcb53e49f8c482386612cd7b2efc1e3c44 Mon Sep 17 00:00:00 2001 From: Pierre GIRAUD Date: Mon, 16 Oct 2017 10:23:35 +0200 Subject: [PATCH 07/27] Fix auto-use fixture doc --- doc/en/fixture.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/en/fixture.rst b/doc/en/fixture.rst index db7ef0ca2e4..1f2a52a778f 100644 --- a/doc/en/fixture.rst +++ b/doc/en/fixture.rst @@ -858,7 +858,7 @@ into a conftest.py file **without** using ``autouse``:: # content of conftest.py @pytest.fixture - def transact(self, request, db): + def transact(request, db): db.begin() yield db.rollback() From 11b391ff4911c8a0d0a35df3586d3c7e2342ef7a Mon Sep 17 00:00:00 2001 From: Matty G Date: Tue, 17 Oct 2017 14:11:07 -0700 Subject: [PATCH 08/27] Update mark.py --- _pytest/mark.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_pytest/mark.py b/_pytest/mark.py index e4b6238eb9e..91aa3747ed1 100644 --- a/_pytest/mark.py +++ b/_pytest/mark.py @@ -383,7 +383,7 @@ def store_mark(obj, mark): """ assert isinstance(mark, Mark), mark # always reassign name to avoid updating pytestmark - # in a referene that was only borrowed + # in a reference that was only borrowed obj.pytestmark = get_unpacked_marks(obj) + [mark] From baadd569e8e1be13fc0d7b0c55273520f5367463 Mon Sep 17 00:00:00 2001 From: Christoph Buchner Date: Tue, 17 Oct 2017 23:42:32 +0200 Subject: [PATCH 09/27] Clarify the documentation of fixture scopes. Closes #538. --- changelog/538.doc | 1 + doc/en/fixture.rst | 18 +++++++++++------- 2 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 changelog/538.doc diff --git a/changelog/538.doc b/changelog/538.doc new file mode 100644 index 00000000000..bc5fb712fa4 --- /dev/null +++ b/changelog/538.doc @@ -0,0 +1 @@ +Clarify the documentation of available fixture scopes. diff --git a/doc/en/fixture.rst b/doc/en/fixture.rst index db7ef0ca2e4..73f38d726f5 100644 --- a/doc/en/fixture.rst +++ b/doc/en/fixture.rst @@ -27,7 +27,7 @@ functions: * fixture management scales from simple unit to complex functional testing, allowing to parametrize fixtures and tests according to configuration and component options, or to re-use fixtures - across class, module or whole test session scopes. + across function, class, module or whole test session scopes. In addition, pytest continues to support :ref:`xunitsetup`. You can mix both styles, moving incrementally from classic to new style, as you @@ -129,8 +129,8 @@ functions take the role of the *injector* and test functions are the .. _smtpshared: -Sharing a fixture across tests in a module (or class/session) ------------------------------------------------------------------ +Scope: Sharing a fixture across tests in a class, module or session +------------------------------------------------------------------- .. regendoc:wipe @@ -139,10 +139,12 @@ usually time-expensive to create. Extending the previous example, we can add a ``scope='module'`` parameter to the :py:func:`@pytest.fixture <_pytest.python.fixture>` invocation to cause the decorated ``smtp`` fixture function to only be invoked once -per test module. Multiple test functions in a test module will thus -each receive the same ``smtp`` fixture instance. The next example puts -the fixture function into a separate ``conftest.py`` file so -that tests from multiple test modules in the directory can +per test *module* (the default is to invoke once per test *function*). +Multiple test functions in a test module will thus +each receive the same ``smtp`` fixture instance, thus saving time. + +The next example puts the fixture function into a separate ``conftest.py`` file +so that tests from multiple test modules in the directory can access the fixture function:: # content of conftest.py @@ -223,6 +225,8 @@ instance, you can simply declare it: # the returned fixture value will be shared for # all tests needing it +Finally, the ``class`` scope will invoke the fixture once per test *class*. + .. _`finalization`: Fixture finalization / executing teardown code From 46cc9ab77c5e04fbb6c8b06688b8a41b686d3c97 Mon Sep 17 00:00:00 2001 From: Christoph Buchner Date: Wed, 18 Oct 2017 21:30:56 +0200 Subject: [PATCH 10/27] Add documentation about python -m pytest invocation. --- changelog/911.doc | 1 + doc/en/pythonpath.rst | 5 +++++ doc/en/usage.rst | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 changelog/911.doc diff --git a/changelog/911.doc b/changelog/911.doc new file mode 100644 index 00000000000..e9d94f21ce8 --- /dev/null +++ b/changelog/911.doc @@ -0,0 +1 @@ +Add documentation about the ``python -m pytest`` invocation adding the current directory to sys.path. diff --git a/doc/en/pythonpath.rst b/doc/en/pythonpath.rst index 67de7f5d2ca..b6474276887 100644 --- a/doc/en/pythonpath.rst +++ b/doc/en/pythonpath.rst @@ -68,4 +68,9 @@ imported in the global import namespace. This is also discussed in details in :ref:`test discovery`. +Invoking ``pytest`` versus ``python -m pytest`` +----------------------------------------------- +Running pytest with ``python -m pytest [...]`` instead of ``pytest [...]`` yields nearly +equivalent behaviour, except that the former call will add the current directory to ``sys.path``. +See also :ref:`cmdline`. diff --git a/doc/en/usage.rst b/doc/en/usage.rst index a8c6d40a027..c5b919fe9eb 100644 --- a/doc/en/usage.rst +++ b/doc/en/usage.rst @@ -17,7 +17,7 @@ You can invoke testing through the Python interpreter from the command line:: python -m pytest [...] This is almost equivalent to invoking the command line script ``pytest [...]`` -directly, except that Python will also add the current directory to ``sys.path``. +directly, except that calling via ``python`` will also add the current directory to ``sys.path``. Possible exit codes -------------------------------------------------------------- From 3cdbb1854f802bc3250ebcf3014fc1241389c48c Mon Sep 17 00:00:00 2001 From: Owen Tuz Date: Sat, 21 Oct 2017 23:12:49 +0100 Subject: [PATCH 11/27] #2692: Document setup/teardown behaviour when using unittest-based suites --- doc/en/unittest.rst | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/en/unittest.rst b/doc/en/unittest.rst index 92b9e653eee..db169202935 100644 --- a/doc/en/unittest.rst +++ b/doc/en/unittest.rst @@ -233,3 +233,13 @@ was executed ahead of the ``test_method``. overwrite ``unittest.TestCase`` ``__call__`` or ``run``, they need to to overwrite ``debug`` in the same way (this is also true for standard unittest). + +.. note:: + + Due to architectural differences between the two frameworks, setup and + teardown for ``unittest``-based tests is performed during the ``call`` phase + of testing instead of in ``pytest``'s standard ``setup`` and ``teardown`` + stages. This can be important to understand in some situations, particularly + when reasoning about errors. For example, if a ``unittest``-based suite + exhibits errors during setup, ``pytest`` will report no errors during its + ``setup`` phase and will instead raise the error during ``call``. From 29fa9d5bff93c90526286117ddc788799ff18673 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 12:28:54 +0100 Subject: [PATCH 12/27] Add failing test --- testing/test_collection.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/testing/test_collection.py b/testing/test_collection.py index 5d165441094..f15e4925c78 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -2,6 +2,7 @@ import pytest import py +import _pytest._code from _pytest.main import Session, EXIT_NOTESTSCOLLECTED, _in_venv @@ -830,3 +831,28 @@ def test_continue_on_collection_errors_maxfail(testdir): "*Interrupted: stopping after 3 failures*", "*1 failed, 2 error*", ]) + + +def test_fixture_scope_sibling_conftests(testdir): + """Regression test case for https://github.com/pytest-dev/pytest/issues/2836""" + foo_path = testdir.mkpydir("foo") + foo_path.join("conftest.py").write(_pytest._code.Source(""" + import pytest + @pytest.fixture + def fix(): + return 1 + """)) + foo_path.join("test_foo.py").write("def test_foo(fix): assert fix == 1") + + # Tests in `food/` should not see the conftest fixture from `foo/` + food_path = testdir.mkpydir("food") + food_path.join("test_food.py").write("def test_food(fix): assert fix == 1") + + res = testdir.runpytest() + assert res.ret == 1 + + # TODO Work out what this will really look like. Currently the retcode assertion above fails (as expected). + res.stdout.fnmatch_lines([ + "collected 2 items / 1 errors", + "*1 passed, 1 error*", + ]) From c03612f729a81f60002b540cc1b1310f4fa94752 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 12:40:32 +0100 Subject: [PATCH 13/27] Test now looks for real expected output --- testing/test_collection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/test_collection.py b/testing/test_collection.py index f15e4925c78..49a290060c4 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -851,8 +851,8 @@ def fix(): res = testdir.runpytest() assert res.ret == 1 - # TODO Work out what this will really look like. Currently the retcode assertion above fails (as expected). res.stdout.fnmatch_lines([ - "collected 2 items / 1 errors", + "*ERROR at setup of test_food*", + "E*fixture 'fix' not found", "*1 passed, 1 error*", ]) From 1e6dc6f8e5efd63c3626785982e34353a5d799f2 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 13:26:42 +0100 Subject: [PATCH 14/27] Working (I think) fix for #2836 --- _pytest/fixtures.py | 36 +++++++++++++++++++++++++++++++++++- testing/test_fixtures.py | 18 ++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 testing/test_fixtures.py diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index e1c2d05f432..51ccaed67c9 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1130,7 +1130,41 @@ def getfixturedefs(self, argname, nodeid): else: return tuple(self._matchfactories(fixturedefs, nodeid)) + @classmethod + def _splitnode(cls, nodeid): + """Split a nodeid into constituent 'parts'. + + Node IDs are strings, and can be things like: + '', + 'testing/code', + 'testing/code/test_excinfo.py' + 'testing/code/test_excinfo.py::TestFormattedExcinfo::()' + + """ + if nodeid == '': + return [] + sep = py.path.local.sep + parts = nodeid.split(sep) + if parts: + last_part = parts[-1] + if '::' in last_part: + namespace_parts = last_part.split("::") + parts[-1:] = namespace_parts + return parts + + @classmethod + def _ischildnode(cls, baseid, nodeid): + """Return True if the nodeid is a child node of the baseid. + + E.g. 'foo/bar::Baz::()' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp' + """ + base_parts = cls._splitnode(baseid) + node_parts = cls._splitnode(nodeid) + if len(node_parts) < len(base_parts): + return False + return node_parts[:len(base_parts)] == base_parts + def _matchfactories(self, fixturedefs, nodeid): for fixturedef in fixturedefs: - if nodeid.startswith(fixturedef.baseid): + if self._ischildnode(fixturedef.baseid, nodeid): yield fixturedef diff --git a/testing/test_fixtures.py b/testing/test_fixtures.py new file mode 100644 index 00000000000..8d595fe5271 --- /dev/null +++ b/testing/test_fixtures.py @@ -0,0 +1,18 @@ +import pytest + +from _pytest import fixtures + + +@pytest.mark.parametrize("baseid, nodeid, expected", ( + ('', '', True), + ('', 'foo', True), + ('', 'foo/bar', True), + ('', 'foo/bar::TestBaz::()', True), + ('foo', 'food', False), + ('foo/bar::TestBaz::()', 'foo/bar', False), + ('foo/bar::TestBaz::()', 'foo/bar::TestBop::()', False), + ('foo/bar', 'foo/bar::TestBop::()', True), +)) +def test_fixturemanager_ischildnode(baseid, nodeid, expected): + result = fixtures.FixtureManager._ischildnode(baseid, nodeid) + assert result is expected From f003914d4b066e4825da2afa1180914620eecfbc Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 15:37:02 +0100 Subject: [PATCH 15/27] Add changelog entry for #2836 --- changelog/2836.bug | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog/2836.bug diff --git a/changelog/2836.bug b/changelog/2836.bug new file mode 100644 index 00000000000..a6e36a34b01 --- /dev/null +++ b/changelog/2836.bug @@ -0,0 +1,3 @@ +Attempted fix for https://github.com/pytest-dev/pytest/issues/2836 + +Improves the handling of "nodeid"s to be more aware of path and class separators. From de9d116a49eff117f124b70f89e0bd0e5cd8ffc0 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 15:37:15 +0100 Subject: [PATCH 16/27] Added Tom Dalton to AUTHORS --- AUTHORS | 1 + 1 file changed, 1 insertion(+) diff --git a/AUTHORS b/AUTHORS index 6e341d64e8f..cbebd9e3093 100644 --- a/AUTHORS +++ b/AUTHORS @@ -164,6 +164,7 @@ Stephan Obermann Tareq Alayan Ted Xiao Thomas Grainger +Tom Dalton Tom Viner Trevor Bekolay Tyler Goodlet From ee7e1c94d2436e82ad4e455f0cc436ddacae44e3 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 16:12:07 +0100 Subject: [PATCH 17/27] Remove redundant if, tidy if-body --- _pytest/fixtures.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 51ccaed67c9..f69326f1d42 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1145,11 +1145,10 @@ def _splitnode(cls, nodeid): return [] sep = py.path.local.sep parts = nodeid.split(sep) - if parts: - last_part = parts[-1] - if '::' in last_part: - namespace_parts = last_part.split("::") - parts[-1:] = namespace_parts + last_part = parts[-1] + if '::' in last_part: + # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' + parts[-1:] = last_part.split("::") return parts @classmethod From d714c196a523a121960573fa75771572c4db0b9b Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 16:55:35 +0100 Subject: [PATCH 18/27] Shorter code, longer docstring --- _pytest/fixtures.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index f69326f1d42..c62e9185a40 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1135,20 +1135,20 @@ def _splitnode(cls, nodeid): """Split a nodeid into constituent 'parts'. Node IDs are strings, and can be things like: - '', - 'testing/code', + '' + 'testing/code' 'testing/code/test_excinfo.py' 'testing/code/test_excinfo.py::TestFormattedExcinfo::()' + Return values are lists e.g. + [''] + ['testing', 'code'] + ['testing', 'code', test_excinfo.py'] + ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo', '()'] """ - if nodeid == '': - return [] - sep = py.path.local.sep - parts = nodeid.split(sep) - last_part = parts[-1] - if '::' in last_part: - # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' - parts[-1:] = last_part.split("::") + parts = nodeid.split(py.path.local.sep) + # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' + parts[-1:] = parts[-1].split("::") return parts @classmethod From a7199fa8ab0d90dccd2496f122d5f438af961448 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 16:59:56 +0100 Subject: [PATCH 19/27] Docstring typo --- _pytest/fixtures.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index c62e9185a40..9b2abc94f14 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1143,7 +1143,7 @@ def _splitnode(cls, nodeid): Return values are lists e.g. [''] ['testing', 'code'] - ['testing', 'code', test_excinfo.py'] + ['testing', 'code', 'test_excinfo.py'] ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo', '()'] """ parts = nodeid.split(py.path.local.sep) From 655ab0bf8b8af9962e40360b40c7e5b55c2092f6 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Mon, 23 Oct 2017 17:49:49 +0100 Subject: [PATCH 20/27] Address more review comments, fix massive bug I reintroduced in the node-splitting code :-/ --- _pytest/fixtures.py | 47 ++++----------------- _pytest/nodes.py | 37 ++++++++++++++++ changelog/2836.bug | 4 +- testing/{test_fixtures.py => test_nodes.py} | 6 +-- 4 files changed, 49 insertions(+), 45 deletions(-) create mode 100644 _pytest/nodes.py rename testing/{test_fixtures.py => test_nodes.py} (71%) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 9b2abc94f14..3a1321664d4 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -1,12 +1,12 @@ from __future__ import absolute_import, division, print_function -import sys -from py._code.code import FormattedExcinfo +import inspect +import sys +import warnings import py -import warnings +from py._code.code import FormattedExcinfo -import inspect import _pytest from _pytest._code.code import TerminalRepr from _pytest.compat import ( @@ -15,9 +15,11 @@ is_generator, isclass, getimfunc, getlocation, getfuncargnames, safe_getattr, + FuncargnamesCompatAttr, ) +from _pytest.nodes import ischildnode from _pytest.outcomes import fail, TEST_OUTCOME -from _pytest.compat import FuncargnamesCompatAttr + if sys.version_info[:2] == (2, 6): from ordereddict import OrderedDict @@ -1130,40 +1132,7 @@ def getfixturedefs(self, argname, nodeid): else: return tuple(self._matchfactories(fixturedefs, nodeid)) - @classmethod - def _splitnode(cls, nodeid): - """Split a nodeid into constituent 'parts'. - - Node IDs are strings, and can be things like: - '' - 'testing/code' - 'testing/code/test_excinfo.py' - 'testing/code/test_excinfo.py::TestFormattedExcinfo::()' - - Return values are lists e.g. - [''] - ['testing', 'code'] - ['testing', 'code', 'test_excinfo.py'] - ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo', '()'] - """ - parts = nodeid.split(py.path.local.sep) - # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' - parts[-1:] = parts[-1].split("::") - return parts - - @classmethod - def _ischildnode(cls, baseid, nodeid): - """Return True if the nodeid is a child node of the baseid. - - E.g. 'foo/bar::Baz::()' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp' - """ - base_parts = cls._splitnode(baseid) - node_parts = cls._splitnode(nodeid) - if len(node_parts) < len(base_parts): - return False - return node_parts[:len(base_parts)] == base_parts - def _matchfactories(self, fixturedefs, nodeid): for fixturedef in fixturedefs: - if self._ischildnode(fixturedef.baseid, nodeid): + if ischildnode(fixturedef.baseid, nodeid): yield fixturedef diff --git a/_pytest/nodes.py b/_pytest/nodes.py new file mode 100644 index 00000000000..c28c631804e --- /dev/null +++ b/_pytest/nodes.py @@ -0,0 +1,37 @@ +import py + + +def _splitnode(nodeid): + """Split a nodeid into constituent 'parts'. + + Node IDs are strings, and can be things like: + '' + 'testing/code' + 'testing/code/test_excinfo.py' + 'testing/code/test_excinfo.py::TestFormattedExcinfo::()' + + Return values are lists e.g. + [] + ['testing', 'code'] + ['testing', 'code', 'test_excinfo.py'] + ['testing', 'code', 'test_excinfo.py', 'TestFormattedExcinfo', '()'] + """ + if nodeid == '': + # If there is no root node at all, return an empty list so the caller's logic can remain sane + return [] + parts = nodeid.split(py.path.local.sep) + # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' + parts[-1:] = parts[-1].split("::") + return parts + + +def ischildnode(baseid, nodeid): + """Return True if the nodeid is a child node of the baseid. + + E.g. 'foo/bar::Baz::()' is a child of 'foo', 'foo/bar' and 'foo/bar::Baz', but not of 'foo/blorp' + """ + base_parts = _splitnode(baseid) + node_parts = _splitnode(nodeid) + if len(node_parts) < len(base_parts): + return False + return node_parts[:len(base_parts)] == base_parts diff --git a/changelog/2836.bug b/changelog/2836.bug index a6e36a34b01..afa1961d73e 100644 --- a/changelog/2836.bug +++ b/changelog/2836.bug @@ -1,3 +1 @@ -Attempted fix for https://github.com/pytest-dev/pytest/issues/2836 - -Improves the handling of "nodeid"s to be more aware of path and class separators. +Match fixture paths against actual path segments in order to avoid matching folders which share a prefix. diff --git a/testing/test_fixtures.py b/testing/test_nodes.py similarity index 71% rename from testing/test_fixtures.py rename to testing/test_nodes.py index 8d595fe5271..6f4540f99b9 100644 --- a/testing/test_fixtures.py +++ b/testing/test_nodes.py @@ -1,6 +1,6 @@ import pytest -from _pytest import fixtures +from _pytest import nodes @pytest.mark.parametrize("baseid, nodeid, expected", ( @@ -13,6 +13,6 @@ ('foo/bar::TestBaz::()', 'foo/bar::TestBop::()', False), ('foo/bar', 'foo/bar::TestBop::()', True), )) -def test_fixturemanager_ischildnode(baseid, nodeid, expected): - result = fixtures.FixtureManager._ischildnode(baseid, nodeid) +def test_ischildnode(baseid, nodeid, expected): + result = nodes.ischildnode(baseid, nodeid) assert result is expected From dc5f33ba5c7668907e41720de024465a8a795e50 Mon Sep 17 00:00:00 2001 From: Pavel Karateev Date: Mon, 23 Oct 2017 21:39:13 +0300 Subject: [PATCH 21/27] Remove typo @ in assignment --- doc/en/warnings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/en/warnings.rst b/doc/en/warnings.rst index c842771739f..de8456af023 100644 --- a/doc/en/warnings.rst +++ b/doc/en/warnings.rst @@ -109,7 +109,7 @@ decorator or to all tests in a module by setting the ``pytestmark`` variable: .. code-block:: python # turns all warnings into errors for this module - pytestmark = @pytest.mark.filterwarnings('error') + pytestmark = pytest.mark.filterwarnings('error') .. note:: From a3ec3df0c883d0655beadb44e3873c25bc38d06c Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 23 Oct 2017 18:19:15 -0200 Subject: [PATCH 22/27] Add E722 and E741 flake errors to the ignore list Also fixed 'E704 multiple statements on one line (def)' in python_api --- _pytest/python_api.py | 3 ++- tox.ini | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/_pytest/python_api.py b/_pytest/python_api.py index b73e0457c3e..a3cf1c8cb06 100644 --- a/_pytest/python_api.py +++ b/_pytest/python_api.py @@ -217,7 +217,8 @@ def tolerance(self): absolute tolerance or a relative tolerance, depending on what the user specified or which would be larger. """ - def set_default(x, default): return x if x is not None else default + def set_default(x, default): + return x if x is not None else default # Figure out what the absolute tolerance should be. ``self.abs`` is # either None or a value specified by the user. diff --git a/tox.ini b/tox.ini index 9245ff4187e..33e5fa02cfa 100644 --- a/tox.ini +++ b/tox.ini @@ -213,3 +213,8 @@ filterwarnings = [flake8] max-line-length = 120 exclude = _pytest/vendored_packages/pluggy.py +ignore= + # do not use bare except' + E722 + # ambiguous variable name 'l' + E741 From fe560b7192ba76cb56fb6a91b7e68e401808571b Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 23 Oct 2017 18:46:14 -0200 Subject: [PATCH 23/27] Make CONTRIBUTING and PR template more consistent regarding doc contributions --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- CONTRIBUTING.rst | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index ad3fea61ef9..bf9fc199f59 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -12,4 +12,4 @@ Here's a quick checklist that should be present in PRs: Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please: -- [ ] Add yourself to `AUTHORS`; +- [ ] Add yourself to `AUTHORS`, in alphabetical order; diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 917ce3c338b..68db81398dd 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -177,7 +177,8 @@ Short version #. Write a ``changelog`` entry: ``changelog/2574.bugfix``, use issue id number and one of ``bugfix``, ``removal``, ``feature``, ``vendor``, ``doc`` or ``trivial`` for the issue type. -#. Add yourself to ``AUTHORS`` file if not there yet, in alphabetical order. +#. Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please + add yourself to the ``AUTHORS`` file, in alphabetical order; Long version From 14e3a5fcb927adac8d5a07679f6655a46ef6c0cd Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Tue, 24 Oct 2017 10:42:16 +0100 Subject: [PATCH 24/27] Move the generic separator to a constant --- _pytest/fixtures.py | 8 ++++---- _pytest/junitxml.py | 3 ++- _pytest/main.py | 9 +++++---- _pytest/nodes.py | 5 ++++- _pytest/terminal.py | 3 ++- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index 3a1321664d4..f71f3576834 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -8,6 +8,7 @@ from py._code.code import FormattedExcinfo import _pytest +from _pytest import nodes from _pytest._code.code import TerminalRepr from _pytest.compat import ( NOTSET, exc_clear, _format_args, @@ -17,7 +18,6 @@ safe_getattr, FuncargnamesCompatAttr, ) -from _pytest.nodes import ischildnode from _pytest.outcomes import fail, TEST_OUTCOME @@ -983,8 +983,8 @@ def pytest_plugin_registered(self, plugin): # by their test id) if p.basename.startswith("conftest.py"): nodeid = p.dirpath().relto(self.config.rootdir) - if p.sep != "/": - nodeid = nodeid.replace(p.sep, "/") + if p.sep != nodes.SEP: + nodeid = nodeid.replace(p.sep, nodes.SEP) self.parsefactories(plugin, nodeid) def _getautousenames(self, nodeid): @@ -1134,5 +1134,5 @@ def getfixturedefs(self, argname, nodeid): def _matchfactories(self, fixturedefs, nodeid): for fixturedef in fixturedefs: - if ischildnode(fixturedef.baseid, nodeid): + if nodes.ischildnode(fixturedef.baseid, nodeid): yield fixturedef diff --git a/_pytest/junitxml.py b/_pytest/junitxml.py index ed3ba2e9a53..7fb40dc3548 100644 --- a/_pytest/junitxml.py +++ b/_pytest/junitxml.py @@ -17,6 +17,7 @@ import sys import time import pytest +from _pytest import nodes from _pytest.config import filename_arg # Python 2.X and 3.X compatibility @@ -252,7 +253,7 @@ def mangle_test_address(address): except ValueError: pass # convert file path to dotted path - names[0] = names[0].replace("/", '.') + names[0] = names[0].replace(nodes.SEP, '.') names[0] = _py_ext_re.sub("", names[0]) # put any params back names[-1] += possible_open_bracket + params diff --git a/_pytest/main.py b/_pytest/main.py index 4bddf1e2d02..91083c85fa1 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -6,6 +6,7 @@ import sys import _pytest +from _pytest import nodes import _pytest._code import py try: @@ -14,8 +15,8 @@ from UserDict import DictMixin as MappingMixin from _pytest.config import directory_arg, UsageError, hookimpl -from _pytest.runner import collect_one_node from _pytest.outcomes import exit +from _pytest.runner import collect_one_node tracebackcutdir = py.path.local(_pytest.__file__).dirpath() @@ -516,14 +517,14 @@ def __init__(self, fspath, parent=None, config=None, session=None): rel = fspath.relto(parent.fspath) if rel: name = rel - name = name.replace(os.sep, "/") + name = name.replace(os.sep, nodes.SEP) super(FSCollector, self).__init__(name, parent, config, session) self.fspath = fspath def _makeid(self): relpath = self.fspath.relto(self.config.rootdir) - if os.sep != "/": - relpath = relpath.replace(os.sep, "/") + if os.sep != nodes.SEP: + relpath = relpath.replace(os.sep, nodes.SEP) return relpath diff --git a/_pytest/nodes.py b/_pytest/nodes.py index c28c631804e..7b6b695b527 100644 --- a/_pytest/nodes.py +++ b/_pytest/nodes.py @@ -1,6 +1,9 @@ import py +SEP = "/" + + def _splitnode(nodeid): """Split a nodeid into constituent 'parts'. @@ -19,7 +22,7 @@ def _splitnode(nodeid): if nodeid == '': # If there is no root node at all, return an empty list so the caller's logic can remain sane return [] - parts = nodeid.split(py.path.local.sep) + parts = nodeid.split(SEP) # Replace single last element 'test_foo.py::Bar::()' with multiple elements 'test_foo.py', 'Bar', '()' parts[-1:] = parts[-1].split("::") return parts diff --git a/_pytest/terminal.py b/_pytest/terminal.py index bb114391dce..f56b966f35d 100644 --- a/_pytest/terminal.py +++ b/_pytest/terminal.py @@ -13,6 +13,7 @@ import time import platform +from _pytest import nodes import _pytest._pluggy as pluggy @@ -452,7 +453,7 @@ def mkrel(nodeid): if fspath: res = mkrel(nodeid).replace("::()", "") # parens-normalization - if nodeid.split("::")[0] != fspath.replace("\\", "/"): + if nodeid.split("::")[0] != fspath.replace("\\", nodes.SEP): res += " <- " + self.startdir.bestrelpath(fspath) else: res = "[location]" From f5e72d2f5f9817a504a8e01e0de4dc89880a7621 Mon Sep 17 00:00:00 2001 From: Tom Dalton Date: Tue, 24 Oct 2017 11:25:42 +0100 Subject: [PATCH 25/27] Unused import / lint --- _pytest/nodes.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/_pytest/nodes.py b/_pytest/nodes.py index 7b6b695b527..ad3af2ce67c 100644 --- a/_pytest/nodes.py +++ b/_pytest/nodes.py @@ -1,6 +1,3 @@ -import py - - SEP = "/" From 6b86b0dbfea3895a1d16d7db970d3ea91de92ecc Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 24 Oct 2017 21:01:00 -0200 Subject: [PATCH 26/27] Fix additional linting issues --- _pytest/compat.py | 12 ++++-------- _pytest/pytester.py | 4 +--- _pytest/recwarn.py | 6 ++---- testing/logging/test_fixture.py | 2 +- 4 files changed, 8 insertions(+), 16 deletions(-) diff --git a/_pytest/compat.py b/_pytest/compat.py index 8499e888205..26330586172 100644 --- a/_pytest/compat.py +++ b/_pytest/compat.py @@ -14,7 +14,6 @@ import _pytest from _pytest.outcomes import TEST_OUTCOME - try: import enum except ImportError: # pragma: no cover @@ -110,11 +109,10 @@ def getfuncargnames(function, is_method=False, cls=None): # ordered mapping of parameter names to Parameter instances. This # creates a tuple of the names of the parameters that don't have # defaults. - arg_names = tuple( - p.name for p in signature(function).parameters.values() - if (p.kind is Parameter.POSITIONAL_OR_KEYWORD - or p.kind is Parameter.KEYWORD_ONLY) and - p.default is Parameter.empty) + arg_names = tuple(p.name for p in signature(function).parameters.values() + if (p.kind is Parameter.POSITIONAL_OR_KEYWORD or + p.kind is Parameter.KEYWORD_ONLY) and + p.default is Parameter.empty) # If this function should be treated as a bound method even though # it's passed as an unbound method or function, remove the first # parameter name. @@ -173,8 +171,6 @@ def ascii_escaped(val): STRING_TYPES = bytes, str, unicode UNICODE_TYPES = unicode, - from itertools import imap, izip # NOQA - def ascii_escaped(val): """In py2 bytes and str are the same type, so return if it's a bytes object, return it unchanged if it is a full ascii string, diff --git a/_pytest/pytester.py b/_pytest/pytester.py index 345a1acd0f4..a65e3f02704 100644 --- a/_pytest/pytester.py +++ b/_pytest/pytester.py @@ -23,9 +23,7 @@ from _pytest.assertion.rewrite import AssertionRewritingHook -PYTEST_FULLPATH = os.path.abspath( - pytest.__file__.rstrip("oc") - ).replace("$py.class", ".py") +PYTEST_FULLPATH = os.path.abspath(pytest.__file__.rstrip("oc")).replace("$py.class", ".py") def pytest_addoption(parser): diff --git a/_pytest/recwarn.py b/_pytest/recwarn.py index c9f86a4839a..4fceb10a7f3 100644 --- a/_pytest/recwarn.py +++ b/_pytest/recwarn.py @@ -232,7 +232,5 @@ def __exit__(self, *exc_info): else: fail("DID NOT WARN. No warnings of type {0} matching" " ('{1}') was emitted. The list of emitted warnings" - " is: {2}.".format( - self.expected_warning, - self.match_expr, - [each.message for each in self])) + " is: {2}.".format(self.expected_warning, self.match_expr, + [each.message for each in self])) diff --git a/testing/logging/test_fixture.py b/testing/logging/test_fixture.py index b5bee4233a5..c27b31137ff 100644 --- a/testing/logging/test_fixture.py +++ b/testing/logging/test_fixture.py @@ -3,7 +3,7 @@ logger = logging.getLogger(__name__) -sublogger = logging.getLogger(__name__+'.baz') +sublogger = logging.getLogger(__name__ + '.baz') def test_fixture_help(testdir): From 4e581b637f3082836a0c711b9fb62dac485cca79 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 24 Oct 2017 22:13:32 -0200 Subject: [PATCH 27/27] Use zip and map from six --- _pytest/compat.py | 2 -- _pytest/mark.py | 4 ++-- _pytest/python_api.py | 5 +++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/_pytest/compat.py b/_pytest/compat.py index 26330586172..7560fbec397 100644 --- a/_pytest/compat.py +++ b/_pytest/compat.py @@ -127,8 +127,6 @@ def getfuncargnames(function, is_method=False, cls=None): if _PY3: - imap = map - izip = zip STRING_TYPES = bytes, str UNICODE_TYPES = str, diff --git a/_pytest/mark.py b/_pytest/mark.py index f4058989a09..03b058d95b9 100644 --- a/_pytest/mark.py +++ b/_pytest/mark.py @@ -5,7 +5,7 @@ import warnings from collections import namedtuple from operator import attrgetter -from .compat import imap +from six.moves import map from .deprecated import MARK_PARAMETERSET_UNPACKING @@ -427,7 +427,7 @@ def add_mark(self, mark): def __iter__(self): """ yield MarkInfo objects each relating to a marking-call. """ - return imap(MarkInfo, self._marks) + return map(MarkInfo, self._marks) MARK_GEN = MarkGenerator() diff --git a/_pytest/python_api.py b/_pytest/python_api.py index 6ae3e81b696..bf1cd147e06 100644 --- a/_pytest/python_api.py +++ b/_pytest/python_api.py @@ -2,8 +2,9 @@ import sys import py +from six.moves import zip -from _pytest.compat import isclass, izip +from _pytest.compat import isclass from _pytest.outcomes import fail import _pytest._code @@ -145,7 +146,7 @@ def __eq__(self, actual): return ApproxBase.__eq__(self, actual) def _yield_comparisons(self, actual): - return izip(actual, self.expected) + return zip(actual, self.expected) class ApproxScalar(ApproxBase):