Skip to content

terminal: fix progress report with duplicate nodeids #6599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 13 commits into from
1 change: 1 addition & 0 deletions changelog/6599.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix percentage progress report with duplicate nodeids.
59 changes: 34 additions & 25 deletions src/_pytest/terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
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
from typing import Set
from typing import Tuple

import attr
Expand Down Expand Up @@ -278,7 +278,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._progress_items_reported = 0
self._show_progress_info = self._determine_show_progress_info()
self._collect_report_last_write = None # type: Optional[float]

Expand Down Expand Up @@ -440,6 +440,8 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
else:
markup = None
self._add_stats(category, [rep])
if rep.when == "call" or (rep.when == "setup" and rep.failed):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this come after if not letter and not word:? Because "probably passed setup/teardown" should not be reported again.

self._progress_items_reported += 1
if not letter and not word:
# probably passed setup/teardown
return
Expand All @@ -462,7 +464,6 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:
else:
self._tw.write(letter, **markup)
else:
self._progress_nodeids_reported.add(rep.nodeid)
line = self._locationline(rep.nodeid, *rep.location)
if not running_xdist:
self.write_ensure_prefix(line, word, **markup)
Expand All @@ -483,43 +484,51 @@ def pytest_runtest_logreport(self, report: TestReport) -> None:

@property
def _is_last_item(self):
return len(self._progress_nodeids_reported) == self._session.testscollected
return self._progress_items_reported == self._session.testscollected

def pytest_runtest_logfinish(self, nodeid):
assert self._session
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_logfinish(self) -> None:
"""Write progress if past edge."""
if self.verbosity >= 0 or not self._show_progress_info:
return

self._progress_nodeids_reported.add(nodeid)
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%]")

if self._is_last_item:
self._write_progress_information_filling_space()
else:
main_color, _ = self._get_main_color()
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})
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, _ = self._get_main_color()
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 (
getattr(self, "_tests_ran", False)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should avoid attributes being set but not declared on __init__ when that is possible; this has been a great source of confusion/bugs in the past.

and self.verbosity <= 0
and self._show_progress_info
):
self._write_progress_information_filling_space()

def _get_progress_information_message(self) -> str:
assert self._session
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._progress_items_reported, collected)
return " [ {} / {} ]".format(collected, collected)
else:
if collected:
return " [{:3d}%]".format(
len(self._progress_nodeids_reported) * 100 // collected
self._progress_items_reported * 100 // collected
)
return " [100%]"

Expand Down
40 changes: 40 additions & 0 deletions testing/test_terminal.py
Original file line number Diff line number Diff line change
Expand Up @@ -1687,6 +1687,46 @@ def test_normal(self, many_tests_files, testdir):
]
)

def test_same_nodeids(self, testdir: Testdir, color_mapping) -> None:
p1 = testdir.makeconftest(
"""
def pytest_collection_modifyitems(items):
for item in items[1:]:
item._nodeid = items[0].nodeid
"""
)
p1 = testdir.makepyfile(
"""
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))
result.stdout.fnmatch_lines(
color_mapping.format_for_fnmatch(
[
"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%]",
]
)
)

def test_colored_progress(self, testdir, monkeypatch, color_mapping):
monkeypatch.setenv("PY_COLORS", "1")
testdir.makepyfile(
Expand Down