From 2c7f94fdb94d166593384ec3455a52654a735e8b Mon Sep 17 00:00:00 2001 From: Andrew Toolan Date: Sat, 20 Jan 2018 18:23:08 +0000 Subject: [PATCH 1/7] Added basic fix and test --- _pytest/config.py | 5 ++++- testing/test_config.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/_pytest/config.py b/_pytest/config.py index 22bf6c60c..544c260ee 100644 --- a/_pytest/config.py +++ b/_pytest/config.py @@ -1192,12 +1192,15 @@ class Config(object): # and -o foo1=bar1 -o foo2=bar2 options # always use the last item if multiple value set for same ini-name, # e.g. -o foo=bar1 -o foo=bar2 will set foo to bar2 + first_override_set = False for ini_config_list in self._override_ini: for ini_config in ini_config_list: try: (key, user_ini_value) = ini_config.split("=", 1) + first_override_set = True except ValueError: - raise UsageError("-o/--override-ini expects option=value style.") + if not first_override_set: + raise UsageError("-o/--override-ini expects option=value style.") if key == name: value = user_ini_value return value diff --git a/testing/test_config.py b/testing/test_config.py index 44b8c317a..f958b5890 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -860,3 +860,40 @@ class TestOverrideIniArgs(object): config = get_config() config._preparse([], addopts=True) assert config._override_ini == [['cache_dir=%s' % cache_dir]] + + def test_all_the_things(self, testdir): + testdir.makeconftest(""" + def pytest_addoption(parser): + addini = parser.addini + addini("custom_option_1", "", default="o1") + addini("custom_option_2", "", default="o2")""") + testdir.makepyfile(""" + def test_multiple_options(pytestconfig): + prefix = "custom_option" + for x in range(1, 3): + ini_value=pytestconfig.getini("%s_%d" % (prefix, x)) + print('\\nini%d:%s' % (x, ini_value))""") + + result = testdir.runpytest( + "--override-ini", 'custom_option_1=fulldir=/tmp/user1', + 'custom_option_2=url=/tmp/user2?a=b&d=e', + "test_all_the_things.py") + assert "ERROR: -o/--override-ini expects option=value style." not in result.stderr.str() + + def test_throw_exception_if_not_value_pair(self, testdir): + testdir.makeconftest(""" + def pytest_addoption(parser): + addini = parser.addini + addini("custom_option_1", "", default="o1") + addini("custom_option_2", "", default="o2")""") + testdir.makepyfile(""" + def test_multiple_options(pytestconfig): + prefix = "custom_option" + for x in range(1, 3): + ini_value=pytestconfig.getini("%s_%d" % (prefix, x)) + print('\\nini%d:%s' % (x, ini_value))""") + + result = testdir.runpytest( + "--override-ini", 'custom_option_1', + "test_all_the_things.py") + assert "ERROR: -o/--override-ini expects option=value style." in result.stderr.str() From 203508d9f3ea4478317375c1bdf09a8db02c8826 Mon Sep 17 00:00:00 2001 From: Aron Coyle Date: Sat, 20 Jan 2018 21:44:08 +0000 Subject: [PATCH 2/7] cleanup test cases --- testing/test_config.py | 44 ++++++++---------------------------------- 1 file changed, 8 insertions(+), 36 deletions(-) diff --git a/testing/test_config.py b/testing/test_config.py index f958b5890..57c719edc 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -861,39 +861,11 @@ class TestOverrideIniArgs(object): config._preparse([], addopts=True) assert config._override_ini == [['cache_dir=%s' % cache_dir]] - def test_all_the_things(self, testdir): - testdir.makeconftest(""" - def pytest_addoption(parser): - addini = parser.addini - addini("custom_option_1", "", default="o1") - addini("custom_option_2", "", default="o2")""") - testdir.makepyfile(""" - def test_multiple_options(pytestconfig): - prefix = "custom_option" - for x in range(1, 3): - ini_value=pytestconfig.getini("%s_%d" % (prefix, x)) - print('\\nini%d:%s' % (x, ini_value))""") - - result = testdir.runpytest( - "--override-ini", 'custom_option_1=fulldir=/tmp/user1', - 'custom_option_2=url=/tmp/user2?a=b&d=e', - "test_all_the_things.py") - assert "ERROR: -o/--override-ini expects option=value style." not in result.stderr.str() - - def test_throw_exception_if_not_value_pair(self, testdir): - testdir.makeconftest(""" - def pytest_addoption(parser): - addini = parser.addini - addini("custom_option_1", "", default="o1") - addini("custom_option_2", "", default="o2")""") - testdir.makepyfile(""" - def test_multiple_options(pytestconfig): - prefix = "custom_option" - for x in range(1, 3): - ini_value=pytestconfig.getini("%s_%d" % (prefix, x)) - print('\\nini%d:%s' % (x, ini_value))""") - - result = testdir.runpytest( - "--override-ini", 'custom_option_1', - "test_all_the_things.py") - assert "ERROR: -o/--override-ini expects option=value style." in result.stderr.str() + def test_no_error_if_true_first_key_value_pair(self, testdir): + testdir.makeini(""" + [pytest] + xdist_strict=False + """) + result = testdir.runpytest('--override-ini', 'xdist_strict=True', + 'test_no_error_if_true_first_key_value_pair.py') + assert 'ERROR: -o/--override-ini expects option=value style.' not in result.stderr.str() From 46d87deb5d38d6138947d981158ca2839c3ad8d7 Mon Sep 17 00:00:00 2001 From: Aron Coyle Date: Sat, 20 Jan 2018 22:00:38 +0000 Subject: [PATCH 3/7] Add changelog update authors --- AUTHORS | 1 + changelog/3103.bugfix | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog/3103.bugfix diff --git a/AUTHORS b/AUTHORS index 8d981ae9a..b01f4bd85 100644 --- a/AUTHORS +++ b/AUTHORS @@ -18,6 +18,7 @@ Anthon van der Neut Anthony Sottile Antony Lee Armin Rigo +Aron Coyle Aron Curzon Aviv Palivoda Barney Gale diff --git a/changelog/3103.bugfix b/changelog/3103.bugfix new file mode 100644 index 000000000..e650c035d --- /dev/null +++ b/changelog/3103.bugfix @@ -0,0 +1 @@ +Fixed UsageError being raised when specifying config override options followed by test path \ No newline at end of file From 30ca9f9d3890c7d83e34014a8ff096744db7e690 Mon Sep 17 00:00:00 2001 From: Aron Coyle Date: Sat, 20 Jan 2018 22:02:44 +0000 Subject: [PATCH 4/7] Add endline --- changelog/3103.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/3103.bugfix b/changelog/3103.bugfix index e650c035d..116795b43 100644 --- a/changelog/3103.bugfix +++ b/changelog/3103.bugfix @@ -1 +1 @@ -Fixed UsageError being raised when specifying config override options followed by test path \ No newline at end of file +Fixed UsageError being raised when specifying config override options followed by test path From 8426c57a9e8631186a6cadcedd14756246218efb Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 22 Jan 2018 17:58:04 -0200 Subject: [PATCH 5/7] Ensure changes in the message in the future do not make the test pass by accident --- testing/test_config.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/testing/test_config.py b/testing/test_config.py index 57c719edc..6ae42a829 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -861,11 +861,16 @@ class TestOverrideIniArgs(object): config._preparse([], addopts=True) assert config._override_ini == [['cache_dir=%s' % cache_dir]] - def test_no_error_if_true_first_key_value_pair(self, testdir): + def test_no_error_if_true_first_key_value_pair(self, testdir, request): + """Ensure a file path following a '-o' option does not generate an error (#3103)""" testdir.makeini(""" [pytest] xdist_strict=False """) - result = testdir.runpytest('--override-ini', 'xdist_strict=True', - 'test_no_error_if_true_first_key_value_pair.py') - assert 'ERROR: -o/--override-ini expects option=value style.' not in result.stderr.str() + testdir.makepyfile(""" + def test(): + pass + """) + result = testdir.runpytest('--override-ini', 'xdist_strict=True', '{}.py'.format(request.node.name)) + assert 'ERROR:' not in result.stderr.str() + result.stdout.fnmatch_lines('* 1 passed in *') From 443275f0258715a8975ecf2f003f2919cc00d94e Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Mon, 22 Jan 2018 17:59:00 -0200 Subject: [PATCH 6/7] Reword changelog a bit --- changelog/3103.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/3103.bugfix b/changelog/3103.bugfix index 116795b43..ec493eb4e 100644 --- a/changelog/3103.bugfix +++ b/changelog/3103.bugfix @@ -1 +1 @@ -Fixed UsageError being raised when specifying config override options followed by test path +Fix ``UsageError`` being raised when specifying ``-o/--override`` command-line option followed by a test path. From 3f5e9ea71e997746f89c71507888414f9477e2b9 Mon Sep 17 00:00:00 2001 From: Bruno Oliveira Date: Tue, 23 Jan 2018 21:13:20 -0200 Subject: [PATCH 7/7] Fix -o behavior to no longer swallow all remaining options The current behavior was too error-prone because a "-o" option would swallow all the following non-option parameters: pytest -o foo=bar path/to/test.py path/to/test.py would be captured by the -o option, and would fail because "path/to/test.py" is not in the format "key=value". --- _pytest/config.py | 20 +++++++--------- _pytest/helpconfig.py | 4 ++-- changelog/3103.bugfix | 2 +- doc/en/customize.rst | 17 +++++++++++++- testing/test_config.py | 52 +++++++++++++++++++++++++++++------------- 5 files changed, 63 insertions(+), 32 deletions(-) diff --git a/_pytest/config.py b/_pytest/config.py index 544c260ee..a9ded208e 100644 --- a/_pytest/config.py +++ b/_pytest/config.py @@ -1188,19 +1188,15 @@ class Config(object): def _get_override_ini_value(self, name): value = None - # override_ini is a list of list, to support both -o foo1=bar1 foo2=bar2 and - # and -o foo1=bar1 -o foo2=bar2 options - # always use the last item if multiple value set for same ini-name, + # override_ini is a list of "ini=value" options + # always use the last item if multiple values are set for same ini-name, # e.g. -o foo=bar1 -o foo=bar2 will set foo to bar2 - first_override_set = False - for ini_config_list in self._override_ini: - for ini_config in ini_config_list: - try: - (key, user_ini_value) = ini_config.split("=", 1) - first_override_set = True - except ValueError: - if not first_override_set: - raise UsageError("-o/--override-ini expects option=value style.") + for ini_config in self._override_ini: + try: + key, user_ini_value = ini_config.split("=", 1) + except ValueError: + raise UsageError("-o/--override-ini expects option=value style.") + else: if key == name: value = user_ini_value return value diff --git a/_pytest/helpconfig.py b/_pytest/helpconfig.py index e744637f8..5a81a5bd3 100644 --- a/_pytest/helpconfig.py +++ b/_pytest/helpconfig.py @@ -57,9 +57,9 @@ def pytest_addoption(parser): action="store_true", dest="debug", default=False, help="store internal tracing debug information in 'pytestdebug.log'.") group._addoption( - '-o', '--override-ini', nargs='*', dest="override_ini", + '-o', '--override-ini', dest="override_ini", action="append", - help="override config option with option=value style, e.g. `-o xfail_strict=True`.") + help='override ini option with "option=value" style, e.g. `-o xfail_strict=True -o cache_dir=cache`.') @pytest.hookimpl(hookwrapper=True) diff --git a/changelog/3103.bugfix b/changelog/3103.bugfix index ec493eb4e..4bdb23820 100644 --- a/changelog/3103.bugfix +++ b/changelog/3103.bugfix @@ -1 +1 @@ -Fix ``UsageError`` being raised when specifying ``-o/--override`` command-line option followed by a test path. +**Incompatible change**: ``-o/--override`` option no longer eats all the remaining options, which can lead to surprising behavior: for example, ``pytest -o foo=1 /path/to/test.py`` would fail because ``/path/to/test.py`` would be considered as part of the ``-o`` command-line argument. One consequence of this is that now multiple configuration overrides need multiple ``-o`` flags: ``pytest -o foo=1 -o bar=2``. diff --git a/doc/en/customize.rst b/doc/en/customize.rst index 8133704a5..6edeedf98 100644 --- a/doc/en/customize.rst +++ b/doc/en/customize.rst @@ -152,11 +152,25 @@ above will show verbose output because ``-v`` overwrites ``-q``. Builtin configuration file options ---------------------------------------------- +Here is a list of builtin configuration options that may be written in a ``pytest.ini``, ``tox.ini`` or ``setup.cfg`` +file, usually located at the root of your repository. All options must be under a ``[pytest]`` section +(``[tool:pytest]`` for ``setup.cfg`` files). + +Configuration file options may be overwritten in the command-line by using ``-o/--override``, which can also be +passed multiple times. The expected format is ``name=value``. For example:: + + pytest -o console_output_style=classic -o cache_dir=/tmp/mycache + + .. confval:: minversion Specifies a minimal pytest version required for running tests. - minversion = 2.1 # will fail if we run with pytest-2.0 + .. code-block:: ini + + # content of pytest.ini + [pytest] + minversion = 3.0 # will fail if we run with pytest-2.8 .. confval:: addopts @@ -165,6 +179,7 @@ Builtin configuration file options .. code-block:: ini + # content of pytest.ini [pytest] addopts = --maxfail=2 -rf # exit after 2 failures, report fail info diff --git a/testing/test_config.py b/testing/test_config.py index 6ae42a829..f5b11775a 100644 --- a/testing/test_config.py +++ b/testing/test_config.py @@ -781,16 +781,18 @@ class TestOverrideIniArgs(object): testdir.makeini(""" [pytest] custom_option_1=custom_option_1 - custom_option_2=custom_option_2""") + custom_option_2=custom_option_2 + """) testdir.makepyfile(""" def test_multiple_options(pytestconfig): prefix = "custom_option" for x in range(1, 5): ini_value=pytestconfig.getini("%s_%d" % (prefix, x)) - print('\\nini%d:%s' % (x, ini_value))""") + print('\\nini%d:%s' % (x, ini_value)) + """) result = testdir.runpytest( "--override-ini", 'custom_option_1=fulldir=/tmp/user1', - 'custom_option_2=url=/tmp/user2?a=b&d=e', + '-o', 'custom_option_2=url=/tmp/user2?a=b&d=e', "-o", 'custom_option_3=True', "-o", 'custom_option_4=no', "-s") result.stdout.fnmatch_lines(["ini1:fulldir=/tmp/user1", @@ -853,24 +855,42 @@ class TestOverrideIniArgs(object): assert rootdir == tmpdir assert inifile is None - def test_addopts_before_initini(self, testdir, tmpdir, monkeypatch): + def test_addopts_before_initini(self, monkeypatch): cache_dir = '.custom_cache' monkeypatch.setenv('PYTEST_ADDOPTS', '-o cache_dir=%s' % cache_dir) from _pytest.config import get_config config = get_config() config._preparse([], addopts=True) - assert config._override_ini == [['cache_dir=%s' % cache_dir]] + assert config._override_ini == ['cache_dir=%s' % cache_dir] - def test_no_error_if_true_first_key_value_pair(self, testdir, request): + def test_override_ini_does_not_contain_paths(self): + """Check that -o no longer swallows all options after it (#3103)""" + from _pytest.config import get_config + config = get_config() + config._preparse(['-o', 'cache_dir=/cache', '/some/test/path']) + assert config._override_ini == ['cache_dir=/cache'] + + def test_multiple_override_ini_options(self, testdir, request): """Ensure a file path following a '-o' option does not generate an error (#3103)""" - testdir.makeini(""" - [pytest] - xdist_strict=False - """) - testdir.makepyfile(""" - def test(): - pass - """) - result = testdir.runpytest('--override-ini', 'xdist_strict=True', '{}.py'.format(request.node.name)) + testdir.makepyfile(**{ + "conftest.py": """ + def pytest_addoption(parser): + parser.addini('foo', default=None, help='some option') + parser.addini('bar', default=None, help='some option') + """, + "test_foo.py": """ + def test(pytestconfig): + assert pytestconfig.getini('foo') == '1' + assert pytestconfig.getini('bar') == '0' + """, + "test_bar.py": """ + def test(): + assert False + """, + }) + result = testdir.runpytest('-o', 'foo=1', '-o', 'bar=0', 'test_foo.py') assert 'ERROR:' not in result.stderr.str() - result.stdout.fnmatch_lines('* 1 passed in *') + result.stdout.fnmatch_lines([ + 'collected 1 item', + '*= 1 passed in *=', + ])