Skip to content

Commit 6e595d6

Browse files
serhiy-storchakamiss-islington
authored andcommitted
gh-132581: Report why the execution environment was altered (GH-155294)
The list of tests which altered the execution environment now includes the reasons -- an unraisable exception, a modified sys.path, leaked temporary files, etc -- one per line. The final result no longer repeats the same state twice, like "ENV CHANGED then ENV CHANGED". (cherry picked from commit 350fc64) Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent ac4e5d2 commit 6e595d6

10 files changed

Lines changed: 67 additions & 17 deletions

File tree

Lib/test/libregrtest/main.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ def run_tests_sequentially(self, runtests: RunTests) -> None:
443443

444444
def get_state(self) -> str:
445445
state = self.results.get_state(self.fail_env_changed)
446-
if self.first_state:
446+
if self.first_state and self.first_state != state:
447447
state = f'{self.first_state} then {state}'
448448
return state
449449

Lib/test/libregrtest/result.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,9 @@ class TestResult:
100100
# partial coverage in a worker run; not used by sequential in-process runs
101101
covered_lines: list[Location] | None = None
102102

103+
# short descriptions of how the test altered the execution environment
104+
env_changed_reasons: list[str] | None = None
105+
103106
def is_failed(self, fail_env_changed: bool) -> bool:
104107
if self.state == State.ENV_CHANGED:
105108
return fail_env_changed
@@ -175,9 +178,15 @@ def __str__(self) -> str:
175178
def has_meaningful_duration(self):
176179
return State.has_meaningful_duration(self.state)
177180

178-
def set_env_changed(self):
181+
def set_env_changed(self, *reasons):
179182
if self.state is None or self.state == State.PASSED:
180183
self.state = State.ENV_CHANGED
184+
if reasons:
185+
if self.env_changed_reasons is None:
186+
self.env_changed_reasons = []
187+
for reason in reasons:
188+
if reason not in self.env_changed_reasons:
189+
self.env_changed_reasons.append(reason)
181190

182191
def must_stop(self, fail_fast: bool, fail_env_changed: bool) -> bool:
183192
if State.must_stop(self.state):

Lib/test/libregrtest/results.py

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ def __init__(self) -> None:
3030
self.skipped: TestList = []
3131
self.resource_denied: TestList = []
3232
self.env_changed: TestList = []
33+
# test name => how the test altered the execution environment
34+
self.env_changed_reasons: dict[TestName, list[str]] = {}
3335
self.run_no_tests: TestList = []
3436
self.rerun: TestList = []
3537
self.rerun_results: list[TestResult] = []
@@ -107,6 +109,9 @@ def accumulate_result(self, result: TestResult, runtests: RunTests) -> None:
107109
self.good.append(test_name)
108110
case State.ENV_CHANGED:
109111
self.env_changed.append(test_name)
112+
if result.env_changed_reasons:
113+
self.env_changed_reasons[test_name] = \
114+
result.env_changed_reasons
110115
self.rerun_results.append(result)
111116
case State.SKIPPED:
112117
self.skipped.append(test_name)
@@ -254,7 +259,18 @@ def display_result(self, tests: TestTuple, quiet: bool, print_slowest: bool) ->
254259
print()
255260
count_text = count(len(tests_list), count_text)
256261
print(title_format.format(count_text))
257-
printlist(tests_list)
262+
if tests_list is self.env_changed:
263+
# List every test and every reason on a separate line.
264+
for test_name in sorted(tests_list):
265+
reasons = self.env_changed_reasons.get(test_name)
266+
if reasons:
267+
print(f" {test_name}:")
268+
for reason in reasons:
269+
print(f" {reason}")
270+
else:
271+
print(f" {test_name}")
272+
else:
273+
printlist(tests_list)
258274

259275
if self.good and not quiet:
260276
print()

Lib/test/libregrtest/run_workers.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,8 @@ def _runtest(self, test_name: TestName) -> MultiprocessResult:
386386
f'Warning -- {test_name} leaked temporary files '
387387
f'({len(tmp_files)}): {", ".join(sorted(tmp_files))}')
388388
stdout += msg
389-
result.set_env_changed()
389+
result.set_env_changed(
390+
f"leaked temporary files: {', '.join(sorted(tmp_files))}")
390391

391392
return MultiprocessResult(result, stdout)
392393

