Skip to content

Fix -vv overriding --durations-min (#12938) #13100

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

Merged
merged 8 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ Joseph Hunkeler
Joseph Sawaya
Josh Karpel
Joshua Bronson
Julian Valentin
Jurko Gospodnetić
Justice Ndou
Justyna Janczyszyn
Expand Down
1 change: 1 addition & 0 deletions changelog/12938.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed ``--durations-min`` argument not respected if ``-vv`` is used.
14 changes: 11 additions & 3 deletions src/_pytest/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
"--durations-min",
action="store",
type=float,
default=0.005,
default=None,
metavar="N",
help="Minimal duration in seconds for inclusion in slowest list. "
"Default: 0.005.",
Expand All @@ -74,6 +74,8 @@
verbose = terminalreporter.config.get_verbosity()
if durations is None:
return
if durations_min is None:
durations_min = 0.005 if verbose < 2 else 0.0

Check warning on line 78 in src/_pytest/runner.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/runner.py#L78

Added line #L78 was not covered by tests
tr = terminalreporter
dlist = []
for replist in tr.stats.values():
Expand All @@ -90,10 +92,16 @@
dlist = dlist[:durations]

for i, rep in enumerate(dlist):
if verbose < 2 and rep.duration < durations_min:
if rep.duration < durations_min:
tr.write_line("")
tr.write_line(
f"({len(dlist) - i} durations < {durations_min:g}s hidden. Use -vv to show these durations.)"
f"({len(dlist) - i} durations < {durations_min:g}s hidden."
+ (
" Use -vv to show these durations."
if terminalreporter.config.option.durations_min is None
else ""
)
+ ")"
)
break
tr.write_line(f"{rep.duration:02.2f}s {rep.when:<8} {rep.nodeid}")
Expand Down
44 changes: 28 additions & 16 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# mypy: allow-untyped-defs
from __future__ import annotations

from collections.abc import Sequence
import dataclasses
import importlib.metadata
import os
Expand Down Expand Up @@ -970,28 +971,39 @@ def test_calls_showall(self, pytester: Pytester, mock_timing) -> None:
pytester.makepyfile(self.source)
result = pytester.runpytest_inprocess("--durations=0")
assert result.ret == 0

tested = "3"
for x in tested:
Comment on lines -974 to -975
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only checks whether test 3 is in the output, but not if the other tests are not. Fixed this.

for y in ("call",): # 'setup', 'call', 'teardown':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what the rationale for this comment is. It was added with --durations in 3b9fd3a, 13 years ago. I removed it.

for line in result.stdout.lines:
if (f"test_{x}") in line and y in line:
break
else:
raise AssertionError(f"not found {x} {y}")
print(result.stdout)
TestDurations.check_tests_in_output(result.stdout.lines, "23")

def test_calls_showall_verbose(self, pytester: Pytester, mock_timing) -> None:
pytester.makepyfile(self.source)
result = pytester.runpytest_inprocess("--durations=0", "-vv")
assert result.ret == 0
TestDurations.check_tests_in_output(result.stdout.lines, "123")

for x in "123":
for y in ("call",): # 'setup', 'call', 'teardown':
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking for "call" is not sufficient, as the output in question contains test_calls_showall_verbose.py::test_X PASSED (e.g.) for all test numbers X, which contains call as well, even though the call might be missing in the durations list. Added a space before and after call to fix this.

for line in result.stdout.lines:
if (f"test_{x}") in line and y in line:
break
else:
raise AssertionError(f"not found {x} {y}")
def test_calls_showall_durationsmin(self, pytester: Pytester, mock_timing) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A test for --durations-min was missing. Added one.

pytester.makepyfile(self.source)
result = pytester.runpytest_inprocess("--durations=0", "--durations-min=0.015")
assert result.ret == 0
TestDurations.check_tests_in_output(result.stdout.lines, "3")

def test_calls_showall_durationsmin_verbose(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the actual test that fails without the change in src/_pytest/runner.py.

self, pytester: Pytester, mock_timing
) -> None:
pytester.makepyfile(self.source)
result = pytester.runpytest_inprocess(
"--durations=0", "--durations-min=0.015", "-vv"
)
assert result.ret == 0
TestDurations.check_tests_in_output(result.stdout.lines, "3")

@staticmethod
def check_tests_in_output(lines: Sequence[str], expected_test_numbers: str) -> None:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reduced code duplication by refactoring common code into a separate function.

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to take expected_test_numbers: Collection[int], or better yet *expected_test_numbers: int, then check for more than just 1/2/3 in the condition below, and finally assert a comparison between eg lists.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wanted to have somewhat minimal diff, that's why I used strings as well. Changed it to *expected_test_numbers: int.

check for more than just 1/2/3 in the condition below

I don't see how we could do this. As mentioned in #13100 (comment), the tests now also checks whether the other tests are not in the output. For this, we need to know how many tests there are.

Copy link
Member

Choose a reason for hiding this comment

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

Right, so I'd make number_of_tests: int an argument to the helper function rather than a constant defined in the body. That way it works for cases where there are other numbers of tests too 🙂

found_test_numbers = "".join(
test_number
for test_number in "123"
if any(f"test_{test_number}" in line and " call " in line for line in lines)
)
assert found_test_numbers == expected_test_numbers

def test_with_deselected(self, pytester: Pytester, mock_timing) -> None:
pytester.makepyfile(self.source)
Expand Down
Loading