From 87edc09deaf56dc058631e9491596c54b1ce020b Mon Sep 17 00:00:00 2001 From: symonk Date: Mon, 13 Apr 2020 13:25:06 +0100 Subject: [PATCH 1/3] Gracefully handle eval() failure(s) for marker expressions --- AUTHORS | 1 + changelog/4583.bugfix.rst | 1 + src/_pytest/mark/legacy.py | 9 ++++++-- testing/test_mark.py | 47 +++++++++++++++++++++++++++++++++++--- 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 changelog/4583.bugfix.rst diff --git a/AUTHORS b/AUTHORS index 6efc41d9712..bbb8e463dc9 100644 --- a/AUTHORS +++ b/AUTHORS @@ -246,6 +246,7 @@ Segev Finer Serhii Mozghovyi Seth Junot Simon Gomizelj +Simon Kerr Skylar Downes Srinivas Reddy Thatiparthy Stefan Farmbauer diff --git a/changelog/4583.bugfix.rst b/changelog/4583.bugfix.rst new file mode 100644 index 00000000000..64a680f1429 --- /dev/null +++ b/changelog/4583.bugfix.rst @@ -0,0 +1 @@ +Prevent crashing and provide user friendly error(s) when marker expressions (-m) invoking of eval() raises a SyntaxError or TypeError diff --git a/src/_pytest/mark/legacy.py b/src/_pytest/mark/legacy.py index 80a520a0a9e..7581421daf1 100644 --- a/src/_pytest/mark/legacy.py +++ b/src/_pytest/mark/legacy.py @@ -84,8 +84,13 @@ def matchmark(colitem, markexpr): """Tries to match on any marker names, attached to the given colitem.""" try: return eval(markexpr, {}, MarkMapping.from_item(colitem)) - except SyntaxError as e: - raise SyntaxError(str(e) + "\nMarker expression must be valid Python!") + except (SyntaxError, TypeError): + raise UsageError( + "Marker expression provided to -m:{} was not valid python syntax." + " Please check the syntax provided and ensure it is correct".format( + markexpr + ) + ) def matchkeyword(colitem, keywordexpr): diff --git a/testing/test_mark.py b/testing/test_mark.py index 530f9f1688c..69f751012cb 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -608,10 +608,13 @@ def test_a(): pass """ ) - result = testdir.runpytest("-m bogus/") - result.stdout.fnmatch_lines( - ["INTERNALERROR> Marker expression must be valid Python!"] + expected = ( + "ERROR: Marker expression provided to -m: bogus/ was not valid python syntax. Please " + "check the syntax provided and ensure it is correct" ) + result = testdir.runpytest("-m bogus/") + result.stderr.fnmatch_lines([expected]) + assert result.ret == ExitCode.USAGE_ERROR def test_keywords_at_node_level(self, testdir): testdir.makepyfile( @@ -1022,3 +1025,41 @@ def test_pytest_param_id_requires_string(): @pytest.mark.parametrize("s", (None, "hello world")) def test_pytest_param_id_allows_none_or_string(s): assert pytest.param(id=s) + + +def test_ux_eval_syntax_error(testdir): + foo = testdir.makepyfile( + """ + import pytest + + @pytest.mark.internal_err + def test_foo(): + pass + """ + ) + expected = ( + "ERROR: Marker expression provided to -m: NOT internal_err was not valid python syntax. Please " + "check the syntax provided and ensure it is correct" + ) + result = testdir.runpytest(foo, "-m NOT internal_err") + result.stderr.fnmatch_lines([expected]) + assert result.ret == ExitCode.USAGE_ERROR + + +def test_ux_eval_type_error(testdir): + foo = testdir.makepyfile( + """ + import pytest + + @pytest.mark.internal_err + def test_foo(): + pass + """ + ) + expected = ( + "ERROR: Marker expression provided to -m: NOT (internal_err) was not valid python syntax. Please " + "check the syntax provided and ensure it is correct" + ) + result = testdir.runpytest(foo, "-m NOT (internal_err)") + result.stderr.fnmatch_lines([expected]) + assert result.ret == ExitCode.USAGE_ERROR From 251e8f212e49a94b999c057fa5164a11faa7eed2 Mon Sep 17 00:00:00 2001 From: symonk Date: Mon, 13 Apr 2020 14:25:01 +0100 Subject: [PATCH 2/3] refactor mark tests, widen catching and make error msg more concise --- src/_pytest/mark/legacy.py | 9 ++------ testing/test_mark.py | 44 ++++---------------------------------- 2 files changed, 6 insertions(+), 47 deletions(-) diff --git a/src/_pytest/mark/legacy.py b/src/_pytest/mark/legacy.py index 7581421daf1..eb50340f249 100644 --- a/src/_pytest/mark/legacy.py +++ b/src/_pytest/mark/legacy.py @@ -84,13 +84,8 @@ def matchmark(colitem, markexpr): """Tries to match on any marker names, attached to the given colitem.""" try: return eval(markexpr, {}, MarkMapping.from_item(colitem)) - except (SyntaxError, TypeError): - raise UsageError( - "Marker expression provided to -m:{} was not valid python syntax." - " Please check the syntax provided and ensure it is correct".format( - markexpr - ) - ) + except Exception: + raise UsageError("Wrong expression passed to '-m': {}".format(markexpr)) def matchkeyword(colitem, keywordexpr): diff --git a/testing/test_mark.py b/testing/test_mark.py index 69f751012cb..2aad2b1ba5a 100644 --- a/testing/test_mark.py +++ b/testing/test_mark.py @@ -601,21 +601,6 @@ def test_unmarked(): deselected_tests = dlist[0].items assert len(deselected_tests) == 2 - def test_invalid_m_option(self, testdir): - testdir.makepyfile( - """ - def test_a(): - pass - """ - ) - expected = ( - "ERROR: Marker expression provided to -m: bogus/ was not valid python syntax. Please " - "check the syntax provided and ensure it is correct" - ) - result = testdir.runpytest("-m bogus/") - result.stderr.fnmatch_lines([expected]) - assert result.ret == ExitCode.USAGE_ERROR - def test_keywords_at_node_level(self, testdir): testdir.makepyfile( """ @@ -1027,26 +1012,8 @@ def test_pytest_param_id_allows_none_or_string(s): assert pytest.param(id=s) -def test_ux_eval_syntax_error(testdir): - foo = testdir.makepyfile( - """ - import pytest - - @pytest.mark.internal_err - def test_foo(): - pass - """ - ) - expected = ( - "ERROR: Marker expression provided to -m: NOT internal_err was not valid python syntax. Please " - "check the syntax provided and ensure it is correct" - ) - result = testdir.runpytest(foo, "-m NOT internal_err") - result.stderr.fnmatch_lines([expected]) - assert result.ret == ExitCode.USAGE_ERROR - - -def test_ux_eval_type_error(testdir): +@pytest.mark.parametrize("expr", ("NOT internal_err", "NOT (internal_err)", "bogus/")) +def test_marker_expr_eval_failure_handling(testdir, expr): foo = testdir.makepyfile( """ import pytest @@ -1056,10 +1023,7 @@ def test_foo(): pass """ ) - expected = ( - "ERROR: Marker expression provided to -m: NOT (internal_err) was not valid python syntax. Please " - "check the syntax provided and ensure it is correct" - ) - result = testdir.runpytest(foo, "-m NOT (internal_err)") + expected = "ERROR: Wrong expression passed to '-m': {}".format(expr) + result = testdir.runpytest(foo, "-m", expr) result.stderr.fnmatch_lines([expected]) assert result.ret == ExitCode.USAGE_ERROR From 6fd30134d323919ed38c8c9fe0785f7d9e0779ca Mon Sep 17 00:00:00 2001 From: Simon K Date: Mon, 13 Apr 2020 14:29:59 +0100 Subject: [PATCH 3/3] Update changelog/4583.bugfix.rst Co-Authored-By: Ran Benita --- changelog/4583.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog/4583.bugfix.rst b/changelog/4583.bugfix.rst index 64a680f1429..f0a82030338 100644 --- a/changelog/4583.bugfix.rst +++ b/changelog/4583.bugfix.rst @@ -1 +1 @@ -Prevent crashing and provide user friendly error(s) when marker expressions (-m) invoking of eval() raises a SyntaxError or TypeError +Prevent crashing and provide a user-friendly error when a marker expression (-m) invoking of eval() raises any exception.