From f6060775f650beaac95717866c246013c258e54e Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Tue, 28 Jan 2020 15:29:51 +0100 Subject: [PATCH 01/11] terminal: fix progress report with duplicate nodeids Fixes https://github.com/pytest-dev/pytest/issues/6597. --- changelog/6597.bugfix.rst | 1 + src/_pytest/terminal.py | 19 +++++++------------ testing/test_terminal.py | 25 +++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 12 deletions(-) create mode 100644 changelog/6597.bugfix.rst diff --git a/changelog/6597.bugfix.rst b/changelog/6597.bugfix.rst new file mode 100644 index 00000000000..4c1208c97f8 --- /dev/null +++ b/changelog/6597.bugfix.rst @@ -0,0 +1 @@ +Fix percentage progress report with duplicate nodeids. diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 2206b5d98fa..71b3a52dda8 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -15,7 +15,6 @@ from typing import List from typing import Mapping from typing import Optional -from typing import Set from typing import Tuple import attr @@ -258,7 +257,7 @@ def __init__(self, config: Config, file=None) -> None: self.reportchars = getreportopt(config) self.hasmarkup = self._tw.hasmarkup self.isatty = file.isatty() - self._progress_nodeids_reported = set() # type: Set[str] + self._numreported = 0 self._show_progress_info = self._determine_show_progress_info() self._collect_report_last_write = None # type: Optional[float] @@ -437,7 +436,8 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: self._tw.write(letter, **markup) else: - self._progress_nodeids_reported.add(rep.nodeid) + if rep.when == "call": + self._numreported += 1 line = self._locationline(rep.nodeid, *rep.location) if not running_xdist: self.write_ensure_prefix(line, word, **markup) @@ -467,10 +467,8 @@ def pytest_runtest_logfinish(self, nodeid): main_color, _ = _get_main_color(self.stats) - self._progress_nodeids_reported.add(nodeid) - is_last_item = ( - len(self._progress_nodeids_reported) == self._session.testscollected - ) + self._numreported += 1 + is_last_item = self._numreported == self._session.testscollected if is_last_item: self._write_progress_information_filling_space(color=main_color) else: @@ -485,16 +483,13 @@ def _get_progress_information_message(self) -> str: collected = self._session.testscollected if self._show_progress_info == "count": if collected: - progress = self._progress_nodeids_reported counter_format = "{{:{}d}}".format(len(str(collected))) format_string = " [{}/{{}}]".format(counter_format) - return format_string.format(len(progress), collected) + return format_string.format(self._numreported, collected) return " [ {} / {} ]".format(collected, collected) else: if collected: - return " [{:3d}%]".format( - len(self._progress_nodeids_reported) * 100 // collected - ) + return " [{:3d}%]".format(self._numreported * 100 // collected) return " [100%]" def _write_progress_information_filling_space(self, color=None): diff --git a/testing/test_terminal.py b/testing/test_terminal.py index cc2f6d5fb69..96256ec8efa 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -1597,6 +1597,31 @@ def test_normal(self, many_tests_files, testdir): ] ) + def test_same_nodeids(self, testdir: Testdir) -> None: + p1 = testdir.makepyfile( + """ + import pytest + + @pytest.mark.parametrize("v", ["", " "]) + @pytest.mark.parametrize("w", ["", " "]) + def test(v, w): + pass + """ + ) + result = testdir.runpytest("-v", str(p1)) + trans = str.maketrans({"[": "[[]", "]": "[]]"}) + result.stdout.fnmatch_lines( + [ + line.translate(trans) + for line in [ + "test_same_nodeids.py::test[] PASSED * [ 25%]", + "test_same_nodeids.py::test[ ] PASSED * [ 50%]", + "test_same_nodeids.py::test[ ] PASSED * [ 75%]", + "test_same_nodeids.py::test[ - ] PASSED * [100%]", + ] + ] + ) + def test_colored_progress(self, testdir, monkeypatch): monkeypatch.setenv("PY_COLORS", "1") testdir.makepyfile( From aac97ef21da01b39ded992cbdad77bfa29cb5427 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sun, 9 Feb 2020 10:06:08 +0100 Subject: [PATCH 02/11] test_same_nodeids --- testing/test_terminal.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index 96256ec8efa..bd555a067cc 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -1598,26 +1598,26 @@ def test_normal(self, many_tests_files, testdir): ) def test_same_nodeids(self, testdir: Testdir) -> None: + p1 = testdir.makeconftest( + """ + def pytest_collection_modifyitems(items): + assert len(items) == 2 + items[1]._nodeid = items[0].nodeid + """ + ) p1 = testdir.makepyfile( """ - import pytest - - @pytest.mark.parametrize("v", ["", " "]) - @pytest.mark.parametrize("w", ["", " "]) - def test(v, w): - pass + def test1(): pass + def test2(): pass """ ) result = testdir.runpytest("-v", str(p1)) - trans = str.maketrans({"[": "[[]", "]": "[]]"}) result.stdout.fnmatch_lines( [ - line.translate(trans) + line.translate(TRANS_FNMATCH) for line in [ - "test_same_nodeids.py::test[] PASSED * [ 25%]", - "test_same_nodeids.py::test[ ] PASSED * [ 50%]", - "test_same_nodeids.py::test[ ] PASSED * [ 75%]", - "test_same_nodeids.py::test[ - ] PASSED * [100%]", + "test_same_nodeids.py::test1 PASSED * [50%]", + "test_same_nodeids.py::test2 PASSED * [100%]", ] ] ) From a5606b4a1a73a623f5c3713454ba61f7083a987d Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sun, 9 Feb 2020 10:12:58 +0100 Subject: [PATCH 03/11] count setups --- src/_pytest/terminal.py | 4 ++-- testing/test_terminal.py | 28 ++++++++++++++++++++++------ 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 71b3a52dda8..b018117c3a2 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -414,6 +414,8 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: markup = None self.stats.setdefault(category, []).append(rep) + if rep.when == "setup": + self._numreported += 1 if not letter and not word: # probably passed setup/teardown return @@ -436,8 +438,6 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: self._tw.write(letter, **markup) else: - if rep.when == "call": - self._numreported += 1 line = self._locationline(rep.nodeid, *rep.location) if not running_xdist: self.write_ensure_prefix(line, word, **markup) diff --git a/testing/test_terminal.py b/testing/test_terminal.py index bd555a067cc..bf97b1cf1a0 100644 --- a/testing/test_terminal.py +++ b/testing/test_terminal.py @@ -1601,14 +1601,27 @@ def test_same_nodeids(self, testdir: Testdir) -> None: p1 = testdir.makeconftest( """ def pytest_collection_modifyitems(items): - assert len(items) == 2 - items[1]._nodeid = items[0].nodeid + for item in items[1:]: + item._nodeid = items[0].nodeid """ ) p1 = testdir.makepyfile( """ - def test1(): pass - def test2(): pass + import pytest + + @pytest.fixture + def err_setup(): + assert 0, "setup-error" + + @pytest.fixture + def err_teardown(): + yield + assert 0, "teardown-error" + + def test1(err_setup): pass + def test2(err_teardown): pass + def test3(): pass + def test4(): pass """ ) result = testdir.runpytest("-v", str(p1)) @@ -1616,8 +1629,11 @@ def test2(): pass [ line.translate(TRANS_FNMATCH) for line in [ - "test_same_nodeids.py::test1 PASSED * [50%]", - "test_same_nodeids.py::test2 PASSED * [100%]", + "test_same_nodeids.py::test1 ERROR * [ 25%]", + "test_same_nodeids.py::test1 PASSED * [ 50%]", + "test_same_nodeids.py::test1 ERROR * [ 50%]", + "test_same_nodeids.py::test1 PASSED * [ 75%]", + "test_same_nodeids.py::test1 PASSED * [100%]", ] ] ) From e8e65674b9f8177aa20176be07a5fc26f2e65b77 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Sun, 9 Feb 2020 10:32:55 +0100 Subject: [PATCH 04/11] fixup! count setups --- src/_pytest/terminal.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index b018117c3a2..b842c54e48d 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -467,7 +467,6 @@ def pytest_runtest_logfinish(self, nodeid): main_color, _ = _get_main_color(self.stats) - self._numreported += 1 is_last_item = self._numreported == self._session.testscollected if is_last_item: self._write_progress_information_filling_space(color=main_color) From bbd80ef866aacc1c9842095cfbd8c61de5809103 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 10 Feb 2020 21:24:59 +0100 Subject: [PATCH 05/11] count in pytest_runtest_logfinish --- 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 b842c54e48d..bf9c2581965 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -414,8 +414,6 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: markup = None self.stats.setdefault(category, []).append(rep) - if rep.when == "setup": - self._numreported += 1 if not letter and not word: # probably passed setup/teardown return @@ -458,6 +456,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: def pytest_runtest_logfinish(self, nodeid): assert self._session + self._numreported += 1 if self.verbosity <= 0 and self._show_progress_info: if self._show_progress_info == "count": num_tests = self._session.testscollected From c3fcc9a14352ea8f8b4b568bea088303871e84ee Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 10 Feb 2020 21:25:19 +0100 Subject: [PATCH 06/11] assert --- src/_pytest/terminal.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index bf9c2581965..560954a2069 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -468,6 +468,9 @@ def pytest_runtest_logfinish(self, nodeid): is_last_item = self._numreported == self._session.testscollected if is_last_item: + if hasattr(self, "_last_item_reported"): + assert 0, self._last_item_reported + self._last_item_reported = nodeid self._write_progress_information_filling_space(color=main_color) else: w = self._width_of_current_line From be6a7cdb903546f8f439a259adff69d5a07e9c4c Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 10 Feb 2020 23:14:52 +0100 Subject: [PATCH 07/11] pytest_runtestloop, use name removed in 5939b336c What's needed to fix https://github.com/pytest-dev/pytest/issues/3088 seems to be checking for `rep.when == "setup"`. This then needs adjustments though (to code done in https://github.com/pytest-dev/pytest/pull/3110) to make this work with xdist. It appears to make sense to split it into hooks that are run per test (for eventually writing if past edge), and one that gets run at the end really (not requiring to check for "last item" then). --- src/_pytest/terminal.py | 54 +++++++++++++++++++++++------------------ 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 560954a2069..f5bcf7be8b9 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -12,6 +12,7 @@ from typing import Any from typing import Callable from typing import Dict +from typing import Generator from typing import List from typing import Mapping from typing import Optional @@ -257,7 +258,7 @@ def __init__(self, config: Config, file=None) -> None: self.reportchars = getreportopt(config) self.hasmarkup = self._tw.hasmarkup self.isatty = file.isatty() - self._numreported = 0 + self._progress_items_reported = 0 self._show_progress_info = self._determine_show_progress_info() self._collect_report_last_write = None # type: Optional[float] @@ -414,6 +415,8 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: markup = None self.stats.setdefault(category, []).append(rep) + if rep.when == "setup": + self._progress_items_reported += 1 if not letter and not word: # probably passed setup/teardown return @@ -454,30 +457,31 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: self._tw.write(" " + line) self.currentfspath = -2 - def pytest_runtest_logfinish(self, nodeid): - assert self._session - self._numreported += 1 - if self.verbosity <= 0 and self._show_progress_info: - if self._show_progress_info == "count": - num_tests = self._session.testscollected - progress_length = len(" [{}/{}]".format(str(num_tests), str(num_tests))) - else: - progress_length = len(" [100%]") + def pytest_runtest_teardown(self) -> None: + """Write progress if past edge.""" + if self.verbosity >= 0 or not self._show_progress_info: + return + + if self._show_progress_info == "count": + assert self._session + num_tests = self._session.testscollected + progress_length = len(" [{0}/{0}]".format(num_tests)) + else: + progress_length = len(" [100%]") + w = self._width_of_current_line + past_edge = w + progress_length + 1 >= self._screen_width + if past_edge: + msg = self._get_progress_information_message() main_color, _ = _get_main_color(self.stats) + self._tw.write(msg + "\n", **{main_color: True}) - is_last_item = self._numreported == self._session.testscollected - if is_last_item: - if hasattr(self, "_last_item_reported"): - assert 0, self._last_item_reported - self._last_item_reported = nodeid - self._write_progress_information_filling_space(color=main_color) - else: - w = self._width_of_current_line - past_edge = w + progress_length + 1 >= self._screen_width - if past_edge: - msg = self._get_progress_information_message() - self._tw.write(msg + "\n", **{main_color: True}) + @pytest.hookimpl(hookwrapper=True) + def pytest_runtestloop(self) -> Generator[None, None, None]: + """Write final progress indicator.""" + yield + if self.verbosity <= 0 and self._show_progress_info: + self._write_progress_information_filling_space() def _get_progress_information_message(self) -> str: assert self._session @@ -486,11 +490,13 @@ def _get_progress_information_message(self) -> str: if collected: counter_format = "{{:{}d}}".format(len(str(collected))) format_string = " [{}/{{}}]".format(counter_format) - return format_string.format(self._numreported, collected) + return format_string.format(self._progress_items_reported, collected) return " [ {} / {} ]".format(collected, collected) else: if collected: - return " [{:3d}%]".format(self._numreported * 100 // collected) + return " [{:3d}%]".format( + self._progress_items_reported * 100 // collected + ) return " [100%]" def _write_progress_information_filling_space(self, color=None): From cad0319c696150d3f6c482438ded39056088855f Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Mon, 10 Feb 2020 23:27:49 +0100 Subject: [PATCH 08/11] make use of never used _tests_ran --- src/_pytest/terminal.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index f5bcf7be8b9..256cdb6aa88 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -480,7 +480,11 @@ def pytest_runtest_teardown(self) -> None: def pytest_runtestloop(self) -> Generator[None, None, None]: """Write final progress indicator.""" yield - if self.verbosity <= 0 and self._show_progress_info: + if ( + getattr(self, "_tests_ran", False) + and self.verbosity <= 0 + and self._show_progress_info + ): self._write_progress_information_filling_space() def _get_progress_information_message(self) -> str: From dbd3f46aa893bb4e0ef41c53c9ff11270a7dfcc6 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 19 Feb 2020 21:40:50 +0100 Subject: [PATCH 09/11] or (rep.when == "setup" and rep.failed) --- src/_pytest/terminal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 14530842fdb..91648d351a7 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -440,7 +440,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: else: markup = None self._add_stats(category, [rep]) - if rep.when == "setup": + if rep.when == "call" or (rep.when == "setup" and rep.failed): self._progress_items_reported += 1 if not letter and not word: # probably passed setup/teardown From f660904622983d938c34e00afbd70e2d53bbf93f Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Wed, 19 Feb 2020 21:44:38 +0100 Subject: [PATCH 10/11] less diff churn --- src/_pytest/terminal.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 91648d351a7..85c38dd638c 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -482,6 +482,10 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: self._tw.write(" " + line) self.currentfspath = -2 + @property + def _is_last_item(self): + return self._progress_items_reported == self._session.testscollected + def pytest_runtest_teardown(self) -> None: """Write progress if past edge.""" if self.verbosity >= 0 or not self._show_progress_info: @@ -1040,10 +1044,6 @@ def show_skipped(lines: List[str]) -> None: for line in lines: self.write_line(line) - @property - def _is_last_item(self): - return self._progress_items_reported == self._session.testscollected - def _get_main_color(self) -> Tuple[str, List[str]]: if self._main_color is None or self._known_types is None or self._is_last_item: self._set_main_color() From b6c7c51250967bbe2a9cedebd9b6a69261687ee7 Mon Sep 17 00:00:00 2001 From: Daniel Hahler Date: Thu, 5 Mar 2020 06:32:46 +0100 Subject: [PATCH 11/11] s/pytest_runtest_teardown/pytest_runtest_logfinish --- src/_pytest/terminal.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/terminal.py b/src/_pytest/terminal.py index 85c38dd638c..cd97fa3a637 100644 --- a/src/_pytest/terminal.py +++ b/src/_pytest/terminal.py @@ -486,7 +486,7 @@ def pytest_runtest_logreport(self, report: TestReport) -> None: def _is_last_item(self): return self._progress_items_reported == self._session.testscollected - def pytest_runtest_teardown(self) -> None: + def pytest_runtest_logfinish(self) -> None: """Write progress if past edge.""" if self.verbosity >= 0 or not self._show_progress_info: return