From da100195b77cea11bc64850ec4343afc30d897c6 Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 20:59:39 +0100 Subject: [PATCH 1/9] monkeypatch os.name for the test in CI, rewrite parametrized IDs greater than 100 on windows --- src/_pytest/python.py | 10 +++++++++- testing/python/metafunc.py | 26 ++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 76fccb4a18d..a693302933e 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -6,6 +6,7 @@ import os import sys import typing +import uuid import warnings from collections import Counter from collections import defaultdict @@ -1230,8 +1231,15 @@ def idmaker( _idvalset(valindex, parameterset, argnames, idfn, ids, config=config, item=item) for valindex, parameterset in enumerate(parametersets) ] + # rewrite parametrized ids greater than the overridable limit on windows + if os.name == "nt": + limit = 100 + rewrite_template = "auto-generated-{}" + resolved_ids = [ + x if len(x) < limit else rewrite_template.format(uuid.uuid4()) + for x in resolved_ids + ] - # All IDs must be unique! unique_ids = set(resolved_ids) if len(unique_ids) != len(resolved_ids): diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 4d41910982b..45b21e773bd 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1,4 +1,5 @@ import itertools +import os import re import sys import textwrap @@ -1899,3 +1900,28 @@ def test_converted_to_str(a, b): "*= 6 passed in *", ] ) + + +def test_windows_autogen_result(monkeypatch, testdir): + # we use '93 in test 2 instead of 99 as parametrization is going to append '-False' onto the resolved_id. + # likewise true uses '95' as '-True' will be appended by resolved_id. + # This test implicitly covers the --win-long-id-limit default. + monkeypatch.setattr(os, "name", "nt") + testdir.makepyfile( + """ + import pytest + @pytest.mark.parametrize('value, expected', + [('*' * 101, True), + ('*' * 93, False), + ('*' * 95, True), (True, False), + (2147000000, False) + ]) + def test_something(request, value, expected): + node_id = request.node.nodeid + print('data was: {} ~ {}'.format(value, expected)) + result = 'auto-generated-' in node_id + assert result == expected + """ + ) + result = testdir.runpytest() + result.assert_outcomes(passed=5) From 33284ffba60a2380bcbc7e014e468f1261ce368e Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 21:11:07 +0100 Subject: [PATCH 2/9] tidy up comments --- src/_pytest/python.py | 2 +- testing/python/metafunc.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index a693302933e..4bc8b8347c5 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1231,7 +1231,7 @@ def idmaker( _idvalset(valindex, parameterset, argnames, idfn, ids, config=config, item=item) for valindex, parameterset in enumerate(parametersets) ] - # rewrite parametrized ids greater than the overridable limit on windows + # rewrite parametrized ids greater than a 100 limit if os.name == "nt": limit = 100 rewrite_template = "auto-generated-{}" diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 45b21e773bd..cb12318eac9 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1905,7 +1905,6 @@ def test_converted_to_str(a, b): def test_windows_autogen_result(monkeypatch, testdir): # we use '93 in test 2 instead of 99 as parametrization is going to append '-False' onto the resolved_id. # likewise true uses '95' as '-True' will be appended by resolved_id. - # This test implicitly covers the --win-long-id-limit default. monkeypatch.setattr(os, "name", "nt") testdir.makepyfile( """ From e5ee3fa58f337305b4968134fc8b9cfecf756fb0 Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 21:19:18 +0100 Subject: [PATCH 3/9] remove print statements from tests --- testing/python/metafunc.py | 1 - 1 file changed, 1 deletion(-) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index cb12318eac9..43cb975f9b1 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1917,7 +1917,6 @@ def test_windows_autogen_result(monkeypatch, testdir): ]) def test_something(request, value, expected): node_id = request.node.nodeid - print('data was: {} ~ {}'.format(value, expected)) result = 'auto-generated-' in node_id assert result == expected """ From 9f4f5aecfa565ad37975a490546f5833c4068309 Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 21:22:28 +0100 Subject: [PATCH 4/9] add accidentally removed comments --- src/_pytest/python.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4bc8b8347c5..4ed06cfe3aa 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1240,6 +1240,7 @@ def idmaker( for x in resolved_ids ] + # All IDs must be unique! unique_ids = set(resolved_ids) if len(unique_ids) != len(resolved_ids): From 89bbabe1c95d1d374bc13df9e87a4be362eba472 Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 21:33:15 +0100 Subject: [PATCH 5/9] skip if not running on windows based systems --- testing/python/metafunc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 43cb975f9b1..6f417b58af3 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1902,6 +1902,7 @@ def test_converted_to_str(a, b): ) +@pytest.mark.skipif(os.name != "nt", reason="relies solely on windows based OS") def test_windows_autogen_result(monkeypatch, testdir): # we use '93 in test 2 instead of 99 as parametrization is going to append '-False' onto the resolved_id. # likewise true uses '95' as '-True' will be appended by resolved_id. From 5f78d4c833e9c53e0f9e38a738d81a47fb3f6260 Mon Sep 17 00:00:00 2001 From: symonk Date: Sun, 24 May 2020 21:45:38 +0100 Subject: [PATCH 6/9] change the solution to be platform agnostic for consistency --- src/_pytest/python.py | 15 +++++++-------- testing/python/metafunc.py | 5 +---- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 4ed06cfe3aa..fe481ed8c7a 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1231,14 +1231,13 @@ def idmaker( _idvalset(valindex, parameterset, argnames, idfn, ids, config=config, item=item) for valindex, parameterset in enumerate(parametersets) ] - # rewrite parametrized ids greater than a 100 limit - if os.name == "nt": - limit = 100 - rewrite_template = "auto-generated-{}" - resolved_ids = [ - x if len(x) < limit else rewrite_template.format(uuid.uuid4()) - for x in resolved_ids - ] + # rewrite extremely long parametrized ids + limit = 100 + rewrite_template = "auto-generated-{}" + resolved_ids = [ + x if len(x) < limit else rewrite_template.format(uuid.uuid4()) + for x in resolved_ids + ] # All IDs must be unique! unique_ids = set(resolved_ids) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 6f417b58af3..70d526ce351 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -1,5 +1,4 @@ import itertools -import os import re import sys import textwrap @@ -1902,11 +1901,9 @@ def test_converted_to_str(a, b): ) -@pytest.mark.skipif(os.name != "nt", reason="relies solely on windows based OS") -def test_windows_autogen_result(monkeypatch, testdir): +def test_auto_generated_long_parametrized_ids(testdir): # we use '93 in test 2 instead of 99 as parametrization is going to append '-False' onto the resolved_id. # likewise true uses '95' as '-True' will be appended by resolved_id. - monkeypatch.setattr(os, "name", "nt") testdir.makepyfile( """ import pytest From 70b0774be62885fb8e365cab83a095670925ce79 Mon Sep 17 00:00:00 2001 From: symonk Date: Mon, 25 May 2020 14:45:37 +0100 Subject: [PATCH 7/9] refactor auto id rewriting to use the inbuilt argname-idx capabilities --- src/_pytest/python.py | 28 +++++++++++----------- testing/python/metafunc.py | 48 ++++++++++++++++++++++++-------------- 2 files changed, 43 insertions(+), 33 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index fe481ed8c7a..a28cc814f9a 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -6,7 +6,6 @@ import os import sys import typing -import uuid import warnings from collections import Counter from collections import defaultdict @@ -1182,19 +1181,25 @@ def _idval( if hook_id: return hook_id + actual_val = None # empty strings are valid as val if isinstance(val, STRING_TYPES): - return _ascii_escaped_by_config(val, config) + actual_val = _ascii_escaped_by_config(val, config) elif val is None or isinstance(val, (float, int, bool)): - return str(val) + actual_val = str(val) elif isinstance(val, REGEX_TYPE): - return ascii_escaped(val.pattern) + actual_val = ascii_escaped(val.pattern) elif isinstance(val, enum.Enum): - return str(val) + actual_val = str(val) elif isinstance(getattr(val, "__name__", None), str): # name of a class, function, module, etc. - name = getattr(val, "__name__") # type: str - return name - return str(argname) + str(idx) + actual_val = getattr(val, "__name__") + + # Check if the post-checks value is greater than 100 chars in length and use auto id gen if so + # isinstance checks can return empty strings which is considered valid, so we explicitly check None on actual_val + # for example @pytest.mark.parametrize('x', ('', ' ')) is acceptable + if actual_val is None or len(actual_val) > 100: + return str(argname) + str(idx) + return actual_val def _idvalset( @@ -1231,13 +1236,6 @@ def idmaker( _idvalset(valindex, parameterset, argnames, idfn, ids, config=config, item=item) for valindex, parameterset in enumerate(parametersets) ] - # rewrite extremely long parametrized ids - limit = 100 - rewrite_template = "auto-generated-{}" - resolved_ids = [ - x if len(x) < limit else rewrite_template.format(uuid.uuid4()) - for x in resolved_ids - ] # All IDs must be unique! unique_ids = set(resolved_ids) diff --git a/testing/python/metafunc.py b/testing/python/metafunc.py index 70d526ce351..a8cb4fb5eca 100644 --- a/testing/python/metafunc.py +++ b/testing/python/metafunc.py @@ -286,9 +286,8 @@ def test_unicode_idval(self) -> None: ("ação", "a\\xe7\\xe3o"), ("josé@blah.com", "jos\\xe9@blah.com"), ( - "δοκ.ιμή@παράδειγμα.δοκιμή", - "\\u03b4\\u03bf\\u03ba.\\u03b9\\u03bc\\u03ae@\\u03c0\\u03b1\\u03c1\\u03ac\\u03b4\\u03b5\\u03b9\\u03b3" - "\\u03bc\\u03b1.\\u03b4\\u03bf\\u03ba\\u03b9\\u03bc\\u03ae", + "δοκ.ιμή@δοκιμή", + "\\u03b4\\u03bf\\u03ba.\\u03b9\\u03bc\\u03ae@\\u03b4\\u03bf\\u03ba\\u03b9\\u03bc\\u03ae", ), ] for val, expected in values: @@ -1902,22 +1901,35 @@ def test_converted_to_str(a, b): def test_auto_generated_long_parametrized_ids(testdir): - # we use '93 in test 2 instead of 99 as parametrization is going to append '-False' onto the resolved_id. - # likewise true uses '95' as '-True' will be appended by resolved_id. testdir.makepyfile( """ - import pytest - @pytest.mark.parametrize('value, expected', - [('*' * 101, True), - ('*' * 93, False), - ('*' * 95, True), (True, False), - (2147000000, False) - ]) - def test_something(request, value, expected): - node_id = request.node.nodeid - result = 'auto-generated-' in node_id - assert result == expected - """ + import pytest + from datetime import datetime + from enum import Enum + + class CustomEnum(Enum): + OVER = "over" * 26 + UNDER = "under" * 2 + + @pytest.mark.parametrize( + "value, expected", + [ + ("*" * 101, True), + (b"*" * 101, True), + (25, False), + (50.25, False), + ("*" * 50, False), + (True, False), + (datetime(2001, 12, 12), True), + (CustomEnum.OVER, False), + (datetime, False), + (dir, False), + ]) + def test_parametrized_auto_generating_long(request, value, expected): + node_name = request.node.name + was_rewritten = "value" in node_name + assert was_rewritten == expected + """ ) result = testdir.runpytest() - result.assert_outcomes(passed=5) + result.assert_outcomes(passed=10) From b2462d8a9e107d13cd7f18778e2d37ded3ae67dc Mon Sep 17 00:00:00 2001 From: symonk Date: Mon, 25 May 2020 14:47:17 +0100 Subject: [PATCH 8/9] use a more appropriate name for the post-checked val --- src/_pytest/python.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index a28cc814f9a..41cdf938c3b 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1181,25 +1181,25 @@ def _idval( if hook_id: return hook_id - actual_val = None # empty strings are valid as val + modified_val = None # empty strings are valid as val if isinstance(val, STRING_TYPES): - actual_val = _ascii_escaped_by_config(val, config) + modified_val = _ascii_escaped_by_config(val, config) elif val is None or isinstance(val, (float, int, bool)): - actual_val = str(val) + modified_val = str(val) elif isinstance(val, REGEX_TYPE): - actual_val = ascii_escaped(val.pattern) + modified_val = ascii_escaped(val.pattern) elif isinstance(val, enum.Enum): - actual_val = str(val) + modified_val = str(val) elif isinstance(getattr(val, "__name__", None), str): # name of a class, function, module, etc. - actual_val = getattr(val, "__name__") + modified_val = getattr(val, "__name__") # Check if the post-checks value is greater than 100 chars in length and use auto id gen if so - # isinstance checks can return empty strings which is considered valid, so we explicitly check None on actual_val + # isinstance checks can return empty strings which is considered valid, so we explicitly check None on modified_val # for example @pytest.mark.parametrize('x', ('', ' ')) is acceptable - if actual_val is None or len(actual_val) > 100: + if modified_val is None or len(modified_val) > 100: return str(argname) + str(idx) - return actual_val + return modified_val def _idvalset( From ce48153d99844d1207da08eb28501e7d99eb7c7d Mon Sep 17 00:00:00 2001 From: symonk Date: Mon, 25 May 2020 19:06:14 +0100 Subject: [PATCH 9/9] make code comments a bit clearer --- src/_pytest/python.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 41cdf938c3b..1e6a5ffce4c 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -1194,9 +1194,9 @@ def _idval( # name of a class, function, module, etc. modified_val = getattr(val, "__name__") - # Check if the post-checks value is greater than 100 chars in length and use auto id gen if so - # isinstance checks can return empty strings which is considered valid, so we explicitly check None on modified_val - # for example @pytest.mark.parametrize('x', ('', ' ')) is acceptable + # val does not always enter the isinstance checks due to its type + # when it does we will check the string length returned and use auto generation of ids when over 100 chars + # on types which do not match isinstance checks pytest uses auto generate of ids automatically anyway if modified_val is None or len(modified_val) > 100: return str(argname) + str(idx) return modified_val