GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) by tiran · Pull Request #94253 · python/cpython

Conversation

@tiran

@tiran tiran commented Jun 25, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Co-authored-by: Victor Stinner [email protected]

  • Issue: Emscripten test worker sometimes fails witth "BlockingIOError: write could not complete without blocking" #94026

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@tiran tiran added the needs backport to 3.11 only security fixes label Jun 25, 2022
@bedevere-bot bedevere-bot added the awaiting core review label Jun 25, 2022
@tiran tiran mentioned this pull request Jun 25, 2022
gh-94026: use a temporary file for regrtest worker process stdout #94222
Closed
@tiran tiran added the skip news label Jun 25, 2022
@tiran tiran mentioned this pull request Jun 26, 2022
gh-94026: Workaround for BlockingIOError in runtest_mp on Emscripten #94164
Closed
@tiran
tiran marked this pull request as ready for review June 26, 2022 08:05
@tiran
tiran requested a review from vstinner June 26, 2022 12:31
@tiran

tiran commented Jun 27, 2022

Copy link
Copy Markdown
Member Author

@vstinner are you ok with my patch?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tiran
tiran force-pushed the gh-94026-tmpfile branch from 128af5d to df2f3fb Compare June 28, 2022 07:35
@tiran tiran added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 28, 2022
@bedevere-bot

bedevere-bot commented Jun 28, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @tiran for commit df2f3fbb63b946f5e709262c37cfad76d7372158 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jun 28, 2022
@tiran

tiran commented Jun 28, 2022

Copy link
Copy Markdown
Member Author

@vstinner please take a look. I would like to merge the PR today.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

vstinner
vstinner reviewed Jun 28, 2022
View reviewed changes
Comment thread Lib/test/libregrtest/runtest_mp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread Lib/test/libregrtest/runtest_mp.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Jun 28, 2022

Copy link
Copy Markdown
Member

The overall change LGTM. I added a few comments.

IMO it's ok to use a temporary file to worker process stdout and stderr. If we can any issue later, we can reconsider this choice later. But so far, buildbots are happy, so I'm happy as well :-)

tiran reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tiran and others added 3 commits June 29, 2022 09:40
@tiran @vstinner
pythongh-94026: Buffer regrtest worker stdout in temporary file
198027a
Co-authored-by: Victor Stinner <[email protected]>
@tiran
Address review
141d491
@tiran
get retcode directly
bd76060
@tiran
tiran force-pushed the gh-94026-tmpfile branch from ffdeae1 to bd76060 Compare June 29, 2022 07:41
@tiran tiran changed the title gh-94026: Buffer regrtest worker stdout in temporary file gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) Jun 29, 2022
@tiran
tiran merged commit 199ba23 into python:main Jun 29, 2022
@tiran
tiran deleted the gh-94026-tmpfile branch June 29, 2022 08:05
@miss-islington

miss-islington commented Jun 29, 2022

Copy link
Copy Markdown
Contributor

Thanks @tiran for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting core review label Jun 29, 2022
@miss-islington

miss-islington commented Jun 29, 2022

Copy link
Copy Markdown
Contributor

Sorry, @tiran, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 199ba233248ab279f445e0809c2077976f0711bc 3.11

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned tiran Jun 29, 2022
tiran added a commit to tiran/cpython that referenced this pull request Jun 29, 2022
@tiran
[3.11] pythongh-94026: Buffer regrtest worker stdout in temporary file (
5dea1bb
pythonGH-94253)

Co-authored-by: Victor Stinner <[email protected]>.
(cherry picked from commit 199ba23)

Co-authored-by: Christian Heimes <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jun 29, 2022
@bedevere-bot

bedevere-bot commented Jun 29, 2022

Copy link
Copy Markdown

GH-94408 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tiran added a commit that referenced this pull request Jun 29, 2022
@tiran
[3.11] gh-94026: Buffer regrtest worker stdout in temporary file (GH-…
0122ab2
…94253) (GH-94408)

