From b62fd79c0cfb6caeb6f620a033415bf0571f0f3c Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Wed, 6 Dec 2017 21:55:04 +0000 Subject: [PATCH 01/16] Fix issue 2985. --- AUTHORS | 1 + _pytest/main.py | 19 +++++++++++ changelog/2985.bugfix | 1 + testing/acceptance_test.py | 65 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+) create mode 100644 changelog/2985.bugfix diff --git a/AUTHORS b/AUTHORS index 44ae6aa43..0492a1659 100644 --- a/AUTHORS +++ b/AUTHORS @@ -74,6 +74,7 @@ Grig Gheorghiu Grigorii Eremeev (budulianin) Guido Wesdorp Harald Armin Massa +Henk-Jaap Wagenaar Hugo van Kemenade Hui Wang (coldnight) Ian Bicking diff --git a/_pytest/main.py b/_pytest/main.py index 25554098d..cb3ba4b8a 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -729,6 +729,25 @@ class Session(FSCollector): """ import pkgutil + + if six.PY2: # python 3.4+ uses importlib instead + def find_module_patched(self, fullname, path=None): + subname = fullname.split(".")[-1] + if subname != fullname and self.path is None: + return None + if self.path is None: + path = None + else: + path = [self.path] + try: + file, filename, etc = pkgutil.imp.find_module(subname, + path) + except ImportError: + return None + return pkgutil.ImpLoader(fullname, file, filename, etc) + + pkgutil.ImpImporter.find_module = find_module_patched + try: loader = pkgutil.find_loader(x) except ImportError: diff --git a/changelog/2985.bugfix b/changelog/2985.bugfix new file mode 100644 index 000000000..1a268c0c5 --- /dev/null +++ b/changelog/2985.bugfix @@ -0,0 +1 @@ +Fix conversion of pyargs to filename to not convert symlinks and not use deprecated features on Python 3. diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index a7838545b..fe55c5f8b 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -3,6 +3,8 @@ from __future__ import absolute_import, division, print_function import os import sys +import six + import _pytest._code import py import pytest @@ -645,6 +647,69 @@ class TestInvocationVariants(object): "*1 passed*" ]) + @pytest.mark.skipif(not hasattr(os, "symlink"), reason="requires symlinks") + def test_cmdline_python_package_symlink(self, testdir, monkeypatch): + """ + test --pyargs option with packages with path containing symlink can + have conftest.py in their package (#2985) + """ + monkeypatch.delenv('PYTHONDONTWRITEBYTECODE', raising=False) + + search_path = ["lib", os.path.join("local", "lib")] + + dirname = "lib" + d = testdir.mkdir(dirname) + foo = d.mkdir("foo") + foo.ensure("__init__.py") + lib = foo.mkdir('bar') + lib.ensure("__init__.py") + lib.join("test_bar.py"). \ + write("def test_bar(): pass\n" + "def test_other(a_fixture):pass") + lib.join("conftest.py"). \ + write("import pytest\n" + "@pytest.fixture\n" + "def a_fixture():pass") + + d_local = testdir.mkdir("local") + symlink_location = os.path.join(str(d_local), "lib") + if six.PY2: + os.symlink(str(d), symlink_location) + else: + os.symlink(str(d), symlink_location, target_is_directory=True) + + # The structure of the test directory is now: + # . + # ├── local + # │ └── lib -> ../world + # └── lib + # └── foo + # ├── __init__.py + # └── bar + # ├── __init__.py + # ├── conftest.py + # └── test_world.py + + def join_pythonpath(*dirs): + cur = py.std.os.environ.get('PYTHONPATH') + if cur: + dirs += (cur,) + return os.pathsep.join(str(p) for p in dirs) + + monkeypatch.setenv('PYTHONPATH', join_pythonpath(*search_path)) + for p in search_path: + monkeypatch.syspath_prepend(p) + + # module picked up in symlink-ed directory: + result = testdir.runpytest("--pyargs", "-v", "foo.bar") + testdir.chdir() + assert result.ret == 0 + result.stdout.fnmatch_lines([ + "*lib/foo/bar/test_bar.py::test_bar*PASSED*", + "*lib/foo/bar/test_bar.py::test_other*PASSED*", + "*2 passed*" + ]) + def test_cmdline_python_package_not_exists(self, testdir): result = testdir.runpytest("--pyargs", "tpkgwhatv") assert result.ret From ba209b52305a99cb5642716510718feb7f98f829 Mon Sep 17 00:00:00 2001 From: Thomas Hisch Date: Thu, 7 Dec 2017 15:35:23 +0100 Subject: [PATCH 02/16] Integrate logging_using_handler into catching_logs logging_using_handler is only used in catching_logs. Therefore it makes sense to remove one of the many context managers from the logging plugin. --- _pytest/logging.py | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/_pytest/logging.py b/_pytest/logging.py index ed4db25ad..b29f48503 100644 --- a/_pytest/logging.py +++ b/_pytest/logging.py @@ -78,23 +78,6 @@ def pytest_addoption(parser): help='log date format as used by the logging module.') -@contextmanager -def logging_using_handler(handler, logger=None): - """Context manager that safely registers a given handler.""" - logger = logger or logging.getLogger(logger) - - if handler in logger.handlers: # reentrancy - # Adding the same handler twice would confuse logging system. - # Just don't do that. - yield - else: - logger.addHandler(handler) - try: - yield - finally: - logger.removeHandler(handler) - - @contextmanager def catching_logs(handler, formatter=None, level=logging.NOTSET, logger=None): @@ -105,13 +88,20 @@ def catching_logs(handler, formatter=None, handler.setFormatter(formatter) handler.setLevel(level) - with logging_using_handler(handler, logger): - orig_level = logger.level - logger.setLevel(min(orig_level, level)) - try: - yield handler - finally: - logger.setLevel(orig_level) + # Adding the same handler twice would confuse logging system. + # Just don't do that. + add_new_handler = handler not in logger.handlers + + if add_new_handler: + logger.addHandler(handler) + orig_level = logger.level + logger.setLevel(min(orig_level, level)) + try: + yield handler + finally: + logger.setLevel(orig_level) + if add_new_handler: + logger.removeHandler(handler) class LogCaptureHandler(logging.StreamHandler): From dc196242486dc52123ca086a4cfd9b6e46f2352a Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 08:26:40 +0000 Subject: [PATCH 03/16] Improve testing comments and code of issue 2985 (symlink in pyargs). --- testing/acceptance_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index fe55c5f8b..9678f6914 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -681,17 +681,17 @@ class TestInvocationVariants(object): # The structure of the test directory is now: # . # ├── local - # │ └── lib -> ../world + # │ └── lib -> ../lib # └── lib # └── foo # ├── __init__.py # └── bar # ├── __init__.py # ├── conftest.py - # └── test_world.py + # └── test_bar.py def join_pythonpath(*dirs): - cur = py.std.os.environ.get('PYTHONPATH') + cur = os.getenv('PYTHONPATH') if cur: dirs += (cur,) return os.pathsep.join(str(p) for p in dirs) From 3ca1e4b7f056bb054b0394bcfb45dc2c78ffa364 Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 08:27:08 +0000 Subject: [PATCH 04/16] Make patch for issue in pkgutil.ImpImporter local by using contextmanager. --- _pytest/main.py | 61 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/_pytest/main.py b/_pytest/main.py index cb3ba4b8a..0866a838e 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -1,8 +1,10 @@ """ core implementation of testing process: init, session, runtest loop. """ from __future__ import absolute_import, division, print_function +import contextlib import functools import os +import pkgutil import six import sys @@ -728,28 +730,48 @@ class Session(FSCollector): """Convert a dotted module name to path. """ - import pkgutil + @contextlib.contextmanager + def _patched_find_module(): + """Patch bug in pkgutil.ImpImporter.find_module - if six.PY2: # python 3.4+ uses importlib instead - def find_module_patched(self, fullname, path=None): - subname = fullname.split(".")[-1] - if subname != fullname and self.path is None: - return None - if self.path is None: - path = None - else: - path = [self.path] + When using pkgutil.find_loader on python<3.4 it removes symlinks + from the path due to a call to os.path.realpath. This is not consistent + with actually doing the import (in these versions, pkgutil and __import__ + did not share the same underlying code). This can break conftest + discovery for pytest where symlinks are involved. + + The only supported python<3.4 by pytest is python 2.7. + """ + if six.PY2: # python 3.4+ uses importlib instead + def find_module_patched(self, fullname, path=None): + # Note: we ignore 'path' argument since it is only used via meta_path + subname = fullname.split(".")[-1] + if subname != fullname and self.path is None: + return None + if self.path is None: + path = None + else: + # original: path = [os.path.realpath(self.path)] + path = [self.path] + try: + file, filename, etc = pkgutil.imp.find_module(subname, + path) + except ImportError: + return None + return pkgutil.ImpLoader(fullname, file, filename, etc) + + old_find_module = pkgutil.ImpImporter.find_module + pkgutil.ImpImporter.find_module = find_module_patched try: - file, filename, etc = pkgutil.imp.find_module(subname, - path) - except ImportError: - return None - return pkgutil.ImpLoader(fullname, file, filename, etc) - - pkgutil.ImpImporter.find_module = find_module_patched + yield + finally: + pkgutil.ImpImporter.find_module = old_find_module + else: + yield try: - loader = pkgutil.find_loader(x) + with _patched_find_module(): + loader = pkgutil.find_loader(x) except ImportError: return x if loader is None: @@ -757,7 +779,8 @@ class Session(FSCollector): # This method is sometimes invoked when AssertionRewritingHook, which # does not define a get_filename method, is already in place: try: - path = loader.get_filename(x) + with _patched_find_module(): + path = loader.get_filename(x) except AttributeError: # Retrieve path from AssertionRewritingHook: path = loader.modules[x][0].co_filename From 1e295535c376599511bfac4521ace951af645b52 Mon Sep 17 00:00:00 2001 From: Henk-Jaap Wagenaar Date: Tue, 12 Dec 2017 09:53:06 +0000 Subject: [PATCH 05/16] Move _patched_find_module to module namespace. --- _pytest/main.py | 78 +++++++++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 38 deletions(-) diff --git a/_pytest/main.py b/_pytest/main.py index 0866a838e..142240c99 100644 --- a/_pytest/main.py +++ b/_pytest/main.py @@ -208,6 +208,46 @@ def pytest_ignore_collect(path, config): return False +@contextlib.contextmanager +def _patched_find_module(): + """Patch bug in pkgutil.ImpImporter.find_module + + When using pkgutil.find_loader on python<3.4 it removes symlinks + from the path due to a call to os.path.realpath. This is not consistent + with actually doing the import (in these versions, pkgutil and __import__ + did not share the same underlying code). This can break conftest + discovery for pytest where symlinks are involved. + + The only supported python<3.4 by pytest is python 2.7. + """ + if six.PY2: # python 3.4+ uses importlib instead + def find_module_patched(self, fullname, path=None): + # Note: we ignore 'path' argument since it is only used via meta_path + subname = fullname.split(".")[-1] + if subname != fullname and self.path is None: + return None + if self.path is None: + path = None + else: + # original: path = [os.path.realpath(self.path)] + path = [self.path] + try: + file, filename, etc = pkgutil.imp.find_module(subname, + path) + except ImportError: + return None + return pkgutil.ImpLoader(fullname, file, filename, etc) + + old_find_module = pkgutil.ImpImporter.find_module + pkgutil.ImpImporter.find_module = find_module_patched + try: + yield + finally: + pkgutil.ImpImporter.find_module = old_find_module + else: + yield + + class FSHookProxy: def __init__(self, fspath, pm, remove_mods): self.fspath = fspath @@ -730,44 +770,6 @@ class Session(FSCollector): """Convert a dotted module name to path. """ - @contextlib.contextmanager - def _patched_find_module(): - """Patch bug in pkgutil.ImpImporter.find_module - - When using pkgutil.find_loader on python<3.4 it removes symlinks - from the path due to a call to os.path.realpath. This is not consistent - with actually doing the import (in these versions, pkgutil and __import__ - did not share the same underlying code). This can break conftest - discovery for pytest where symlinks are involved. - - The only supported python<3.4 by pytest is python 2.7. - """ - if six.PY2: # python 3.4+ uses importlib instead - def find_module_patched(self, fullname, path=None): - # Note: we ignore 'path' argument since it is only used via meta_path - subname = fullname.split(".")[-1] - if subname != fullname and self.path is None: - return None - if self.path is None: - path = None - else: - # original: path = [os.path.realpath(self.path)] - path = [self.path] - try: - file, filename, etc = pkgutil.imp.find_module(subname, - path) - except ImportError: - return None - return pkgutil.ImpLoader(fullname, file, filename, etc) - - old_find_module = pkgutil.ImpImporter.find_module - pkgutil.ImpImporter.find_module = find_module_patched - try: - yield - finally: - pkgutil.ImpImporter.find_module = old_find_module - else: - yield try: with _patched_find_module(): From e4da9bacdfd6c787786b5971491455e2e8c0029a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jurko=20Gospodneti=C4=87?= Date: Tue, 12 Dec 2017 12:55:32 +0100 Subject: [PATCH 06/16] fix `actial` --> `actual` typo --- tasks/generate.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/generate.py b/tasks/generate.py index fa8ee6557..5aa4752f5 100644 --- a/tasks/generate.py +++ b/tasks/generate.py @@ -151,7 +151,7 @@ def publish_release(ctx, version, user, pypi_name): @invoke.task(help={ 'version': 'version being released', - 'write_out': 'write changes to the actial changelog' + 'write_out': 'write changes to the actual changelog' }) def changelog(ctx, version, write_out=False): if write_out: From a4f4579f19f73cf099f4ca51b6e8182bcdf02134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jurko=20Gospodneti=C4=87?= Date: Tue, 12 Dec 2017 13:41:31 +0100 Subject: [PATCH 07/16] add changelog entry --- changelog/3021.trivial | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/3021.trivial diff --git a/changelog/3021.trivial b/changelog/3021.trivial new file mode 100644 index 000000000..9716d356d --- /dev/null +++ b/changelog/3021.trivial @@ -0,0 +1 @@ +code cleanup From 7b5d4d01ede1822820a921209f87771b1f14b652 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 12 Dec 2017 18:01:31 -0200 Subject: [PATCH 08/16] Add param annotations and types to hookspec Also mention which hook to use instead of the deprecated pytest_cmdline_preparse Fix #3022 --- _pytest/hookspec.py | 125 ++++++++++++++++++++++++++++--------- doc/en/writing_plugins.rst | 9 +++ 2 files changed, 104 insertions(+), 30 deletions(-) diff --git a/_pytest/hookspec.py b/_pytest/hookspec.py index 440bf99d3..f004dd097 100644 --- a/_pytest/hookspec.py +++ b/_pytest/hookspec.py @@ -12,22 +12,31 @@ hookspec = HookspecMarker("pytest") @hookspec(historic=True) def pytest_addhooks(pluginmanager): """called at plugin registration time to allow adding new hooks via a call to - pluginmanager.add_hookspecs(module_or_class, prefix).""" + ``pluginmanager.add_hookspecs(module_or_class, prefix)``. + + + :param _pytest.config.PytestPluginManager pluginmanager: pytest plugin manager + """ @hookspec(historic=True) def pytest_namespace(): """ - DEPRECATED: this hook causes direct monkeypatching on pytest, its use is strongly discouraged + (**Deprecated**) this hook causes direct monkeypatching on pytest, its use is strongly discouraged return dict of name->object to be made globally available in - the pytest namespace. This hook is called at plugin registration - time. + the pytest namespace. + + This hook is called at plugin registration time. """ @hookspec(historic=True) def pytest_plugin_registered(plugin, manager): - """ a new pytest plugin got registered. """ + """ a new pytest plugin got registered. + + :param plugin: the plugin module or instance + :param _pytest.config.PytestPluginManager manager: pytest plugin manager + """ @hookspec(historic=True) @@ -41,7 +50,7 @@ def pytest_addoption(parser): files situated at the tests root directory due to how pytest :ref:`discovers plugins during startup `. - :arg parser: To add command line options, call + :arg _pytest.config.Parser parser: To add command line options, call :py:func:`parser.addoption(...) <_pytest.config.Parser.addoption>`. To add ini-file values call :py:func:`parser.addini(...) <_pytest.config.Parser.addini>`. @@ -56,8 +65,7 @@ def pytest_addoption(parser): a value read from an ini-style file. The config object is passed around on many internal objects via the ``.config`` - attribute or can be retrieved as the ``pytestconfig`` fixture or accessed - via (deprecated) ``pytest.config``. + attribute or can be retrieved as the ``pytestconfig`` fixture. """ @@ -72,8 +80,7 @@ def pytest_configure(config): After that, the hook is called for other conftest files as they are imported. - :arg config: pytest config object - :type config: _pytest.config.Config + :arg _pytest.config.Config config: pytest config object """ # ------------------------------------------------------------------------- @@ -87,11 +94,22 @@ def pytest_configure(config): def pytest_cmdline_parse(pluginmanager, args): """return initialized config object, parsing the specified args. - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult` + + :param _pytest.config.PytestPluginManager pluginmanager: pytest plugin manager + :param list[str] args: list of arguments passed on the command line + """ def pytest_cmdline_preparse(config, args): - """(deprecated) modify command line arguments before option parsing. """ + """(**Deprecated**) modify command line arguments before option parsing. + + This hook is considered deprecated and will be removed in a future pytest version. Consider + using :func:`pytest_load_initial_conftests` instead. + + :param _pytest.config.Config config: pytest config object + :param list[str] args: list of arguments passed on the command line + """ @hookspec(firstresult=True) @@ -99,12 +117,20 @@ def pytest_cmdline_main(config): """ called for performing the main command line action. The default implementation will invoke the configure hooks and runtest_mainloop. - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult` + + :param _pytest.config.Config config: pytest config object + """ def pytest_load_initial_conftests(early_config, parser, args): """ implements the loading of initial conftest files ahead - of command line option parsing. """ + of command line option parsing. + + :param _pytest.config.Config early_config: pytest config object + :param list[str] args: list of arguments passed on the command line + :param _pytest.config.Parser parser: to add command line options + """ # ------------------------------------------------------------------------- @@ -113,18 +139,29 @@ def pytest_load_initial_conftests(early_config, parser, args): @hookspec(firstresult=True) def pytest_collection(session): - """ perform the collection protocol for the given session. + """Perform the collection protocol for the given session. - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult`. + + :param _pytest.main.Session session: the pytest session object + """ def pytest_collection_modifyitems(session, config, items): """ called after collection has been performed, may filter or re-order - the items in-place.""" + the items in-place. + + :param _pytest.main.Session session: the pytest session object + :param _pytest.config.Config config: pytest config object + :param List[_pytest.main.Item] items: list of item objects + """ def pytest_collection_finish(session): - """ called after collection has been performed and modified. """ + """ called after collection has been performed and modified. + + :param _pytest.main.Session session: the pytest session object + """ @hookspec(firstresult=True) @@ -134,6 +171,9 @@ def pytest_ignore_collect(path, config): more specific hooks. Stops at first non-None result, see :ref:`firstresult` + + :param str path: the path to analyze + :param _pytest.config.Config config: pytest config object """ @@ -141,12 +181,18 @@ def pytest_ignore_collect(path, config): def pytest_collect_directory(path, parent): """ called before traversing a directory for collection files. - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult` + + :param str path: the path to analyze + """ def pytest_collect_file(path, parent): """ return collection Node or None for the given path. Any new node - needs to have the specified ``parent`` as a parent.""" + needs to have the specified ``parent`` as a parent. + + :param str path: the path to collect + """ # logging hooks for collection @@ -212,7 +258,12 @@ def pytest_make_parametrize_id(config, val, argname): by @pytest.mark.parametrize calls. Return None if the hook doesn't know about ``val``. The parameter name is available as ``argname``, if required. - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult` + + :param _pytest.config.Config config: pytest config object + :param val: the parametrized value + :param str argname: the automatic parameter name produced by pytest + """ # ------------------------------------------------------------------------- # generic runtest related hooks @@ -224,11 +275,14 @@ def pytest_runtestloop(session): """ called for performing the main runtest loop (after collection finished). - Stops at first non-None result, see :ref:`firstresult` """ + Stops at first non-None result, see :ref:`firstresult` + + :param _pytest.main.Session session: the pytest session object + """ def pytest_itemstart(item, node): - """ (deprecated, use pytest_runtest_logstart). """ + """(**Deprecated**) use pytest_runtest_logstart. """ @hookspec(firstresult=True) @@ -307,15 +361,25 @@ def pytest_fixture_post_finalizer(fixturedef, request): def pytest_sessionstart(session): - """ before session.main() is called. """ + """ before session.main() is called. + + :param _pytest.main.Session session: the pytest session object + """ def pytest_sessionfinish(session, exitstatus): - """ whole test run finishes. """ + """ whole test run finishes. + + :param _pytest.main.Session session: the pytest session object + :param int exitstatus: the status which pytest will return to the system + """ def pytest_unconfigure(config): - """ called before test process is exited. """ + """ called before test process is exited. + + :param _pytest.config.Config config: pytest config object + """ # ------------------------------------------------------------------------- @@ -329,6 +393,8 @@ def pytest_assertrepr_compare(config, op, left, right): of strings. The strings will be joined by newlines but any newlines *in* a string will be escaped. Note that all but the first line will be indented slightly, the intention is for the first line to be a summary. + + :param _pytest.config.Config config: pytest config object """ # ------------------------------------------------------------------------- @@ -339,7 +405,7 @@ def pytest_assertrepr_compare(config, op, left, right): def pytest_report_header(config, startdir): """ return a string or list of strings to be displayed as header info for terminal reporting. - :param config: the pytest config object. + :param _pytest.config.Config config: pytest config object :param startdir: py.path object with the starting dir .. note:: @@ -358,7 +424,7 @@ def pytest_report_collectionfinish(config, startdir, items): This strings will be displayed after the standard "collected X items" message. - :param config: the pytest config object. + :param _pytest.config.Config config: pytest config object :param startdir: py.path object with the starting dir :param items: list of pytest items that are going to be executed; this list should not be modified. """ @@ -418,6 +484,5 @@ def pytest_enter_pdb(config): """ called upon pdb.set_trace(), can be used by plugins to take special action just before the python debugger enters in interactive mode. - :arg config: pytest config object - :type config: _pytest.config.Config + :param _pytest.config.Config config: pytest config object """ diff --git a/doc/en/writing_plugins.rst b/doc/en/writing_plugins.rst index eb5255830..8320d2c6a 100644 --- a/doc/en/writing_plugins.rst +++ b/doc/en/writing_plugins.rst @@ -616,6 +616,7 @@ Collection hooks ``pytest`` calls the following hooks for collecting files and directories: +.. autofunction:: pytest_collection .. autofunction:: pytest_ignore_collect .. autofunction:: pytest_collect_directory .. autofunction:: pytest_collect_file @@ -687,6 +688,14 @@ Reference of objects involved in hooks :members: :show-inheritance: +.. autoclass:: _pytest.main.FSCollector() + :members: + :show-inheritance: + +.. autoclass:: _pytest.main.Session() + :members: + :show-inheritance: + .. autoclass:: _pytest.main.Item() :members: :show-inheritance: From c8e7d1ae349343f17b62d84aa0b3aef50393af7e Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Tue, 12 Dec 2017 17:39:10 -0800 Subject: [PATCH 09/16] Respect PYTEST_DONT_REWRITE for plugins too. --- _pytest/assertion/rewrite.py | 14 ++++++++------ _pytest/pytester.py | 2 +- changelog/2995.bugfix | 2 ++ testing/test_assertrewrite.py | 10 ++++++++++ 4 files changed, 21 insertions(+), 7 deletions(-) create mode 100644 changelog/2995.bugfix diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index 92deb6539..85fbd58e1 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -179,11 +179,12 @@ class AssertionRewritingHook(object): The named module or package as well as any nested modules will be rewritten on import. """ - already_imported = set(names).intersection(set(sys.modules)) - if already_imported: - for name in already_imported: - if name not in self._rewritten_names: - self._warn_already_imported(name) + already_imported = ( + (set(names) & set(sys.modules)) - set(self._rewritten_names)) + for name in already_imported: + if not AssertionRewriter.is_rewrite_disabled( + sys.modules[name].__doc__ or ""): + self._warn_already_imported(name) self._must_rewrite.update(names) def _warn_already_imported(self, name): @@ -635,7 +636,8 @@ class AssertionRewriter(ast.NodeVisitor): not isinstance(field, ast.expr)): nodes.append(field) - def is_rewrite_disabled(self, docstring): + @staticmethod + def is_rewrite_disabled(docstring): return "PYTEST_DONT_REWRITE" in docstring def variable(self): diff --git a/_pytest/pytester.py b/_pytest/pytester.py index 0b25d839b..ee7ca24cd 100644 --- a/_pytest/pytester.py +++ b/_pytest/pytester.py @@ -347,7 +347,7 @@ class RunResult: :stdout: :py:class:`LineMatcher` of stdout, use ``stdout.str()`` to reconstruct stdout or the commonly used ``stdout.fnmatch_lines()`` method - :stderrr: :py:class:`LineMatcher` of stderr + :stderr: :py:class:`LineMatcher` of stderr :duration: duration in seconds """ diff --git a/changelog/2995.bugfix b/changelog/2995.bugfix new file mode 100644 index 000000000..978211824 --- /dev/null +++ b/changelog/2995.bugfix @@ -0,0 +1,2 @@ +PYTEST_DONT_REWRITE is now checked for plugins too (rather than only for +test modules). diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 0e22c6dac..77cfd2900 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -128,6 +128,16 @@ class TestAssertionRewrite(object): assert len(m.body) == 1 assert m.body[0].msg is None + def test_dont_rewrite_plugin(self, testdir): + contents = { + "conftest.py": "pytest_plugins = 'plugin'; import plugin", + "plugin.py": "'PYTEST_DONT_REWRITE'", + "test_foo.py": "def test_foo(): pass", + } + testdir.makepyfile(**contents) + result = testdir.runpytest_subprocess() + assert "warnings" not in "".join(result.outlines) + def test_name(self): def f(): assert False From 8ce6e39b1c50250c13f610ce0903c874f9c14d8e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 13 Dec 2017 06:52:37 -0200 Subject: [PATCH 10/16] Small formatting to CHANGELOG --- changelog/2995.bugfix | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/changelog/2995.bugfix b/changelog/2995.bugfix index 978211824..7a3dde4c8 100644 --- a/changelog/2995.bugfix +++ b/changelog/2995.bugfix @@ -1,2 +1 @@ -PYTEST_DONT_REWRITE is now checked for plugins too (rather than only for -test modules). +``PYTEST_DONT_REWRITE`` is now checked for plugins too rather than only for test modules. From 45e7734b1a68b430538ab51f02bc376ebe2731c3 Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Wed, 13 Dec 2017 00:54:57 -0800 Subject: [PATCH 11/16] Change set ops to use methods instead of operators. --- _pytest/assertion/rewrite.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/_pytest/assertion/rewrite.py b/_pytest/assertion/rewrite.py index 85fbd58e1..db3674930 100644 --- a/_pytest/assertion/rewrite.py +++ b/_pytest/assertion/rewrite.py @@ -179,8 +179,9 @@ class AssertionRewritingHook(object): The named module or package as well as any nested modules will be rewritten on import. """ - already_imported = ( - (set(names) & set(sys.modules)) - set(self._rewritten_names)) + already_imported = (set(names) + .intersection(sys.modules) + .difference(self._rewritten_names)) for name in already_imported: if not AssertionRewriter.is_rewrite_disabled( sys.modules[name].__doc__ or ""): From ebfc1c49d11211b00bd0d5ea8e0d8d520c0a6d78 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 13 Dec 2017 06:58:07 -0200 Subject: [PATCH 12/16] Fix changelog formatting --- changelog/3021.trivial | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/3021.trivial b/changelog/3021.trivial index 9716d356d..a63ba06e6 100644 --- a/changelog/3021.trivial +++ b/changelog/3021.trivial @@ -1 +1 @@ -code cleanup +Code cleanup. From 3862b0b28d1dac1d9ea398fc6155ed5e6de5498c Mon Sep 17 00:00:00 2001 From: Thomas Hisch Date: Wed, 13 Dec 2017 21:05:32 +0100 Subject: [PATCH 13/16] Remove logger parameter from catching_logs The logger parameter of catching_logs is not used anywhere. The main motivation for removing the logger parameter is that it removes the logger = logger or logging.getLogger(logger) line. IMO there are too many occurences of the string 'logg' ;) --- _pytest/logging.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/_pytest/logging.py b/_pytest/logging.py index b29f48503..9e82e801d 100644 --- a/_pytest/logging.py +++ b/_pytest/logging.py @@ -79,10 +79,9 @@ def pytest_addoption(parser): @contextmanager -def catching_logs(handler, formatter=None, - level=logging.NOTSET, logger=None): +def catching_logs(handler, formatter=None, level=logging.NOTSET): """Context manager that prepares the whole logging machinery properly.""" - logger = logger or logging.getLogger(logger) + root_logger = logging.getLogger() if formatter is not None: handler.setFormatter(formatter) @@ -90,18 +89,18 @@ def catching_logs(handler, formatter=None, # Adding the same handler twice would confuse logging system. # Just don't do that. - add_new_handler = handler not in logger.handlers + add_new_handler = handler not in root_logger.handlers if add_new_handler: - logger.addHandler(handler) - orig_level = logger.level - logger.setLevel(min(orig_level, level)) + root_logger.addHandler(handler) + orig_level = root_logger.level + root_logger.setLevel(min(orig_level, level)) try: yield handler finally: - logger.setLevel(orig_level) + root_logger.setLevel(orig_level) if add_new_handler: - logger.removeHandler(handler) + root_logger.removeHandler(handler) class LogCaptureHandler(logging.StreamHandler): From c3f63ac1432044271b15c4ef7959000a7a0d9bfe Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 13 Dec 2017 19:37:45 -0200 Subject: [PATCH 14/16] Fix memory leak caused by fixture values never been garbage collected The leak was caused by the (unused) `FixtureRequest._fixture_values` cache. This was introduced because the `partial` object (created to call FixtureDef.finish() bound with the Request) is kept alive through the entire session when a function-scoped fixture depends on a session-scoped (or higher) fixture because of the nested `addfinalizer` calls. FixtureDef.finish() started receiving a request object in order to obtain the proper hook proxy object (#2127), but this does not seem useful at all in practice because `pytest_fixture_post_finalizer` will be called with the `request` object of the moment the fixture value was *created*, not the request object active when the fixture value is being destroyed. We should probably deprecate/remove the request parameter from `pytest_fixture_post_finalizer`. Fix #2981 --- _pytest/fixtures.py | 17 ++++++++------- changelog/2981.bugfix | 1 + testing/acceptance_test.py | 43 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 7 deletions(-) create mode 100644 changelog/2981.bugfix diff --git a/_pytest/fixtures.py b/_pytest/fixtures.py index e09ffaddb..a7a63f505 100644 --- a/_pytest/fixtures.py +++ b/_pytest/fixtures.py @@ -267,7 +267,6 @@ class FixtureRequest(FuncargnamesCompatAttr): self.fixturename = None #: Scope string, one of "function", "class", "module", "session" self.scope = "function" - self._fixture_values = {} # argname -> fixture value self._fixture_defs = {} # argname -> FixtureDef fixtureinfo = pyfuncitem._fixtureinfo self._arg2fixturedefs = fixtureinfo.name2fixturedefs.copy() @@ -450,8 +449,7 @@ class FixtureRequest(FuncargnamesCompatAttr): raise # remove indent to prevent the python3 exception # from leaking into the call - result = self._getfixturevalue(fixturedef) - self._fixture_values[argname] = result + self._compute_fixture_value(fixturedef) self._fixture_defs[argname] = fixturedef return fixturedef @@ -466,7 +464,14 @@ class FixtureRequest(FuncargnamesCompatAttr): values.append(fixturedef) current = current._parent_request - def _getfixturevalue(self, fixturedef): + def _compute_fixture_value(self, fixturedef): + """ + Creates a SubRequest based on "self" and calls the execute method of the given fixturedef object. This will + force the FixtureDef object to throw away any previous results and compute a new fixture value, which + will be stored into the FixtureDef object itself. + + :param FixtureDef fixturedef: + """ # prepare a subrequest object before calling fixture function # (latter managed by fixturedef) argname = fixturedef.argname @@ -515,12 +520,11 @@ class FixtureRequest(FuncargnamesCompatAttr): exc_clear() try: # call the fixture function - val = fixturedef.execute(request=subrequest) + fixturedef.execute(request=subrequest) finally: # if fixture function failed it might have registered finalizers self.session._setupstate.addfinalizer(functools.partial(fixturedef.finish, request=subrequest), subrequest.node) - return val def _check_scope(self, argname, invoking_scope, requested_scope): if argname == "request": @@ -573,7 +577,6 @@ class SubRequest(FixtureRequest): self.scope = scope self._fixturedef = fixturedef self._pyfuncitem = request._pyfuncitem - self._fixture_values = request._fixture_values self._fixture_defs = request._fixture_defs self._arg2fixturedefs = request._arg2fixturedefs self._arg2index = request._arg2index diff --git a/changelog/2981.bugfix b/changelog/2981.bugfix new file mode 100644 index 000000000..fd6dde37a --- /dev/null +++ b/changelog/2981.bugfix @@ -0,0 +1 @@ +Fix **memory leak** where objects returned by fixtures were never destructed by the garbage collector. diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 9da9091db..4480fc2cf 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -913,3 +913,46 @@ def test_deferred_hook_checking(testdir): }) result = testdir.runpytest() result.stdout.fnmatch_lines(['* 1 passed *']) + + +def test_fixture_values_leak(testdir): + """Ensure that fixture objects are properly destroyed by the garbage collector at the end of their expected + life-times (#2981). + """ + testdir.makepyfile(""" + import attr + import gc + import pytest + import weakref + + @attr.s + class SomeObj(object): + name = attr.ib() + + fix_of_test1_ref = None + session_ref = None + + @pytest.fixture(scope='session') + def session_fix(): + global session_ref + obj = SomeObj(name='session-fixture') + session_ref = weakref.ref(obj) + return obj + + @pytest.fixture + def fix(session_fix): + global fix_of_test1_ref + obj = SomeObj(name='local-fixture') + fix_of_test1_ref = weakref.ref(obj) + return obj + + def test1(fix): + assert fix_of_test1_ref() is fix + + def test2(): + gc.collect() + # fixture "fix" created during test1 must have been destroyed by now + assert fix_of_test1_ref() is None + """) + result = testdir.runpytest() + result.stdout.fnmatch_lines(['* 2 passed *']) From 370daf0441dd70dc082490de80ead536c82566c0 Mon Sep 17 00:00:00 2001 From: Segev Finer Date: Sat, 16 Dec 2017 14:52:30 +0200 Subject: [PATCH 15/16] Use classic console output when -s is used Fixes #3038 --- _pytest/terminal.py | 3 ++- changelog/3038.feature | 1 + testing/test_terminal.py | 8 ++++++++ 3 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 changelog/3038.feature diff --git a/_pytest/terminal.py b/_pytest/terminal.py index 1aba5e845..b20bf5ea8 100644 --- a/_pytest/terminal.py +++ b/_pytest/terminal.py @@ -153,7 +153,8 @@ class TerminalReporter: self.hasmarkup = self._tw.hasmarkup self.isatty = file.isatty() self._progress_items_reported = 0 - self._show_progress_info = self.config.getini('console_output_style') == 'progress' + self._show_progress_info = (self.config.getoption('capture') != 'no' and + self.config.getini('console_output_style') == 'progress') def hasopt(self, char): char = {'xfailed': 'x', 'skipped': 's'}.get(char, char) diff --git a/changelog/3038.feature b/changelog/3038.feature new file mode 100644 index 000000000..107dc6fed --- /dev/null +++ b/changelog/3038.feature @@ -0,0 +1 @@ +Use classic console output when -s is used. diff --git a/testing/test_terminal.py b/testing/test_terminal.py index 97c2f71fb..0db56f6f9 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -1037,3 +1037,11 @@ class TestProgress: r'\[gw\d\] \[\s*\d+%\] PASSED test_foo.py::test_foo\[1\]', r'\[gw\d\] \[\s*\d+%\] PASSED test_foobar.py::test_foobar\[1\]', ]) + + def test_capture_no(self, many_tests_file, testdir): + output = testdir.runpytest('-s') + output.stdout.re_match_lines([ + r'test_bar.py \.{10}', + r'test_foo.py \.{5}', + r'test_foobar.py \.{5}', + ]) From 0a2735a27544a81fcaebaec7305ff9d9acd1def7 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 16 Dec 2017 12:33:34 -0200 Subject: [PATCH 16/16] Reword changelog entry --- changelog/3038.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/3038.feature b/changelog/3038.feature index 107dc6fed..a0da2eef3 100644 --- a/changelog/3038.feature +++ b/changelog/3038.feature @@ -1 +1 @@ -Use classic console output when -s is used. +Console output fallsback to "classic" mode when capture is disabled (``-s``), otherwise the output gets garbled to the point of being useless.