[2.7] bpo-30258: regrtest handles child process crash by vstinner · Pull Request #1431 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation13 Commits1 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Backport the CHILD_ERROR status from master: a test is considered as
failed if a worker process running a test exited with a code
different than zero.
Change also the output: write stdout and stderr of the child process
after the test name, instead of writing it before.
Backport the CHILD_ERROR status from master: a test is considered as failed if a worker process running a test exited with a code different than zero.
Change also the output: write stdout and stderr of the child process after the test name, instead of writing it before.
accumulate_result(): don't use time of CHILD_ERROR or INTERRUPTED results.
| @@ -478,6 +480,9 @@ def accumulate_result(test, result): |
|---|
| elif ok == RESOURCE_DENIED: |
| skipped.append(test) |
| resource_denieds.append(test) |
| else: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.x doesn't have this branch.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on purpose: it should catch bugs in regrtest itself :-D
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be added in 3.x?
| # Strip last refcount output line if it exists, since it |
|---|
| # comes from the shutdown of the interpreter in the subcommand. |
| stderr = debug_output_pat.sub("", stderr) |
| if retcode != 0: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 3.x this is after stdout, _, result = stdout.strip().rpartition("\n").
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh... it's a bug in master. If the process crashed, we must not modify stdout but dump it unchanged!
| result = (CHILD_ERROR, "Exit code %s" % retcode) |
|---|
| output.put((test, stdout.rstrip(), stderr.rstrip(), |
| result)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missed return?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, it means that the worker thread will end. That's a bug :-/
| @@ -545,9 +558,11 @@ def work(): |
|---|
| except BaseException: |
| output.put((None, None, None, None)) |
| raise |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are no empty lines in 3.5.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added empty lines for readability, is that an issue?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only increasing difference between 2.7 and 3.5.
| print(fmt.format( |
|---|
| test_count_width, test_index, test_count, |
| len(bad), test)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 3.5:
if result[0] == CHILD_ERROR:
raise Exception("Child error on {}: {}".format(test, result[1]))
In master this is handled in different way.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to write the smallest patch to handle child error. regrtest in master is very different.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is why I compared the code with 3.5.
I don't suggest backporting the code from master (with showing the progress of parallel long running test etc), but shouldn't these lines be ported from 3.5?
Labels
Tests in the Lib/test dir