Lib/test/libregrtest/save_env.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -347,7 +347,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
347347
current = get()
348348
# Check for changes to the resource's value
349349
if current != original:
350-
support.environment_altered = True
350+
support.set_environment_altered(f"{name} was modified")
351351
restore(original)
352352
if not self.quiet and not self.pgo:
353353
print_warning(

Lib/test/libregrtest/single.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,8 @@ def test_func():
173173
remove_testfn(test_name, runtests.verbose)
174174

175175
if gc.garbage:
176-
support.environment_altered = True
176+
support.set_environment_altered(
177+
f"{len(gc.garbage)} uncollectable object(s)")
177178
print_warning(f"{test_name} created {len(gc.garbage)} "
178179
f"uncollectable object(s)")
179180

@@ -194,6 +195,7 @@ def _runtest_env_changed_exc(result: TestResult, runtests: RunTests,
194195
# Reset the environment_altered flag to detect if a test altered
195196
# the environment
196197
support.environment_altered = False
198+
support.environment_altered_reasons.clear()
197199

198200
pgo = runtests.pgo
199201
if pgo:
@@ -261,7 +263,7 @@ def _runtest_env_changed_exc(result: TestResult, runtests: RunTests,
261263
return
262264

263265
if support.environment_altered:
264-
result.set_env_changed()
266+
result.set_env_changed(*support.environment_altered_reasons)
265267
# Don't override the state if it was already set (REFLEAK or ENV_CHANGED)
266268
if result.state is None:
267269
result.state = State.PASSED

Lib/test/libregrtest/utils.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ def print_warning(msg: str) -> None:
131131

132132
def regrtest_unraisable_hook(unraisable) -> None:
133133
global orig_unraisablehook
134-
support.environment_altered = True
134+
support.set_environment_altered(
135+
f"unraisable exception ({unraisable.exc_type.__name__})")
135136
support.print_warning("Unraisable exception")
136137
old_stderr = sys.stderr
137138
try:
@@ -155,7 +156,8 @@ def setup_unraisable_hook() -> None:
155156

156157
def regrtest_threading_excepthook(args) -> None:
157158
global orig_threading_excepthook
158-
support.environment_altered = True
159+
support.set_environment_altered(
160+
f"uncaught thread exception ({args.exc_type.__name__})")
159161
support.print_warning(f"Uncaught thread exception: {args.exc_type.__name__}")
160162
old_stderr = sys.stderr
161163
try:
@@ -515,7 +517,7 @@ def remove_testfn(test_name: TestName, verbose: int) -> None:
515517

516518
if verbose:
517519
print_warning(f"{test_name} left behind {kind} {name!r}")
518-
support.environment_altered = True
520+
support.set_environment_altered(f"left behind {kind} {name!r}")
519521

520522
try:
521523
import stat

Lib/test/support/__init__.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,14 +1537,25 @@ def print_warning(msg):
15371537
# to cleanup threads.
15381538
environment_altered = False
15391539

1540+
# Short descriptions of what was altered, e.g. "unraisable exception".
1541+
# They are reported by regrtest together with the name of the test.
1542+
environment_altered_reasons = []
1543+
1544+
1545+
def set_environment_altered(reason):
1546+
"""Set the environment_altered flag and record why it was set."""
1547+
global environment_altered
1548+
environment_altered = True
1549+
if reason not in environment_altered_reasons:
1550+
environment_altered_reasons.append(reason)
1551+
1552+
15401553
def reap_children():
15411554
"""Use this function at the end of test_main() whenever sub-processes
15421555
are started. This will help ensure that no extra children (zombies)
15431556
stick around to hog resources and create problems when looking
15441557
for refleaks.
15451558
"""
1546-
global environment_altered
1547-
15481559
# Need os.waitpid(-1, os.WNOHANG): Windows is not supported
15491560
if not (hasattr(os, 'waitpid') and hasattr(os, 'WNOHANG')):
15501561
return
@@ -1564,7 +1575,7 @@ def reap_children():
15641575
break
15651576

15661577
print_warning(f"reap_children() reaped child process {pid}")
1567-
environment_altered = True
1578+
set_environment_altered("reaped child process")
15681579

15691580

15701581
@contextlib.contextmanager

Lib/test/test_regrtest.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -711,9 +711,13 @@ def list_regex(line_format, tests):
711711
self.check_line(output, regex)
712712

713713
if env_changed:
714-
regex = list_regex(r'%s test%s altered the execution environment '
715-
r'\(env changed\)',
716-
env_changed)
714+
# Every test is listed on a separate line, followed by the
715+
# reasons why the environment was altered, one per line.
716+
count = len(env_changed)
717+
regex = (r'%s test%s altered the execution environment '
718+
r'\(env changed\):\n' % (count, plural(count)))
719+
regex += ''.join(r' %s:?\n(?: .*\n)*' % re.escape(name)
720+
for name in sorted(env_changed))
717721
self.check_line(output, regex)
718722

719723
if omitted:
@@ -802,7 +806,8 @@ def list_regex(line_format, tests):
802806
state = ', '.join(state)
803807
if rerun is not None:
804808
new_state = 'SUCCESS' if rerun.success else 'FAILURE'
805-
state = f'{state} then {new_state}'
809+
if new_state != state:
810+
state = f'{state} then {new_state}'
806811
self.check_line(output, f'Result: {state}', full=True)
807812

808813
def parse_random_seed(self, output: str) -> str:
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
The list of tests which altered the execution environment now includes the
2+
reasons why the environment was considered altered, for example an unraisable
3+
exception or a modified :data:`sys.path`. The final result no longer repeats
4+
the same state twice (like ``ENV CHANGED then ENV CHANGED``).

0 commit comments

Comments
 (0)