From a37b902afea21621639b114f087e84f70fb057ba Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 12 Jun 2019 18:49:51 -0300 Subject: [PATCH 01/18] Integrate pytest-faulthandler into the core * Add pytest-faulthandler files unchanged * Adapt imports and tests * Add code to skip registration of the external `pytest_faulthandler` to avoid conflicts Fix #5440 --- changelog/5440.feature.rst | 8 +++ doc/en/usage.rst | 19 +++++- src/_pytest/config/__init__.py | 3 +- src/_pytest/deprecated.py | 8 +++ src/_pytest/faulthandler.py | 102 +++++++++++++++++++++++++++++++++ testing/deprecated_test.py | 23 +++----- testing/test_faulthandler.py | 99 ++++++++++++++++++++++++++++++++ 7 files changed, 245 insertions(+), 17 deletions(-) create mode 100644 changelog/5440.feature.rst create mode 100644 src/_pytest/faulthandler.py create mode 100644 testing/test_faulthandler.py diff --git a/changelog/5440.feature.rst b/changelog/5440.feature.rst new file mode 100644 index 000000000..d3bb95f58 --- /dev/null +++ b/changelog/5440.feature.rst @@ -0,0 +1,8 @@ +The `faulthandler `__ standard library +module is now enabled by default to help users diagnose crashes in C modules. + +This functionality was provided by integrating the external +`pytest-faulthandler `__ plugin into the core, +so users should remove that plugin from their requirements if used. + +For more information see the docs: https://docs.pytest.org/en/latest/usage.html#fault-handler diff --git a/doc/en/usage.rst b/doc/en/usage.rst index acf736f21..c1332706f 100644 --- a/doc/en/usage.rst +++ b/doc/en/usage.rst @@ -408,7 +408,6 @@ Pytest supports the use of ``breakpoint()`` with the following behaviours: Profiling test execution duration ------------------------------------- -.. versionadded: 2.2 To get a list of the slowest 10 test durations: @@ -418,6 +417,24 @@ To get a list of the slowest 10 test durations: By default, pytest will not show test durations that are too small (<0.01s) unless ``-vv`` is passed on the command-line. + +.. _faulthandler: + +Fault Handler +------------- + +.. versionadded:: 5.0 + +The `faulthandler `__ standard module +can be used to dump Python tracebacks on a segfault or after a timeout. + +The module is automatically enabled for pytest runs, unless the ``--no-faulthandler`` is given +on the command-line. + +Also the ``--faulthandler-timeout=X`` can be used to dump the traceback of all threads if a test +takes longer than ``X`` seconds to finish (not available on Windows). + + Creating JUnitXML format files ---------------------------------------------------- diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 1f6ae98f9..74ee4a2bc 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -141,6 +141,7 @@ default_plugins = essential_plugins + ( "warnings", "logging", "reports", + "faulthandler", ) builtin_plugins = set(default_plugins) @@ -299,7 +300,7 @@ class PytestPluginManager(PluginManager): return opts def register(self, plugin, name=None): - if name in ["pytest_catchlog", "pytest_capturelog"]: + if name in _pytest.deprecated.DEPRECATED_EXTERNAL_PLUGINS: warnings.warn( PytestConfigWarning( "{} plugin has been merged into the core, " diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index 3feae8b43..1c544fd36 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -14,6 +14,14 @@ from _pytest.warning_types import UnformattedWarning YIELD_TESTS = "yield tests were removed in pytest 4.0 - {name} will be ignored" +# set of plugins which have been integrated into the core; we use this list to ignore +# them during registration to avoid conflicts +DEPRECATED_EXTERNAL_PLUGINS = { + "pytest_catchlog", + "pytest_capturelog", + "pytest_faulthandler", +} + FIXTURE_FUNCTION_CALL = ( 'Fixture "{name}" called directly. Fixtures are not meant to be called directly,\n' diff --git a/src/_pytest/faulthandler.py b/src/_pytest/faulthandler.py new file mode 100644 index 000000000..48fe0f218 --- /dev/null +++ b/src/_pytest/faulthandler.py @@ -0,0 +1,102 @@ +import io +import os +import sys + +import pytest + + +def pytest_addoption(parser): + group = parser.getgroup("terminal reporting") + group.addoption( + "--no-faulthandler", + action="store_false", + dest="fault_handler", + default=True, + help="Disable faulthandler module.", + ) + + group.addoption( + "--faulthandler-timeout", + type=float, + dest="fault_handler_timeout", + metavar="TIMEOUT", + default=0.0, + help="Dump the traceback of all threads if a test takes " + "more than TIMEOUT seconds to finish.\n" + "Not available on Windows.", + ) + + +def pytest_configure(config): + if config.getoption("fault_handler"): + import faulthandler + + # avoid trying to dup sys.stderr if faulthandler is already enabled + if faulthandler.is_enabled(): + return + + stderr_fd_copy = os.dup(_get_stderr_fileno()) + config.fault_handler_stderr = os.fdopen(stderr_fd_copy, "w") + faulthandler.enable(file=config.fault_handler_stderr) + + +def _get_stderr_fileno(): + try: + return sys.stderr.fileno() + except (AttributeError, io.UnsupportedOperation): + # python-xdist monkeypatches sys.stderr with an object that is not an actual file. + # https://docs.python.org/3/library/faulthandler.html#issue-with-file-descriptors + # This is potentially dangerous, but the best we can do. + return sys.__stderr__.fileno() + + +def pytest_unconfigure(config): + if config.getoption("fault_handler"): + import faulthandler + + faulthandler.disable() + # close our dup file installed during pytest_configure + f = getattr(config, "fault_handler_stderr", None) + if f is not None: + # re-enable the faulthandler, attaching it to the default sys.stderr + # so we can see crashes after pytest has finished, usually during + # garbage collection during interpreter shutdown + config.fault_handler_stderr.close() + del config.fault_handler_stderr + faulthandler.enable(file=_get_stderr_fileno()) + + +@pytest.hookimpl(hookwrapper=True) +def pytest_runtest_protocol(item): + enabled = item.config.getoption("fault_handler") + timeout = item.config.getoption("fault_handler_timeout") + if enabled and timeout > 0: + import faulthandler + + stderr = item.config.fault_handler_stderr + faulthandler.dump_traceback_later(timeout, file=stderr) + try: + yield + finally: + faulthandler.cancel_dump_traceback_later() + else: + yield + + +@pytest.hookimpl(tryfirst=True) +def pytest_enter_pdb(): + """Cancel any traceback dumping due to timeout before entering pdb. + """ + import faulthandler + + faulthandler.cancel_dump_traceback_later() + + +@pytest.hookimpl(tryfirst=True) +def pytest_exception_interact(): + """Cancel any traceback dumping due to an interactive exception being + raised. + """ + import faulthandler + + faulthandler.cancel_dump_traceback_later() diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index 177594c4a..5cbb694b1 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -1,6 +1,7 @@ import os import pytest +from _pytest import deprecated from _pytest.warning_types import PytestDeprecationWarning from _pytest.warnings import SHOW_PYTEST_WARNINGS_ARG @@ -69,22 +70,14 @@ def test_terminal_reporter_writer_attr(pytestconfig): assert terminal_reporter.writer is terminal_reporter._tw -@pytest.mark.parametrize("plugin", ["catchlog", "capturelog"]) +@pytest.mark.parametrize("plugin", deprecated.DEPRECATED_EXTERNAL_PLUGINS) @pytest.mark.filterwarnings("default") -def test_pytest_catchlog_deprecated(testdir, plugin): - testdir.makepyfile( - """ - def test_func(pytestconfig): - pytestconfig.pluginmanager.register(None, 'pytest_{}') - """.format( - plugin - ) - ) - res = testdir.runpytest() - assert res.ret == 0 - res.stdout.fnmatch_lines( - ["*pytest-*log plugin has been merged into the core*", "*1 passed, 1 warnings*"] - ) +def test_external_plugins_integrated(testdir, plugin): + testdir.syspathinsert() + testdir.makepyfile(**{plugin: ""}) + + with pytest.warns(pytest.PytestConfigWarning): + testdir.parseconfig("-p", plugin) def test_raises_message_argument_deprecated(): diff --git a/testing/test_faulthandler.py b/testing/test_faulthandler.py new file mode 100644 index 000000000..d1f2e8b9a --- /dev/null +++ b/testing/test_faulthandler.py @@ -0,0 +1,99 @@ +import sys + +import pytest + + +def test_enabled(testdir): + """Test single crashing test displays a traceback.""" + testdir.makepyfile( + """ + import faulthandler + def test_crash(): + faulthandler._sigabrt() + """ + ) + result = testdir.runpytest_subprocess() + result.stderr.fnmatch_lines(["*Fatal Python error*"]) + assert result.ret != 0 + + +def test_crash_near_exit(testdir): + """Test that fault handler displays crashes that happen even after + pytest is exiting (for example, when the interpreter is shutting down). + """ + testdir.makepyfile( + """ + import faulthandler + import atexit + def test_ok(): + atexit.register(faulthandler._sigabrt) + """ + ) + result = testdir.runpytest_subprocess() + result.stderr.fnmatch_lines(["*Fatal Python error*"]) + assert result.ret != 0 + + +def test_disabled(testdir): + """Test option to disable fault handler in the command line. + """ + testdir.makepyfile( + """ + import faulthandler + def test_disabled(): + assert not faulthandler.is_enabled() + """ + ) + result = testdir.runpytest_subprocess("--no-faulthandler") + result.stdout.fnmatch_lines(["*1 passed*"]) + assert result.ret == 0 + + +@pytest.mark.parametrize("enabled", [True, False]) +def test_timeout(testdir, enabled): + """Test option to dump tracebacks after a certain timeout. + If faulthandler is disabled, no traceback will be dumped. + """ + testdir.makepyfile( + """ + import time + def test_timeout(): + time.sleep(2.0) + """ + ) + args = ["--faulthandler-timeout=1"] + if not enabled: + args.append("--no-faulthandler") + + result = testdir.runpytest_subprocess(*args) + tb_output = "most recent call first" + if sys.version_info[:2] == (3, 3): + tb_output = "Thread" + if enabled: + result.stderr.fnmatch_lines(["*%s*" % tb_output]) + else: + assert tb_output not in result.stderr.str() + result.stdout.fnmatch_lines(["*1 passed*"]) + assert result.ret == 0 + + +@pytest.mark.parametrize("hook_name", ["pytest_enter_pdb", "pytest_exception_interact"]) +def test_cancel_timeout_on_hook(monkeypatch, pytestconfig, hook_name): + """Make sure that we are cancelling any scheduled traceback dumping due + to timeout before entering pdb (pytest-dev/pytest-faulthandler#12) or any other interactive + exception (pytest-dev/pytest-faulthandler#14). + """ + import faulthandler + from _pytest import faulthandler as plugin_module + + called = [] + + monkeypatch.setattr( + faulthandler, "cancel_dump_traceback_later", lambda: called.append(1) + ) + + # call our hook explicitly, we can trust that pytest will call the hook + # for us at the appropriate moment + hook_func = getattr(plugin_module, hook_name) + hook_func() + assert called == [1] From 3ce31b6370fcaa02a63f09c86a2859800d15984a Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Sat, 22 Jun 2019 19:22:43 -0300 Subject: [PATCH 02/18] Change pytest-faulthandler for simplification * The --no-faulthandler option is not necessary given that we can use `-p no:faulthandler`. * The `--faulthandler-timeout` command-line option has become an ini option, for the reasons described in https://github.com/pytest-dev/pytest-faulthandler/issues/34 and users can still set it from the command-line. Fix pytest-dev/pytest-faulthandler#34 --- doc/en/reference.rst | 17 ++++++++++ doc/en/usage.rst | 20 +++++++++-- src/_pytest/faulthandler.py | 64 ++++++++++++++---------------------- testing/test_faulthandler.py | 12 ++++--- 4 files changed, 66 insertions(+), 47 deletions(-) diff --git a/doc/en/reference.rst b/doc/en/reference.rst index 0b168eb54..7ad69d4a8 100644 --- a/doc/en/reference.rst +++ b/doc/en/reference.rst @@ -1076,6 +1076,23 @@ passed multiple times. The expected format is ``name=value``. For example:: for more details. +.. confval:: faulthandler_timeout + + Dumps the tracebacks of all threads if a test takes longer than ``X`` seconds to run (including + fixture setup and teardown). Implemented using the `faulthandler.dump_traceback_later`_ function, + so all caveats there apply. + + .. code-block:: ini + + # content of pytest.ini + [pytest] + faulthandler_timeout=5 + + For more information please refer to :ref:`faulthandler`. + +.. _`faulthandler.dump_traceback_later`: https://docs.python.org/3/library/faulthandler.html#faulthandler.dump_traceback_later + + .. confval:: filterwarnings diff --git a/doc/en/usage.rst b/doc/en/usage.rst index c1332706f..a8acc3551 100644 --- a/doc/en/usage.rst +++ b/doc/en/usage.rst @@ -428,11 +428,25 @@ Fault Handler The `faulthandler `__ standard module can be used to dump Python tracebacks on a segfault or after a timeout. -The module is automatically enabled for pytest runs, unless the ``--no-faulthandler`` is given +The module is automatically enabled for pytest runs, unless the ``-p no:faulthandler`` is given on the command-line. -Also the ``--faulthandler-timeout=X`` can be used to dump the traceback of all threads if a test -takes longer than ``X`` seconds to finish (not available on Windows). +Also the :confval:`faulthandler_timeout=X` configuration option can be used +to dump the traceback of all threads if a test takes longer than ``X`` +seconds to finish (not available on Windows). + +.. note:: + + This functionality has been integrated from the external + `pytest-faulthandler `__ plugin, with two + small differences: + + * To disable it, use ``-p no:faulthandler`` instead of ``--no-faulthandler``: the former + can be used with any plugin, so it saves one option. + + * The ``--faulthandler-timeout`` command-line option has become the + :confval:`faulthandler_timeout` configuration option. It can still be configured from + the command-line using ``-o faulthandler_timeout=X``. Creating JUnitXML format files diff --git a/src/_pytest/faulthandler.py b/src/_pytest/faulthandler.py index 48fe0f218..068bec528 100644 --- a/src/_pytest/faulthandler.py +++ b/src/_pytest/faulthandler.py @@ -6,38 +6,24 @@ import pytest def pytest_addoption(parser): - group = parser.getgroup("terminal reporting") - group.addoption( - "--no-faulthandler", - action="store_false", - dest="fault_handler", - default=True, - help="Disable faulthandler module.", - ) - - group.addoption( - "--faulthandler-timeout", - type=float, - dest="fault_handler_timeout", - metavar="TIMEOUT", - default=0.0, - help="Dump the traceback of all threads if a test takes " + help = ( + "Dump the traceback of all threads if a test takes " "more than TIMEOUT seconds to finish.\n" - "Not available on Windows.", + "Not available on Windows." ) + parser.addini("faulthandler_timeout", help, default=0.0) def pytest_configure(config): - if config.getoption("fault_handler"): - import faulthandler + import faulthandler - # avoid trying to dup sys.stderr if faulthandler is already enabled - if faulthandler.is_enabled(): - return + # avoid trying to dup sys.stderr if faulthandler is already enabled + if faulthandler.is_enabled(): + return - stderr_fd_copy = os.dup(_get_stderr_fileno()) - config.fault_handler_stderr = os.fdopen(stderr_fd_copy, "w") - faulthandler.enable(file=config.fault_handler_stderr) + stderr_fd_copy = os.dup(_get_stderr_fileno()) + config.fault_handler_stderr = os.fdopen(stderr_fd_copy, "w") + faulthandler.enable(file=config.fault_handler_stderr) def _get_stderr_fileno(): @@ -51,26 +37,24 @@ def _get_stderr_fileno(): def pytest_unconfigure(config): - if config.getoption("fault_handler"): - import faulthandler + import faulthandler - faulthandler.disable() - # close our dup file installed during pytest_configure - f = getattr(config, "fault_handler_stderr", None) - if f is not None: - # re-enable the faulthandler, attaching it to the default sys.stderr - # so we can see crashes after pytest has finished, usually during - # garbage collection during interpreter shutdown - config.fault_handler_stderr.close() - del config.fault_handler_stderr - faulthandler.enable(file=_get_stderr_fileno()) + faulthandler.disable() + # close our dup file installed during pytest_configure + f = getattr(config, "fault_handler_stderr", None) + if f is not None: + # re-enable the faulthandler, attaching it to the default sys.stderr + # so we can see crashes after pytest has finished, usually during + # garbage collection during interpreter shutdown + config.fault_handler_stderr.close() + del config.fault_handler_stderr + faulthandler.enable(file=_get_stderr_fileno()) @pytest.hookimpl(hookwrapper=True) def pytest_runtest_protocol(item): - enabled = item.config.getoption("fault_handler") - timeout = item.config.getoption("fault_handler_timeout") - if enabled and timeout > 0: + timeout = float(item.config.getini("faulthandler_timeout") or 0.0) + if timeout > 0: import faulthandler stderr = item.config.fault_handler_stderr diff --git a/testing/test_faulthandler.py b/testing/test_faulthandler.py index d1f2e8b9a..a0cf1d8c1 100644 --- a/testing/test_faulthandler.py +++ b/testing/test_faulthandler.py @@ -44,7 +44,7 @@ def test_disabled(testdir): assert not faulthandler.is_enabled() """ ) - result = testdir.runpytest_subprocess("--no-faulthandler") + result = testdir.runpytest_subprocess("-p", "no:faulthandler") result.stdout.fnmatch_lines(["*1 passed*"]) assert result.ret == 0 @@ -61,9 +61,13 @@ def test_timeout(testdir, enabled): time.sleep(2.0) """ ) - args = ["--faulthandler-timeout=1"] - if not enabled: - args.append("--no-faulthandler") + testdir.makeini( + """ + [pytest] + faulthandler_timeout = 1 + """ + ) + args = ["-p", "no:faulthandler"] if not enabled else [] result = testdir.runpytest_subprocess(*args) tb_output = "most recent call first" From 01a094cc4316a2840254a66f829367f3d67fd442 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 24 Jun 2019 06:05:24 +0200 Subject: [PATCH 03/18] minor: clarify help with reportchars `-ra` / `-rA` will include "w" also. This does not explicitly mention it (allowing for change the behavior), but makes it a) clearer that "w" is a recognized reportchar, and b) less confusing that `-ra --disable-warnings` still displays them. --- src/_pytest/terminal.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 91e373852..bcd6e1f7c 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -76,8 +76,7 @@ def pytest_addoption(parser): help="show extra test summary info as specified by chars: (f)ailed, " "(E)rror, (s)kipped, (x)failed, (X)passed, " "(p)assed, (P)assed with output, (a)ll except passed (p/P), or (A)ll. " - "Warnings are displayed at all times except when " - "--disable-warnings is set.", + "(w)arnings are enabled by default (see --disable-warnings).", ) group._addoption( "--disable-warnings", From 4cd08f9b52e3ed984cf68b58abb6f7350623231f Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sat, 22 Jun 2019 15:02:32 -0700 Subject: [PATCH 04/18] Switch from deprecated imp to importlib --- changelog/1403.bugfix.rst | 1 + changelog/2761.bugfix.rst | 1 + changelog/5078.bugfix.rst | 1 + changelog/5432.bugfix.rst | 1 + changelog/5433.bugfix.rst | 1 + src/_pytest/assertion/rewrite.py | 239 ++++++++++--------------------- src/_pytest/main.py | 22 ++- src/_pytest/pathlib.py | 2 + src/_pytest/pytester.py | 19 +-- testing/acceptance_test.py | 13 ++ testing/test_assertion.py | 4 +- testing/test_assertrewrite.py | 115 ++++++++------- testing/test_pathlib.py | 5 + 13 files changed, 182 insertions(+), 242 deletions(-) create mode 100644 changelog/1403.bugfix.rst create mode 100644 changelog/2761.bugfix.rst create mode 100644 changelog/5078.bugfix.rst create mode 100644 changelog/5432.bugfix.rst create mode 100644 changelog/5433.bugfix.rst diff --git a/changelog/1403.bugfix.rst b/changelog/1403.bugfix.rst new file mode 100644 index 000000000..3fb748aec --- /dev/null +++ b/changelog/1403.bugfix.rst @@ -0,0 +1 @@ +Switch from ``imp`` to ``importlib``. diff --git a/changelog/2761.bugfix.rst b/changelog/2761.bugfix.rst new file mode 100644 index 000000000..c63f02ecd --- /dev/null +++ b/changelog/2761.bugfix.rst @@ -0,0 +1 @@ +Honor PEP 235 on case-insensitive file systems. diff --git a/changelog/5078.bugfix.rst b/changelog/5078.bugfix.rst new file mode 100644 index 000000000..8fed85f5d --- /dev/null +++ b/changelog/5078.bugfix.rst @@ -0,0 +1 @@ +Test module is no longer double-imported when using ``--pyargs``. diff --git a/changelog/5432.bugfix.rst b/changelog/5432.bugfix.rst new file mode 100644 index 000000000..44c01c0cf --- /dev/null +++ b/changelog/5432.bugfix.rst @@ -0,0 +1 @@ +Prevent "already imported" warnings from assertion rewriter when invoking pytest in-process multiple times. diff --git a/changelog/5433.bugfix.rst b/changelog/5433.bugfix.rst new file mode 100644 index 000000000..c3a7472bc --- /dev/null +++ b/changelog/5433.bugfix.rst @@ -0,0 +1 @@ +Fix assertion rewriting in packages (``__init__.py``). diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index ce698f368..cce980005 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -1,18 +1,16 @@ """Rewrite assertion AST to produce nice error messages""" import ast import errno -import imp +import importlib.machinery +import importlib.util import itertools import marshal import os -import re import struct import sys import types -from importlib.util import spec_from_file_location import atomicwrites -import py from _pytest._io.saferepr import saferepr from _pytest.assertion import util @@ -23,23 +21,13 @@ from _pytest.pathlib import fnmatch_ex from _pytest.pathlib import PurePath # pytest caches rewritten pycs in __pycache__. -if hasattr(imp, "get_tag"): - PYTEST_TAG = imp.get_tag() + "-PYTEST" -else: - if hasattr(sys, "pypy_version_info"): - impl = "pypy" - else: - impl = "cpython" - ver = sys.version_info - PYTEST_TAG = "{}-{}{}-PYTEST".format(impl, ver[0], ver[1]) - del ver, impl - +PYTEST_TAG = "{}-PYTEST".format(sys.implementation.cache_tag) PYC_EXT = ".py" + (__debug__ and "c" or "o") PYC_TAIL = "." + PYTEST_TAG + PYC_EXT class AssertionRewritingHook: - """PEP302 Import hook which rewrites asserts.""" + """PEP302/PEP451 import hook which rewrites asserts.""" def __init__(self, config): self.config = config @@ -48,7 +36,6 @@ class AssertionRewritingHook: except ValueError: self.fnpats = ["test_*.py", "*_test.py"] self.session = None - self.modules = {} self._rewritten_names = set() self._must_rewrite = set() # flag to guard against trying to rewrite a pyc file while we are already writing another pyc file, @@ -62,55 +49,51 @@ class AssertionRewritingHook: self.session = session self._session_paths_checked = False - def _imp_find_module(self, name, path=None): - """Indirection so we can mock calls to find_module originated from the hook during testing""" - return imp.find_module(name, path) + # Indirection so we can mock calls to find_spec originated from the hook during testing + _find_spec = importlib.machinery.PathFinder.find_spec - def find_module(self, name, path=None): + def find_spec(self, name, path=None, target=None): if self._writing_pyc: return None state = self.config._assertstate if self._early_rewrite_bailout(name, state): return None state.trace("find_module called for: %s" % name) - names = name.rsplit(".", 1) - lastname = names[-1] - pth = None - if path is not None: - # Starting with Python 3.3, path is a _NamespacePath(), which - # causes problems if not converted to list. - path = list(path) - if len(path) == 1: - pth = path[0] - if pth is None: - try: - fd, fn, desc = self._imp_find_module(lastname, path) - except ImportError: - return None - if fd is not None: - fd.close() - tp = desc[2] - if tp == imp.PY_COMPILED: - if hasattr(imp, "source_from_cache"): - try: - fn = imp.source_from_cache(fn) - except ValueError: - # Python 3 doesn't like orphaned but still-importable - # .pyc files. - fn = fn[:-1] - else: - fn = fn[:-1] - elif tp != imp.PY_SOURCE: - # Don't know what this is. - return None - else: - fn = os.path.join(pth, name.rpartition(".")[2] + ".py") - fn_pypath = py.path.local(fn) - if not self._should_rewrite(name, fn_pypath, state): + spec = self._find_spec(name, path) + if ( + # the import machinery could not find a file to import + spec is None + # this is a namespace package (without `__init__.py`) + # there's nothing to rewrite there + # python3.5 - python3.6: `namespace` + # python3.7+: `None` + or spec.origin in {None, "namespace"} + # if the file doesn't exist, we can't rewrite it + or not os.path.exists(spec.origin) + ): + return None + else: + fn = spec.origin + + if not self._should_rewrite(name, fn, state): return None - self._rewritten_names.add(name) + return importlib.util.spec_from_file_location( + name, + fn, + loader=self, + submodule_search_locations=spec.submodule_search_locations, + ) + + def create_module(self, spec): + return None # default behaviour is fine + + def exec_module(self, module): + fn = module.__spec__.origin + state = self.config._assertstate + + self._rewritten_names.add(module.__name__) # The requested module looks like a test file, so rewrite it. This is # the most magical part of the process: load the source, rewrite the @@ -121,7 +104,7 @@ class AssertionRewritingHook: # cached pyc is always a complete, valid pyc. Operations on it must be # atomic. POSIX's atomic rename comes in handy. write = not sys.dont_write_bytecode - cache_dir = os.path.join(fn_pypath.dirname, "__pycache__") + cache_dir = os.path.join(os.path.dirname(fn), "__pycache__") if write: try: os.mkdir(cache_dir) @@ -132,26 +115,23 @@ class AssertionRewritingHook: # common case) or it's blocked by a non-dir node. In the # latter case, we'll ignore it in _write_pyc. pass - elif e in [errno.ENOENT, errno.ENOTDIR]: + elif e in {errno.ENOENT, errno.ENOTDIR}: # One of the path components was not a directory, likely # because we're in a zip file. write = False - elif e in [errno.EACCES, errno.EROFS, errno.EPERM]: - state.trace("read only directory: %r" % fn_pypath.dirname) + elif e in {errno.EACCES, errno.EROFS, errno.EPERM}: + state.trace("read only directory: %r" % os.path.dirname(fn)) write = False else: raise - cache_name = fn_pypath.basename[:-3] + PYC_TAIL + cache_name = os.path.basename(fn)[:-3] + PYC_TAIL pyc = os.path.join(cache_dir, cache_name) # Notice that even if we're in a read-only directory, I'm going # to check for a cached pyc. This may not be optimal... - co = _read_pyc(fn_pypath, pyc, state.trace) + co = _read_pyc(fn, pyc, state.trace) if co is None: state.trace("rewriting {!r}".format(fn)) - source_stat, co = _rewrite_test(self.config, fn_pypath) - if co is None: - # Probably a SyntaxError in the test. - return None + source_stat, co = _rewrite_test(fn) if write: self._writing_pyc = True try: @@ -160,13 +140,11 @@ class AssertionRewritingHook: self._writing_pyc = False else: state.trace("found cached rewritten pyc for {!r}".format(fn)) - self.modules[name] = co, pyc - return self + exec(co, module.__dict__) def _early_rewrite_bailout(self, name, state): - """ - This is a fast way to get out of rewriting modules. Profiling has - shown that the call to imp.find_module (inside of the find_module + """This is a fast way to get out of rewriting modules. Profiling has + shown that the call to PathFinder.find_spec (inside of the find_spec from this class) is a major slowdown, so, this method tries to filter what we're sure won't be rewritten before getting to it. """ @@ -201,10 +179,9 @@ class AssertionRewritingHook: state.trace("early skip of rewriting module: {}".format(name)) return True - def _should_rewrite(self, name, fn_pypath, state): + def _should_rewrite(self, name, fn, state): # always rewrite conftest files - fn = str(fn_pypath) - if fn_pypath.basename == "conftest.py": + if os.path.basename(fn) == "conftest.py": state.trace("rewriting conftest file: {!r}".format(fn)) return True @@ -217,8 +194,9 @@ class AssertionRewritingHook: # modules not passed explicitly on the command line are only # rewritten if they match the naming convention for test files + fn_path = PurePath(fn) for pat in self.fnpats: - if fn_pypath.fnmatch(pat): + if fnmatch_ex(pat, fn_path): state.trace("matched test file {!r}".format(fn)) return True @@ -249,9 +227,10 @@ class AssertionRewritingHook: set(names).intersection(sys.modules).difference(self._rewritten_names) ) for name in already_imported: + mod = sys.modules[name] if not AssertionRewriter.is_rewrite_disabled( - sys.modules[name].__doc__ or "" - ): + mod.__doc__ or "" + ) and not isinstance(mod.__loader__, type(self)): self._warn_already_imported(name) self._must_rewrite.update(names) self._marked_for_rewrite_cache.clear() @@ -268,45 +247,8 @@ class AssertionRewritingHook: stacklevel=5, ) - def load_module(self, name): - co, pyc = self.modules.pop(name) - if name in sys.modules: - # If there is an existing module object named 'fullname' in - # sys.modules, the loader must use that existing module. (Otherwise, - # the reload() builtin will not work correctly.) - mod = sys.modules[name] - else: - # I wish I could just call imp.load_compiled here, but __file__ has to - # be set properly. In Python 3.2+, this all would be handled correctly - # by load_compiled. - mod = sys.modules[name] = imp.new_module(name) - try: - mod.__file__ = co.co_filename - # Normally, this attribute is 3.2+. - mod.__cached__ = pyc - mod.__loader__ = self - # Normally, this attribute is 3.4+ - mod.__spec__ = spec_from_file_location(name, co.co_filename, loader=self) - exec(co, mod.__dict__) - except: # noqa - if name in sys.modules: - del sys.modules[name] - raise - return sys.modules[name] - - def is_package(self, name): - try: - fd, fn, desc = self._imp_find_module(name) - except ImportError: - return False - if fd is not None: - fd.close() - tp = desc[2] - return tp == imp.PKG_DIRECTORY - def get_data(self, pathname): - """Optional PEP302 get_data API. - """ + """Optional PEP302 get_data API.""" with open(pathname, "rb") as f: return f.read() @@ -314,15 +256,13 @@ class AssertionRewritingHook: def _write_pyc(state, co, source_stat, pyc): # Technically, we don't have to have the same pyc format as # (C)Python, since these "pycs" should never be seen by builtin - # import. However, there's little reason deviate, and I hope - # sometime to be able to use imp.load_compiled to load them. (See - # the comment in load_module above.) + # import. However, there's little reason deviate. try: with atomicwrites.atomic_write(pyc, mode="wb", overwrite=True) as fp: - fp.write(imp.get_magic()) + fp.write(importlib.util.MAGIC_NUMBER) # as of now, bytecode header expects 32-bit numbers for size and mtime (#4903) - mtime = int(source_stat.mtime) & 0xFFFFFFFF - size = source_stat.size & 0xFFFFFFFF + mtime = int(source_stat.st_mtime) & 0xFFFFFFFF + size = source_stat.st_size & 0xFFFFFFFF # " strip_bytes pyc.write(contents[:strip_bytes], mode="wb") - assert _read_pyc(source, str(pyc)) is None # no error + assert _read_pyc(str(source), str(pyc)) is None # no error def test_reload_is_same(self, testdir): # A file that will be picked up during collecting. @@ -1186,14 +1192,17 @@ def test_rewrite_infinite_recursion(testdir, pytestconfig, monkeypatch): # make a note that we have called _write_pyc write_pyc_called.append(True) # try to import a module at this point: we should not try to rewrite this module - assert hook.find_module("test_bar") is None + assert hook.find_spec("test_bar") is None return original_write_pyc(*args, **kwargs) monkeypatch.setattr(rewrite, "_write_pyc", spy_write_pyc) monkeypatch.setattr(sys, "dont_write_bytecode", False) hook = AssertionRewritingHook(pytestconfig) - assert hook.find_module("test_foo") is not None + spec = hook.find_spec("test_foo") + assert spec is not None + module = importlib.util.module_from_spec(spec) + hook.exec_module(module) assert len(write_pyc_called) == 1 @@ -1201,11 +1210,11 @@ class TestEarlyRewriteBailout: @pytest.fixture def hook(self, pytestconfig, monkeypatch, testdir): """Returns a patched AssertionRewritingHook instance so we can configure its initial paths and track - if imp.find_module has been called. + if PathFinder.find_spec has been called. """ - import imp + import importlib.machinery - self.find_module_calls = [] + self.find_spec_calls = [] self.initial_paths = set() class StubSession: @@ -1214,22 +1223,22 @@ class TestEarlyRewriteBailout: def isinitpath(self, p): return p in self._initialpaths - def spy_imp_find_module(name, path): - self.find_module_calls.append(name) - return imp.find_module(name, path) + def spy_find_spec(name, path): + self.find_spec_calls.append(name) + return importlib.machinery.PathFinder.find_spec(name, path) hook = AssertionRewritingHook(pytestconfig) # use default patterns, otherwise we inherit pytest's testing config hook.fnpats[:] = ["test_*.py", "*_test.py"] - monkeypatch.setattr(hook, "_imp_find_module", spy_imp_find_module) + monkeypatch.setattr(hook, "_find_spec", spy_find_spec) hook.set_session(StubSession()) testdir.syspathinsert() return hook def test_basic(self, testdir, hook): """ - Ensure we avoid calling imp.find_module when we know for sure a certain module will not be rewritten - to optimize assertion rewriting (#3918). + Ensure we avoid calling PathFinder.find_spec when we know for sure a certain + module will not be rewritten to optimize assertion rewriting (#3918). """ testdir.makeconftest( """ @@ -1244,24 +1253,24 @@ class TestEarlyRewriteBailout: self.initial_paths.add(foobar_path) # conftest files should always be rewritten - assert hook.find_module("conftest") is not None - assert self.find_module_calls == ["conftest"] + assert hook.find_spec("conftest") is not None + assert self.find_spec_calls == ["conftest"] # files matching "python_files" mask should always be rewritten - assert hook.find_module("test_foo") is not None - assert self.find_module_calls == ["conftest", "test_foo"] + assert hook.find_spec("test_foo") is not None + assert self.find_spec_calls == ["conftest", "test_foo"] # file does not match "python_files": early bailout - assert hook.find_module("bar") is None - assert self.find_module_calls == ["conftest", "test_foo"] + assert hook.find_spec("bar") is None + assert self.find_spec_calls == ["conftest", "test_foo"] # file is an initial path (passed on the command-line): should be rewritten - assert hook.find_module("foobar") is not None - assert self.find_module_calls == ["conftest", "test_foo", "foobar"] + assert hook.find_spec("foobar") is not None + assert self.find_spec_calls == ["conftest", "test_foo", "foobar"] def test_pattern_contains_subdirectories(self, testdir, hook): """If one of the python_files patterns contain subdirectories ("tests/**.py") we can't bailout early - because we need to match with the full path, which can only be found by calling imp.find_module. + because we need to match with the full path, which can only be found by calling PathFinder.find_spec """ p = testdir.makepyfile( **{ @@ -1273,8 +1282,8 @@ class TestEarlyRewriteBailout: ) testdir.syspathinsert(p.dirpath()) hook.fnpats[:] = ["tests/**.py"] - assert hook.find_module("file") is not None - assert self.find_module_calls == ["file"] + assert hook.find_spec("file") is not None + assert self.find_spec_calls == ["file"] @pytest.mark.skipif( sys.platform.startswith("win32"), reason="cannot remove cwd on Windows" diff --git a/testing/test_pathlib.py b/testing/test_pathlib.py index 8ac404070..45daeaed7 100644 --- a/testing/test_pathlib.py +++ b/testing/test_pathlib.py @@ -1,3 +1,4 @@ +import os.path import sys import py @@ -53,6 +54,10 @@ class TestPort: def test_matching(self, match, pattern, path): assert match(pattern, path) + def test_matching_abspath(self, match): + abspath = os.path.abspath(os.path.join("tests/foo.py")) + assert match("tests/foo.py", abspath) + @pytest.mark.parametrize( "pattern, path", [ From 380ca8f8805ee6bbdbb7c9de8a3c136c2614bab1 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Mon, 24 Jun 2019 11:24:03 -0700 Subject: [PATCH 05/18] Use new raise syntax in one case --- src/_pytest/outcomes.py | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/_pytest/outcomes.py b/src/_pytest/outcomes.py index 749e80f3c..fb4d471b5 100644 --- a/src/_pytest/outcomes.py +++ b/src/_pytest/outcomes.py @@ -149,7 +149,6 @@ def importorskip(modname, minversion=None, reason=None): __tracebackhide__ = True compile(modname, "", "eval") # to catch syntaxerrors - import_exc = None with warnings.catch_warnings(): # make sure to ignore ImportWarnings that might happen because @@ -159,12 +158,9 @@ def importorskip(modname, minversion=None, reason=None): try: __import__(modname) except ImportError as exc: - # Do not raise chained exception here(#1485) - import_exc = exc - if import_exc: - if reason is None: - reason = "could not import {!r}: {}".format(modname, import_exc) - raise Skipped(reason, allow_module_level=True) + if reason is None: + reason = "could not import {!r}: {}".format(modname, exc) + raise Skipped(reason, allow_module_level=True) from None mod = sys.modules[modname] if minversion is None: return mod From f43fb131797da038ba28917d1b7c775b4c93c5dd Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 24 Jun 2019 20:37:07 -0300 Subject: [PATCH 06/18] Include pytest version in the cached pyc tags Fix #1671 --- changelog/1671.bugfix.rst | 2 ++ src/_pytest/assertion/rewrite.py | 3 ++- testing/test_assertrewrite.py | 18 ++++++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) create mode 100644 changelog/1671.bugfix.rst diff --git a/changelog/1671.bugfix.rst b/changelog/1671.bugfix.rst new file mode 100644 index 000000000..c46eac828 --- /dev/null +++ b/changelog/1671.bugfix.rst @@ -0,0 +1,2 @@ +The name of the ``.pyc`` files cached by the assertion writer now includes the pytest version +to avoid stale caches. diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index cce980005..f50d8200e 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -13,6 +13,7 @@ import types import atomicwrites from _pytest._io.saferepr import saferepr +from _pytest._version import version from _pytest.assertion import util from _pytest.assertion.util import ( # noqa: F401 format_explanation as _format_explanation, @@ -21,7 +22,7 @@ from _pytest.pathlib import fnmatch_ex from _pytest.pathlib import PurePath # pytest caches rewritten pycs in __pycache__. -PYTEST_TAG = "{}-PYTEST".format(sys.implementation.cache_tag) +PYTEST_TAG = "{}-pytest-{}".format(sys.implementation.cache_tag, version) PYC_EXT = ".py" + (__debug__ and "c" or "o") PYC_TAIL = "." + PYTEST_TAG + PYC_EXT diff --git a/testing/test_assertrewrite.py b/testing/test_assertrewrite.py index 33c889104..258b5d3b7 100644 --- a/testing/test_assertrewrite.py +++ b/testing/test_assertrewrite.py @@ -780,6 +780,24 @@ def test_rewritten(): assert testdir.runpytest().ret == 0 + def test_cached_pyc_includes_pytest_version(self, testdir, monkeypatch): + """Avoid stale caches (#1671)""" + monkeypatch.delenv("PYTHONDONTWRITEBYTECODE", raising=False) + testdir.makepyfile( + test_foo=""" + def test_foo(): + assert True + """ + ) + result = testdir.runpytest_subprocess() + assert result.ret == 0 + found_names = glob.glob( + "__pycache__/*-pytest-{}.pyc".format(pytest.__version__) + ) + assert found_names, "pyc with expected tag not found in names: {}".format( + glob.glob("__pycache__/*.pyc") + ) + @pytest.mark.skipif('"__pypy__" in sys.modules') def test_pyc_vs_pyo(self, testdir, monkeypatch): testdir.makepyfile( From 95714436a1ec29e01f017c914cee63970acdcb2a Mon Sep 17 00:00:00 2001 From: "Kevin J. Foley" Date: Mon, 24 Jun 2019 19:07:40 -0400 Subject: [PATCH 07/18] Pickup additional positional args passed to _parse_parametrize_args --- AUTHORS | 1 + changelog/5482.bugfix.rst | 2 ++ src/_pytest/mark/structures.py | 5 +---- testing/python/metafunc.py | 13 +++++++++++++ 4 files changed, 17 insertions(+), 4 deletions(-) create mode 100644 changelog/5482.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 3d050a346..087fce8d0 100644 --- a/AUTHORS +++ b/AUTHORS @@ -135,6 +135,7 @@ Kale Kundert Katarzyna Jachim Katerina Koukiou Kevin Cox +Kevin J. Foley Kodi B. Arfer Kostis Anagnostopoulos Kristoffer Nordström diff --git a/changelog/5482.bugfix.rst b/changelog/5482.bugfix.rst new file mode 100644 index 000000000..c345458d1 --- /dev/null +++ b/changelog/5482.bugfix.rst @@ -0,0 +1,2 @@ +Fix bug introduced in 4.6.0 causing collection errors when passing +more than 2 positional arguments to ``pytest.mark.parametrize``. diff --git a/src/_pytest/mark/structures.py b/src/_pytest/mark/structures.py index 39cdb57e4..1af7a9b42 100644 --- a/src/_pytest/mark/structures.py +++ b/src/_pytest/mark/structures.py @@ -102,10 +102,7 @@ class ParameterSet(namedtuple("ParameterSet", "values, marks, id")): return cls(parameterset, marks=[], id=None) @staticmethod - def _parse_parametrize_args(argnames, argvalues, **_): - """It receives an ignored _ (kwargs) argument so this function can - take also calls from parametrize ignoring scope, indirect, and other - arguments...""" + def _parse_parametrize_args(argnames, argvalues, *args, **kwargs): if not isinstance(argnames, (tuple, list)): argnames = [x.strip() for x in argnames.split(",") if x.strip()] force_tuple = len(argnames) == 1 diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 6b26be72b..df93d4ef5 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1761,3 +1761,16 @@ class TestMarkersWithParametrization: result.stdout.fnmatch_lines( ["*test_func_a*0*PASS*", "*test_func_a*2*PASS*", "*test_func_b*10*PASS*"] ) + + def test_parametrize_positional_args(self, testdir): + testdir.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("a", [1], False) + def test_foo(a): + pass + """ + ) + result = testdir.runpytest() + result.assert_outcomes(passed=1) From 23aa3bb0ae02bf138a5650f44d67b697cf405172 Mon Sep 17 00:00:00 2001 From: "Kevin J. Foley" Date: Mon, 24 Jun 2019 20:55:51 -0400 Subject: [PATCH 08/18] Clarify changelog entries should be rst files --- CONTRIBUTING.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst index 57628a34b..ec053c081 100644 --- a/CONTRIBUTING.rst +++ b/CONTRIBUTING.rst @@ -173,7 +173,7 @@ Short version The test environments above are usually enough to cover most cases locally. -#. Write a ``changelog`` entry: ``changelog/2574.bugfix``, use issue id number +#. Write a ``changelog`` entry: ``changelog/2574.bugfix.rst``, use issue id number and one of ``bugfix``, ``removal``, ``feature``, ``vendor``, ``doc`` or ``trivial`` for the issue type. #. Unless your change is a trivial or a documentation fix (e.g., a typo or reword of a small section) please @@ -264,7 +264,7 @@ Here is a simple overview, with pytest-specific bits: $ git commit -a -m "" $ git push -u -#. Create a new changelog entry in ``changelog``. The file should be named ``.``, +#. Create a new changelog entry in ``changelog``. The file should be named ``..rst``, where *issueid* is the number of the issue related to the change and *type* is one of ``bugfix``, ``removal``, ``feature``, ``vendor``, ``doc`` or ``trivial``. From d72fb73fa091534a862dbe462257efeef7ecaa2f Mon Sep 17 00:00:00 2001 From: Zac Hatfield-Dodds Date: Tue, 25 Jun 2019 13:51:33 +1000 Subject: [PATCH 09/18] Do not allow abbreviated arguments --- changelog/1149.removal.rst | 7 ++++ extra/get_issues.py | 2 +- scripts/release.py | 2 +- src/_pytest/config/argparsing.py | 39 +++++++++++++++++++ testing/acceptance_test.py | 2 +- .../collect_stats/generate_folders.py | 2 +- testing/test_capture.py | 2 +- testing/test_parseopt.py | 8 ++-- testing/test_pastebin.py | 2 +- 9 files changed, 55 insertions(+), 11 deletions(-) create mode 100644 changelog/1149.removal.rst diff --git a/changelog/1149.removal.rst b/changelog/1149.removal.rst new file mode 100644 index 000000000..f507014d9 --- /dev/null +++ b/changelog/1149.removal.rst @@ -0,0 +1,7 @@ +Pytest no longer accepts prefixes of command-line arguments, for example +typing ``pytest --doctest-mod`` inplace of ``--doctest-modules``. +This was previously allowed where the ``ArgumentParser`` thought it was unambiguous, +but this could be incorrect due to delayed parsing of options for plugins. +See for example issues `#1149 `__, +`#3413 `__, and +`#4009 `__. diff --git a/extra/get_issues.py b/extra/get_issues.py index 9407aeded..ae99c9aa6 100644 --- a/extra/get_issues.py +++ b/extra/get_issues.py @@ -74,7 +74,7 @@ def report(issues): if __name__ == "__main__": import argparse - parser = argparse.ArgumentParser("process bitbucket issues") + parser = argparse.ArgumentParser("process bitbucket issues", allow_abbrev=False) parser.add_argument( "--refresh", action="store_true", help="invalidate cache, refresh issues" ) diff --git a/scripts/release.py b/scripts/release.py index 5009df359..d2a51e25a 100644 --- a/scripts/release.py +++ b/scripts/release.py @@ -105,7 +105,7 @@ def changelog(version, write_out=False): def main(): init(autoreset=True) - parser = argparse.ArgumentParser() + parser = argparse.ArgumentParser(allow_abbrev=False) parser.add_argument("version", help="Release version") options = parser.parse_args() pre_release(options.version) diff --git a/src/_pytest/config/argparsing.py b/src/_pytest/config/argparsing.py index fb36c7985..d62ed0d03 100644 --- a/src/_pytest/config/argparsing.py +++ b/src/_pytest/config/argparsing.py @@ -1,5 +1,7 @@ import argparse +import sys import warnings +from gettext import gettext import py @@ -328,6 +330,7 @@ class MyOptionParser(argparse.ArgumentParser): usage=parser._usage, add_help=False, formatter_class=DropShorterLongHelpFormatter, + allow_abbrev=False, ) # extra_info is a dict of (param -> value) to display if there's # an usage error to provide more contextual information to the user @@ -355,6 +358,42 @@ class MyOptionParser(argparse.ArgumentParser): getattr(args, FILE_OR_DIR).extend(argv) return args + if sys.version_info[:2] < (3, 8): # pragma: no cover + # Backport of https://github.com/python/cpython/pull/14316 so we can + # disable long --argument abbreviations without breaking short flags. + def _parse_optional(self, arg_string): + if not arg_string: + return None + if not arg_string[0] in self.prefix_chars: + return None + if arg_string in self._option_string_actions: + action = self._option_string_actions[arg_string] + return action, arg_string, None + if len(arg_string) == 1: + return None + if "=" in arg_string: + option_string, explicit_arg = arg_string.split("=", 1) + if option_string in self._option_string_actions: + action = self._option_string_actions[option_string] + return action, option_string, explicit_arg + if self.allow_abbrev or not arg_string.startswith("--"): + option_tuples = self._get_option_tuples(arg_string) + if len(option_tuples) > 1: + msg = gettext( + "ambiguous option: %(option)s could match %(matches)s" + ) + options = ", ".join(option for _, option, _ in option_tuples) + self.error(msg % {"option": arg_string, "matches": options}) + elif len(option_tuples) == 1: + option_tuple, = option_tuples + return option_tuple + if self._negative_number_matcher.match(arg_string): + if not self._has_negative_number_optionals: + return None + if " " in arg_string: + return None + return None, arg_string, None + class DropShorterLongHelpFormatter(argparse.HelpFormatter): """shorten help for long options that differ only in extra hyphens diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 9d903f802..bac7d4f3a 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -984,7 +984,7 @@ def test_zipimport_hook(testdir, tmpdir): "app/foo.py": """ import pytest def main(): - pytest.main(['--pyarg', 'foo']) + pytest.main(['--pyargs', 'foo']) """ } ) diff --git a/testing/example_scripts/perf_examples/collect_stats/generate_folders.py b/testing/example_scripts/perf_examples/collect_stats/generate_folders.py index ff1eaf7d6..d2c1a30b2 100644 --- a/testing/example_scripts/perf_examples/collect_stats/generate_folders.py +++ b/testing/example_scripts/perf_examples/collect_stats/generate_folders.py @@ -4,7 +4,7 @@ import pathlib HERE = pathlib.Path(__file__).parent TEST_CONTENT = (HERE / "template_test.py").read_bytes() -parser = argparse.ArgumentParser() +parser = argparse.ArgumentParser(allow_abbrev=False) parser.add_argument("numbers", nargs="*", type=int) diff --git a/testing/test_capture.py b/testing/test_capture.py index 0825745ad..3f75089f4 100644 --- a/testing/test_capture.py +++ b/testing/test_capture.py @@ -735,7 +735,7 @@ def test_capture_badoutput_issue412(testdir): assert 0 """ ) - result = testdir.runpytest("--cap=fd") + result = testdir.runpytest("--capture=fd") result.stdout.fnmatch_lines( """ *def test_func* diff --git a/testing/test_parseopt.py b/testing/test_parseopt.py index 7c581cce1..dd7bc8753 100644 --- a/testing/test_parseopt.py +++ b/testing/test_parseopt.py @@ -200,7 +200,7 @@ class TestParser: def test_drop_short_helper(self): parser = argparse.ArgumentParser( - formatter_class=parseopt.DropShorterLongHelpFormatter + formatter_class=parseopt.DropShorterLongHelpFormatter, allow_abbrev=False ) parser.add_argument( "-t", "--twoword", "--duo", "--two-word", "--two", help="foo" @@ -239,10 +239,8 @@ class TestParser: parser.addoption("--funcarg", "--func-arg", action="store_true") parser.addoption("--abc-def", "--abc-def", action="store_true") parser.addoption("--klm-hij", action="store_true") - args = parser.parse(["--funcarg", "--k"]) - assert args.funcarg is True - assert args.abc_def is False - assert args.klm_hij is True + with pytest.raises(UsageError): + parser.parse(["--funcarg", "--k"]) def test_drop_short_2(self, parser): parser.addoption("--func-arg", "--doit", action="store_true") diff --git a/testing/test_pastebin.py b/testing/test_pastebin.py index 48dea14bd..fd443ed40 100644 --- a/testing/test_pastebin.py +++ b/testing/test_pastebin.py @@ -21,7 +21,7 @@ class TestPasteCapture: pytest.skip("") """ ) - reprec = testdir.inline_run(testpath, "--paste=failed") + reprec = testdir.inline_run(testpath, "--pastebin=failed") assert len(pastebinlist) == 1 s = pastebinlist[0] assert s.find("def test_fail") != -1 From b991810f32bc56ec8ab773eac81fccf69647ffb0 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 25 Jun 2019 08:00:20 -0700 Subject: [PATCH 10/18] Do not attempt to rewrite non-source files --- src/_pytest/assertion/rewrite.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index cce980005..de33b6058 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -69,6 +69,8 @@ class AssertionRewritingHook: # python3.5 - python3.6: `namespace` # python3.7+: `None` or spec.origin in {None, "namespace"} + # we can only rewrite source files + or not isinstance(spec.loader, importlib.machinery.SourceFileLoader) # if the file doesn't exist, we can't rewrite it or not os.path.exists(spec.origin) ): From bd647fdd8b280e2753e07edd82170e00bcf841e0 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 25 Jun 2019 14:50:07 -0700 Subject: [PATCH 11/18] Revert allow_abbrev=False in helper scripts --- extra/get_issues.py | 2 +- scripts/release.py | 2 +- .../perf_examples/collect_stats/generate_folders.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/extra/get_issues.py b/extra/get_issues.py index ae99c9aa6..9407aeded 100644 --- a/extra/get_issues.py +++ b/extra/get_issues.py @@ -74,7 +74,7 @@ def report(issues): if __name__ == "__main__": import argparse - parser = argparse.ArgumentParser("process bitbucket issues", allow_abbrev=False) + parser = argparse.ArgumentParser("process bitbucket issues") parser.add_argument( "--refresh", action="store_true", help="invalidate cache, refresh issues" ) diff --git a/scripts/release.py b/scripts/release.py index d2a51e25a..5009df359 100644 --- a/scripts/release.py +++ b/scripts/release.py @@ -105,7 +105,7 @@ def changelog(version, write_out=False): def main(): init(autoreset=True) - parser = argparse.ArgumentParser(allow_abbrev=False) + parser = argparse.ArgumentParser() parser.add_argument("version", help="Release version") options = parser.parse_args() pre_release(options.version) diff --git a/testing/example_scripts/perf_examples/collect_stats/generate_folders.py b/testing/example_scripts/perf_examples/collect_stats/generate_folders.py index d2c1a30b2..ff1eaf7d6 100644 --- a/testing/example_scripts/perf_examples/collect_stats/generate_folders.py +++ b/testing/example_scripts/perf_examples/collect_stats/generate_folders.py @@ -4,7 +4,7 @@ import pathlib HERE = pathlib.Path(__file__).parent TEST_CONTENT = (HERE / "template_test.py").read_bytes() -parser = argparse.ArgumentParser(allow_abbrev=False) +parser = argparse.ArgumentParser() parser.add_argument("numbers", nargs="*", type=int) From 8c7eb8236348c8fb8db43a6a9c02ba442a09f6b8 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 15 May 2019 12:03:00 +0200 Subject: [PATCH 12/18] Fix/improve comparison of byte strings Fixes https://github.com/pytest-dev/pytest/issues/5260. --- changelog/5260.bugfix.rst | 1 + src/_pytest/assertion/util.py | 5 ++++- testing/test_assertion.py | 12 ++++++++++++ 3 files changed, 17 insertions(+), 1 deletion(-) create mode 100644 changelog/5260.bugfix.rst diff --git a/changelog/5260.bugfix.rst b/changelog/5260.bugfix.rst new file mode 100644 index 000000000..ab521d163 --- /dev/null +++ b/changelog/5260.bugfix.rst @@ -0,0 +1 @@ +Improve/fix comparison of byte strings with Python 3. diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 762e5761d..493c630f6 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -263,8 +263,11 @@ def _compare_eq_sequence(left, right, verbose=0): "At index {} diff: {!r} != {!r}".format(i, left[i], right[i]) ] break - len_diff = len_left - len_right + if isinstance(left, bytes): + return explanation + + len_diff = len_left - len_right if len_diff: if len_diff > 0: dir_with_more = "Left" diff --git a/testing/test_assertion.py b/testing/test_assertion.py index 0fcfd9f27..f9c67d703 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -331,6 +331,18 @@ class TestAssert_reprcompare: assert "- spam" in diff assert "+ eggs" in diff + def test_bytes_diff(self): + diff = callequal(b"spam", b"eggs") + if PY3: + assert diff == [ + "b'spam' == b'eggs'", + "At index 0 diff: 115 != 101", + "Use -v to get the full diff", + ] + else: + # Handled as text on Python 2. + assert diff == ["'spam' == 'eggs'", "- spam", "+ eggs"] + def test_list(self): expl = callequal([0, 1], [0, 2]) assert len(expl) > 1 From 3f2344e8f73784b40df28d82a44249827206dc63 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 25 Jun 2019 20:30:18 -0300 Subject: [PATCH 13/18] Show bytes ascii representation instead of numeric value --- changelog/5260.bugfix.rst | 18 +++++++++++++++++- src/_pytest/assertion/util.py | 22 ++++++++++++++++++++-- testing/test_assertion.py | 29 +++++++++++++++++++---------- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/changelog/5260.bugfix.rst b/changelog/5260.bugfix.rst index ab521d163..484c1438a 100644 --- a/changelog/5260.bugfix.rst +++ b/changelog/5260.bugfix.rst @@ -1 +1,17 @@ -Improve/fix comparison of byte strings with Python 3. +Improved comparison of byte strings. + +When comparing bytes, the assertion message used to show the byte numeric value when showing the differences:: + + def test(): + > assert b'spam' == b'eggs' + E AssertionError: assert b'spam' == b'eggs' + E At index 0 diff: 115 != 101 + E Use -v to get the full diff + +It now shows the actual ascii representation instead, which is often more useful:: + + def test(): + > assert b'spam' == b'eggs' + E AssertionError: assert b'spam' == b'eggs' + E At index 0 diff: b's' != b'e' + E Use -v to get the full diff diff --git a/src/_pytest/assertion/util.py b/src/_pytest/assertion/util.py index 493c630f6..b808cb509 100644 --- a/src/_pytest/assertion/util.py +++ b/src/_pytest/assertion/util.py @@ -254,17 +254,35 @@ def _compare_eq_iterable(left, right, verbose=0): def _compare_eq_sequence(left, right, verbose=0): + comparing_bytes = isinstance(left, bytes) and isinstance(right, bytes) explanation = [] len_left = len(left) len_right = len(right) for i in range(min(len_left, len_right)): if left[i] != right[i]: + if comparing_bytes: + # when comparing bytes, we want to see their ascii representation + # instead of their numeric values (#5260) + # using a slice gives us the ascii representation: + # >>> s = b'foo' + # >>> s[0] + # 102 + # >>> s[0:1] + # b'f' + left_value = left[i : i + 1] + right_value = right[i : i + 1] + else: + left_value = left[i] + right_value = right[i] + explanation += [ - "At index {} diff: {!r} != {!r}".format(i, left[i], right[i]) + "At index {} diff: {!r} != {!r}".format(i, left_value, right_value) ] break - if isinstance(left, bytes): + if comparing_bytes: + # when comparing bytes, it doesn't help to show the "sides contain one or more items" + # longer explanation, so skip it return explanation len_diff = len_left - len_right diff --git a/testing/test_assertion.py b/testing/test_assertion.py index f9c67d703..f58d240a5 100644 --- a/testing/test_assertion.py +++ b/testing/test_assertion.py @@ -331,17 +331,26 @@ class TestAssert_reprcompare: assert "- spam" in diff assert "+ eggs" in diff - def test_bytes_diff(self): + def test_bytes_diff_normal(self): + """Check special handling for bytes diff (#5260)""" diff = callequal(b"spam", b"eggs") - if PY3: - assert diff == [ - "b'spam' == b'eggs'", - "At index 0 diff: 115 != 101", - "Use -v to get the full diff", - ] - else: - # Handled as text on Python 2. - assert diff == ["'spam' == 'eggs'", "- spam", "+ eggs"] + + assert diff == [ + "b'spam' == b'eggs'", + "At index 0 diff: b's' != b'e'", + "Use -v to get the full diff", + ] + + def test_bytes_diff_verbose(self): + """Check special handling for bytes diff (#5260)""" + diff = callequal(b"spam", b"eggs", verbose=True) + assert diff == [ + "b'spam' == b'eggs'", + "At index 0 diff: b's' != b'e'", + "Full diff:", + "- b'spam'", + "+ b'eggs'", + ] def test_list(self): expl = callequal([0, 1], [0, 2]) From bfba33ec9e8f4c55d3184294473ade83c6300104 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Tue, 25 Jun 2019 20:24:13 -0700 Subject: [PATCH 14/18] Delete stray comment --- src/pytest.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pytest.py b/src/pytest.py index a3fa26084..b4faf4978 100644 --- a/src/pytest.py +++ b/src/pytest.py @@ -2,7 +2,6 @@ """ pytest: unit and functional testing with Python. """ -# else we are imported from _pytest import __version__ from _pytest.assertion import register_assert_rewrite from _pytest.config import cmdline From fdb6e35b1b9b19d53e00adc2f726140693702f3e Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 26 Jun 2019 20:23:35 +1000 Subject: [PATCH 15/18] Fix typo replace `circuting` with `circuiting`. --- src/_pytest/assertion/rewrite.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/assertion/rewrite.py b/src/_pytest/assertion/rewrite.py index f50d8200e..374bb2518 100644 --- a/src/_pytest/assertion/rewrite.py +++ b/src/_pytest/assertion/rewrite.py @@ -771,7 +771,7 @@ warn_explicit( fail_save = self.on_failure levels = len(boolop.values) - 1 self.push_format_context() - # Process each operand, short-circuting if needed. + # Process each operand, short-circuiting if needed. for i, v in enumerate(boolop.values): if i: fail_inner = [] From d81f758285499a8953e874b2b53ba8a5b1fa7430 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 26 Jun 2019 20:31:45 +1000 Subject: [PATCH 16/18] Update changelog with trivial as per ./CONTRIBUTING.rst --- changelog/5497.trivial.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog/5497.trivial.rst diff --git a/changelog/5497.trivial.rst b/changelog/5497.trivial.rst new file mode 100644 index 000000000..735d1f928 --- /dev/null +++ b/changelog/5497.trivial.rst @@ -0,0 +1 @@ +Fix typo replace `circuting` with `circuiting`. From 994c32235c04480415a0eae55c8eab7332cc5f07 Mon Sep 17 00:00:00 2001 From: Tim Gates Date: Wed, 26 Jun 2019 20:46:09 +1000 Subject: [PATCH 17/18] Fix rst support --- changelog/5497.trivial.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/5497.trivial.rst b/changelog/5497.trivial.rst index 735d1f928..4cd2087bf 100644 --- a/changelog/5497.trivial.rst +++ b/changelog/5497.trivial.rst @@ -1 +1 @@ -Fix typo replace `circuting` with `circuiting`. +Fix typo replace ``circuting`` with circuiting. From a48feb3261c46ecda88fb97172d84f9f22ec1a88 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Wed, 26 Jun 2019 11:09:04 -0300 Subject: [PATCH 18/18] Delete 5497.trivial.rst Just a typo, no need for a changelog entry. :) --- changelog/5497.trivial.rst | 1 - 1 file changed, 1 deletion(-) delete mode 100644 changelog/5497.trivial.rst diff --git a/changelog/5497.trivial.rst b/changelog/5497.trivial.rst deleted file mode 100644 index 4cd2087bf..000000000 --- a/changelog/5497.trivial.rst +++ /dev/null @@ -1 +0,0 @@ -Fix typo replace ``circuting`` with circuiting.