Skip to content

Commit 0000154

Browse files
treyshafferclaude
andcommitted
Refactor: Remove unused global state and improve hook priority
Based on comprehensive triple-agent code review findings: 1. Remove unused `_nodeids_with_warnings` global set: - Set was populated but never consulted by production code - Only used by one test, which has been updated - Eliminates potential memory leak and thread safety concerns 2. Update test_warning_tracking_for_yellow_coloring: - Now verifies `report.has_warnings` attribute directly - More direct test of the actual implementation - No longer depends on module-level state 3. Add `tryfirst=True` to pytest_report_teststatus hook: - Ensures warnings plugin runs before other plugins - Prevents potential plugin ordering issues - Guarantees yellow markup is applied correctly These changes address all critical issues identified in the meta-analysis while maintaining 100% test coverage and functionality. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 5b4100a commit 0000154

File tree

2 files changed

+29
-25
lines changed

2 files changed

+29
-25
lines changed

src/_pytest/warnings.py

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,6 @@
2424
# StashKey for storing warning log on items
2525
warning_captured_log_key = StashKey[list[warnings.WarningMessage]]()
2626

27-
# Track which nodeids have warnings (for pytest_report_teststatus)
28-
_nodeids_with_warnings: set[str] = set()
29-
3027

3128
@contextmanager
3229
def catch_warnings_for_item(
@@ -99,7 +96,6 @@ def pytest_runtest_makereport(
9996
if report.when == "call":
10097
warning_log = item.stash.get(warning_captured_log_key, None)
10198
if warning_log:
102-
_nodeids_with_warnings.add(item.nodeid)
10399
# Set attribute on report for xdist compatibility
104100
report.has_warnings = True # type: ignore[attr-defined]
105101

@@ -114,7 +110,7 @@ def pytest_runtest_makereport(
114110
)
115111

116112

117-
@pytest.hookimpl()
113+
@pytest.hookimpl(tryfirst=True)
118114
def pytest_report_teststatus(report: TestReport, config: Config):
119115
"""Provide yellow markup for passed tests that have warnings."""
120116
if report.passed and report.when == "call":

testing/test_warnings.py

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -891,7 +891,7 @@ def test_resource_warning(tmp_path):
891891

892892

893893
def test_warning_tracking_for_yellow_coloring(pytester: Pytester) -> None:
894-
"""Test that warnings are tracked and pytest_report_teststatus provides yellow markup."""
894+
"""Test that warnings set has_warnings attribute on reports for yellow markup."""
895895
pytester.makepyfile(
896896
"""
897897
import warnings
@@ -904,31 +904,39 @@ def test_without_warning():
904904
"""
905905
)
906906

907-
# Use inline_run to verify the tracking mechanism
907+
# Use inline_run to verify the has_warnings attribute is set correctly
908908
reprec = pytester.inline_run()
909909

910-
# Check that the nodeid with warnings was tracked
911-
from _pytest.warnings import _nodeids_with_warnings
910+
# Get all call phase reports
911+
reports = reprec.getreports("pytest_runtest_logreport")
912+
call_reports = [r for r in reports if r.when == "call"]
912913

913-
# Find which test had warnings
914-
test_with_warning_nodeid = None
915-
test_without_warning_nodeid = None
916-
for nodeid in _nodeids_with_warnings:
917-
if "test_with_warning" in nodeid:
918-
test_with_warning_nodeid = nodeid
914+
# Find the reports for each test
915+
test_with_warning_report = None
916+
test_without_warning_report = None
917+
for report in call_reports:
918+
if "test_with_warning" in report.nodeid:
919+
test_with_warning_report = report
920+
elif "test_without_warning" in report.nodeid:
921+
test_without_warning_report = report
919922

920-
# Get all nodeids from reports
921-
reports = reprec.getreports("pytest_runtest_logreport")
922-
for report in reports:
923-
if report.when == "call":
924-
if "test_without_warning" in report.nodeid:
925-
test_without_warning_nodeid = report.nodeid
923+
# Verify test_with_warning has the has_warnings attribute set
924+
assert test_with_warning_report is not None, (
925+
"Expected to find test_with_warning report"
926+
)
927+
assert hasattr(test_with_warning_report, "has_warnings"), (
928+
"Expected test_with_warning report to have has_warnings attribute"
929+
)
930+
assert test_with_warning_report.has_warnings is True, (
931+
"Expected has_warnings to be True for test with warnings"
932+
)
926933

927-
assert test_with_warning_nodeid is not None, (
928-
"Expected test_with_warning to be tracked"
934+
# Verify test_without_warning does NOT have the has_warnings attribute
935+
assert test_without_warning_report is not None, (
936+
"Expected to find test_without_warning report"
929937
)
930-
assert test_without_warning_nodeid not in _nodeids_with_warnings, (
931-
"Did not expect test_without_warning to be tracked"
938+
assert not hasattr(test_without_warning_report, "has_warnings"), (
939+
"Did not expect test_without_warning report to have has_warnings attribute"
932940
)
933941

934942

0 commit comments

Comments
 (0)