-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-44708: Only re-run test methods that match names of previously failing test methods #27287
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
Changes from 3 commits
db6e311
59d83c1
89b209e
c049888
d55e65b
aad108f
67c3e3d
26f3ba3
fb4346a
d9d7e0e
3e4622b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,10 +11,10 @@ | |
import unittest | ||
from test.libregrtest.cmdline import _parse_args | ||
from test.libregrtest.runtest import ( | ||
findtests, runtest, get_abs_module, | ||
STDTESTS, NOTTESTS, PASSED, FAILED, ENV_CHANGED, SKIPPED, RESOURCE_DENIED, | ||
INTERRUPTED, CHILD_ERROR, TEST_DID_NOT_RUN, TIMEOUT, | ||
PROGRESS_MIN_TIME, format_test_result, is_failed) | ||
findtests, runtest, get_abs_module, is_failed, | ||
STDTESTS, NOTTESTS, PROGRESS_MIN_TIME, | ||
Passed, Failed, EnvChanged, Skipped, ResourceDenied, Interrupted, | ||
ChildError, DidNotRun) | ||
from test.libregrtest.setup import setup_tests | ||
from test.libregrtest.pgo import setup_pgo_tests | ||
from test.libregrtest.utils import removepy, count, format_duration, printlist | ||
|
@@ -99,34 +99,31 @@ def get_executed(self): | |
| set(self.run_no_tests)) | ||
|
||
def accumulate_result(self, result, rerun=False): | ||
test_name = result.test_name | ||
ok = result.result | ||
test_name = result.name | ||
|
||
if ok not in (CHILD_ERROR, INTERRUPTED) and not rerun: | ||
self.test_times.append((result.test_time, test_name)) | ||
if not isinstance(result, (ChildError, Interrupted)) and not rerun: | ||
self.test_times.append((result.duration_sec, test_name)) | ||
|
||
if ok == PASSED: | ||
if isinstance(result, Passed): | ||
self.good.append(test_name) | ||
elif ok in (FAILED, CHILD_ERROR): | ||
if not rerun: | ||
self.bad.append(test_name) | ||
elif ok == ENV_CHANGED: | ||
self.environment_changed.append(test_name) | ||
elif ok == SKIPPED: | ||
elif isinstance(result, Skipped): | ||
self.skipped.append(test_name) | ||
elif ok == RESOURCE_DENIED: | ||
elif isinstance(result, ResourceDenied): | ||
self.skipped.append(test_name) | ||
self.resource_denieds.append(test_name) | ||
elif ok == TEST_DID_NOT_RUN: | ||
elif isinstance(result, EnvChanged): | ||
self.environment_changed.append(test_name) | ||
elif isinstance(result, Failed): | ||
if not rerun: | ||
self.bad.append(test_name) | ||
elif isinstance(result, DidNotRun): | ||
self.run_no_tests.append(test_name) | ||
elif ok == INTERRUPTED: | ||
elif isinstance(result, Interrupted): | ||
self.interrupted = True | ||
elif ok == TIMEOUT: | ||
self.bad.append(test_name) | ||
else: | ||
raise ValueError("invalid test result: %r" % ok) | ||
raise ValueError("invalid test result: %r" % result) | ||
|
||
if rerun and ok not in {FAILED, CHILD_ERROR, INTERRUPTED}: | ||
if rerun and not isinstance(result, (Failed, Interrupted)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For refleaks/envchanged I don't know if it makes sense to rerun because it those fail, then there is no way that the test suite is going to succeed after that, no? OTOH I don't see how it would hurt (just let's make sure that it works as we expect and it doesn't interfere with the refleak machinery). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This function is a little cryptic. The |
||
self.bad.remove(test_name) | ||
|
||
xml_data = result.xml_data | ||
|
@@ -322,7 +319,7 @@ def rerun_failed_tests(self): | |
|
||
self.accumulate_result(result, rerun=True) | ||
|
||
if result.result == INTERRUPTED: | ||
if isinstance(result, Interrupted): | ||
break | ||
|
||
if self.bad: | ||
|
@@ -423,14 +420,14 @@ def run_tests_sequential(self): | |
result = runtest(self.ns, test_name) | ||
self.accumulate_result(result) | ||
|
||
if result.result == INTERRUPTED: | ||
if isinstance(result, Interrupted): | ||
break | ||
|
||
previous_test = format_test_result(result) | ||
previous_test = str(result) | ||
test_time = time.monotonic() - start_time | ||
if test_time >= PROGRESS_MIN_TIME: | ||
previous_test = "%s in %s" % (previous_test, format_duration(test_time)) | ||
elif result.result == PASSED: | ||
elif isinstance(result, Passed): | ||
# be quiet: say nothing if the test passed shortly | ||
previous_test = None | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The different order here is required due to how inheritance works.
isinstance(Failed)
would catch all the lower cases.