Co-authored-by: Victor Stinner <[email protected]>.
Co-authored-by: Christian Heimes <[email protected]>
gvanrossum pushed a commit to gvanrossum/cpython that referenced this pull request Jun 30, 2022
@tiran @vstinner @gvanrossum
pythongh-94026: Buffer regrtest worker stdout in temporary file (pyth…
2e919ba
…onGH-94253)

Co-authored-by: Victor Stinner <[email protected]>
@tim-one

tim-one commented Aug 7, 2022

Copy link
Copy Markdown
Member

Since this was checked in, parallel regrtest runs (-j) on Windows always fail for me. They just die midstream, skipping loads of tests, but not in any reproducible place.

After a long time, I eventually figured out that this happens if, and only if, test_disutils is being run. Add -x test_distutils, and no problem. OTOH, running test_distutils by itself fails even with -j1:

C:\Code\Python\PCbuild>rt -q -j1 test_distutils

C:\Code\Python\PCbuild>"C:\Code\Python\PCbuild\amd64\python.exe"  -u -Wd -E -bb -m test  -j1 test_distutils
0:00:00 Run tests in parallel using 1 child processes
0:01:00 load avg: 0.00 running: test_distutils (60.0 sec)
Warning -- regrtest worker thread failed: Traceback (most recent call last):
Warning --   File "C:\Code\Python\Lib\test\libregrtest\runtest_mp.py", line 334, in run
Warning --     mp_result = self._runtest(test_name)
Warning --                 ^^^^^^^^^^^^^^^^^^^^^^^^
Warning --   File "C:\Code\Python\Lib\test\libregrtest\runtest_mp.py", line 293, in _runtest
Warning --     stdout = stdout_fh.read().strip()
Warning --              ^^^^^^^^^^^^^^^^
Warning --   File "C:\Code\Python\Lib\tempfile.py", line 483, in func_wrapper
Warning --     return func(*args, **kwargs)
Warning --            ^^^^^^^^^^^^^^^^^^^^^
Warning --   File "<frozen codecs>", line 322, in decode
Warning -- UnicodeDecodeError: 'utf-8' codec can't decode byte 0x91 in position 3149: invalid start byte

== Tests result: NO TEST RUN ==

1 test omitted:
    test_distutils

Total duration: 1 min 17 sec
Tests result: NO TEST RUN

1 test omitted:
    test_distutils

Total duration: 55.8 sec
Tests result: NO TEST RUN

All the failures have "something like that" in them, although in context an irrelevant test usually appears to be at fault, because this unhandled exception causes all sorts of other crud to get skipped, and then Windows usually dies when teardown code is trying to delete a file that's still in use by some other test.

I don't understand what the code is trying to do, so just note that all visible problems go away if, when stdout_fh is created, the encoding is set to "latin1".

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner

vstinner commented Sep 8, 2022

Copy link
Copy Markdown
Member

Warning -- UnicodeDecodeError: 'utf-8' codec can't decode byte 0x91 in position 3149: invalid start byte

Issue now tracked by #95027

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@vstinner vstinner mentioned this pull request Sep 8, 2022
Running tests in parallel on Windows quits too soon #95027
Closed
vstinner added a commit to vstinner/cpython that referenced this pull request Sep 2, 2023
@vstinner
Revert "[3.11] pythongh-94026: Buffer regrtest worker stdout in tempo…
57c050e
…rary file (pythonGH-94253) (pythonGH-94408)"

This reverts commit 0122ab2.
vstinner added a commit to vstinner/cpython that referenced this pull request Sep 2, 2023
@tiran @vstinner
pythongh-94026: Buffer regrtest worker stdout in temporary file (pyth…
24d94ca
…onGH-94253)

Co-authored-by: Victor Stinner <[email protected]>
(cherry picked from commit 199ba23)
@vstinner vstinner mentioned this pull request Sep 2, 2023
[3.11] gh-108822: Backport libregrtest changes from the main branch #108820
Merged
vstinner added a commit that referenced this pull request Sep 3, 2023
@vstinner @tiran
@zware @sobolevn @zitterbewegung @gpshead
[3.11] gh-108822: Backport libregrtest changes from the main branch (#…
79f7a4c
…108820)

* Revert "[3.11] gh-101634: regrtest reports decoding error as failed test (#106169) (#106175)"

This reverts commit d5418e9.

* Revert "[3.11] bpo-46523: fix tests rerun when `setUp[Class|Module]` fails (GH-30895) (GH-103342)"

This reverts commit ecb09a8.

* Revert "gh-95027: Fix regrtest stdout encoding on Windows (GH-98492)"

This reverts commit b2aa28e.

* Revert "[3.11] gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253) (GH-94408)"

This reverts commit 0122ab2.

* Revert "Run Tools/scripts/reindent.py (GH-94225)"

This reverts commit f0f3a42.

* Revert "gh-94052: Don't re-run failed tests with --python option (GH-94054)"

This reverts commit 1347607.

* Revert "[3.11] gh-84461: Fix Emscripten umask and permission issues (GH-94002) (GH-94006)"

This reverts commit 1073184.

* gh-93353: regrtest checks for leaked temporary files (#93776)

When running tests with -jN, create a temporary directory per process
and mark a test as "environment changed" if a test leaks a temporary
file or directory.

(cherry picked from commit e566ce5)

* gh-93353: Fix regrtest for -jN with N >= 2 (GH-93813)

(cherry picked from commit 36934a1)

* gh-93353: regrtest supports checking tmp files with -j2 (#93909)

regrtest now also implements checking for leaked temporary files and
directories when using -jN for N >= 2. Use tempfile.mkdtemp() to
create the temporary directory. Skip this check on WASI.

(cherry picked from commit 4f85cec)

* gh-84461: Fix Emscripten umask and permission issues (GH-94002)

- Emscripten's default umask is too strict, see
  emscripten-core/emscripten#17269
- getuid/getgid and geteuid/getegid are stubs that always return 0
  (root). Disable effective uid/gid syscalls and fix tests that use
  chmod() current user.
- Cannot drop X bit from directory.

(cherry picked from commit 2702e40)

* gh-94052: Don't re-run failed tests with --python option (#94054)

(cherry picked from commit 0ff7b99)

* Run Tools/scripts/reindent.py (#94225)

Reindent files which were not properly formatted (PEP 8: 4 spaces).

Remove also some trailing spaces.

(cherry picked from commit e87ada4)

* gh-94026: Buffer regrtest worker stdout in temporary file (GH-94253)

Co-authored-by: Victor Stinner <[email protected]>
(cherry picked from commit 199ba23)

* gh-96465: Clear fractions hash lru_cache under refleak testing (GH-96689)

Automerge-Triggered-By: GH:zware
(cherry picked from commit 9c8f379)

* gh-95027: Fix regrtest stdout encoding on Windows (#98492)

On Windows, when the Python test suite is run with the -jN option,
the ANSI code page is now used as the encoding for the stdout
temporary file, rather than using UTF-8 which can lead to decoding
errors.

(cherry picked from commit ec1f6f5)

* gh-98903: Test suite fails with exit code 4 if no tests ran (#98904)

The Python test suite now fails wit exit code 4 if no tests ran. It
should help detecting typos in test names and test methods.

* Add "EXITCODE_" constants to Lib/test/libregrtest/main.py.
* Fix a typo: "NO TEST RUN" becomes "NO TESTS RAN"

(cherry picked from commit c76db37)

* gh-100086: Add build info to test.libregrtest (#100093)

The Python test runner (libregrtest) now logs Python build information like
"debug" vs "release" build, or LTO and PGO optimizations.

(cherry picked from commit 3c89202)

* bpo-46523: fix tests rerun when `setUp[Class|Module]` fails (#30895)

Co-authored-by: Jelle Zijlstra <[email protected]>
Co-authored-by: Łukasz Langa <[email protected]>
(cherry picked from commit 9953860)

* gh-82054: allow test runner to split test_asyncio to execute in parallel by sharding. (#103927)

This runs test_asyncio sub-tests in parallel using sharding from Cinder. This suite is typically the longest-pole in runs because it is a test package with a lot of further sub-tests otherwise run serially. By breaking out the sub-tests as independent modules we can run a lot more in parallel.

After porting we can see the direct impact on a multicore system.

Without this change:
  Running make test is 5 min 26 seconds
With this change:
  Running make test takes 3 min 39 seconds

That'll vary based on system and parallelism. On a `-j 4` run similar to what CI and buildbot systems often do, it reduced the overall test suite completion latency by 10%.

The drawbacks are that this implementation is hacky and due to the sorting of the tests it obscures when the asyncio tests occur and involves changing CPython test infrastructure but, the wall time saved it is worth it, especially in low-core count CI runs as it pulls a long tail. The win for productivity and reserved CI resource usage is significant.

Future tests that deserve to be refactored into split up suites to benefit from are test_concurrent_futures and the way the _test_multiprocessing suite gets run for all start methods. As exposed by passing the -o flag to python -m test to get a list of the 10 longest running tests.

---------

Co-authored-by: Carl Meyer <[email protected]>
Co-authored-by: Gregory P. Smith <[email protected]> [Google, LLC]
(cherry picked from commit 9e011e7)

* Display the sanitizer config in the regrtest header. (#105301)

Display the sanitizers present in libregrtest.

Having this in the CI output for tests with the relevant environment
variable displayed will help make it easier to do what we need to
create an equivalent local test run.

(cherry picked from commit 852348a)

* gh-101634: regrtest reports decoding error as failed test (#106169)

When running the Python test suite with -jN option, if a worker stdout
cannot be decoded from the locale encoding report a failed testn so the
exitcode is non-zero.

(cherry picked from commit 2ac3eec)

* gh-108223: test.pythoninfo and libregrtest log Py_NOGIL (#108238)

Enable with --disable-gil --without-pydebug:

    $ make pythoninfo|grep NOGIL
    sysconfig[Py_NOGIL]: 1

    $ ./python -m test
    ...
    == Python build: nogil debug
    ...

(cherry picked from commit 5afe0c1)

* gh-90791: test.pythoninfo logs ASAN_OPTIONS env var (#108289)

* Cleanup libregrtest code logging ASAN_OPTIONS.
* Fix a typo on "ASAN_OPTIONS" vs "MSAN_OPTIONS".

(cherry picked from commit 3a1ac87)

* gh-108388: regrtest splits test_asyncio package (#108393)

Currently, test_asyncio package is only splitted into sub-tests when
using command "./python -m test". With this change, it's also
splitted when passing it on the command line:
"./python -m test test_asyncio".

Remove the concept of "STDTESTS". Python is now mature enough to not
have to bother with that anymore. Removing STDTESTS simplify the
code.

(cherry picked from commit 174e9da)

* regrtest computes statistics (#108793)

test_netrc, test_pep646_syntax and test_xml_etree now return results
in the test_main() function.

Changes:

* Rewrite TestResult as a dataclass with a new State class.
* Add test.support.TestStats class and Regrtest.stats_dict attribute.
* libregrtest.runtest functions now modify a TestResult instance
  in-place.
* libregrtest summary lists the number of run tests and skipped
  tests, and denied resources.
* Add TestResult.has_meaningful_duration() method.
* Compute TestResult duration in the upper function.
* Use time.perf_counter() instead of time.monotonic().
* Regrtest: rename 'resource_denieds' attribute to 'resource_denied'.
* Rename CHILD_ERROR to MULTIPROCESSING_ERROR.
* Use match/case syntadx to have different code depending on the
  test state.

Co-authored-by: Alex Waygood <[email protected]>
(cherry picked from commit d4e534c)

* gh-108822: Add Changelog entry for regrtest statistics (#108821)

---------

Co-authored-by: Christian Heimes <[email protected]>
Co-authored-by: Zachary Ware <[email protected]>
Co-authored-by: Nikita Sobolev <[email protected]>
Co-authored-by: Joshua Herman <[email protected]>
Co-authored-by: Gregory P. Smith <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@tiran tiran

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@tiran @bedevere-bot @vstinner @miss-islington @tim-one
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.