moved rhapsody-in-green off v3
treefmt / nix fmt (pull_request) Successful in 4s
build_systems / prebuild-common-x86-64-v3 (pull_request) Successful in 24s
build_systems / build-portal-1 (pull_request) Successful in 20s
test ebook search / test-ebook-search (pull_request) Successful in 1m13s
build_systems / build-bob (pull_request) Successful in 54s
build_systems / build-brain (pull_request) Successful in 57s
build_systems / build-rhapsody-in-green (pull_request) Successful in 1m4s
build_systems / build-jeeves (pull_request) Successful in 1m30s
treefmt / nix fmt (pull_request) Successful in 4s
build_systems / prebuild-common-x86-64-v3 (pull_request) Successful in 24s
build_systems / build-portal-1 (pull_request) Successful in 20s
test ebook search / test-ebook-search (pull_request) Successful in 1m13s
build_systems / build-bob (pull_request) Successful in 54s
build_systems / build-brain (pull_request) Successful in 57s
build_systems / build-rhapsody-in-green (pull_request) Successful in 1m4s
build_systems / build-jeeves (pull_request) Successful in 1m30s
This commit is contained in:
@@ -0,0 +1,79 @@
|
||||
# pytest-xdist test fixes
|
||||
|
||||
With two workers and a restart limit of three, the fourth worker crash
|
||||
requests shutdown while another test can still be running. That test may
|
||||
also crash. The original queued-work test requires exactly four failures,
|
||||
even though five failures can occur without exceeding the replacement limit.
|
||||
|
||||
## Scope and behavior
|
||||
|
||||
`concurrent-worker-crashes.patch` changes the assertions in
|
||||
`TestNodeFailure.test_max_worker_restart_tests_queued` in
|
||||
`testing/acceptance_test.py`. It requires exactly three replacements, four or
|
||||
five failed tests, the failed-tests exit status, the limit message, and no
|
||||
internal error. It retains the two-worker workload and ten queued tests.
|
||||
|
||||
`worker-startup-timeout.patch` changes the remote-test helper's event timeout
|
||||
from 10 to 60 seconds so loaded builders have time to start workers. The
|
||||
helper returns immediately when an event arrives and still has a bounded wait.
|
||||
|
||||
The existing nixpkgs pytest-9 compatibility patches remain in place.
|
||||
Production scheduling and worker-restart behavior are unchanged.
|
||||
|
||||
## Reproduction and focused checks
|
||||
|
||||
Use a disposable pytest-xdist 3.8.0 checkout with its test dependencies and
|
||||
the nixpkgs pytest-9 compatibility patches where required. From this directory:
|
||||
|
||||
```sh
|
||||
patch --fuzz=0 -d /path/to/pytest-xdist -p1 < concurrent-worker-crashes.patch
|
||||
patch --fuzz=0 -d /path/to/pytest-xdist -p1 < worker-startup-timeout.patch
|
||||
cd /path/to/pytest-xdist
|
||||
python -m pytest testing/acceptance_test.py \
|
||||
-k test_max_worker_restart_tests_queued -q
|
||||
python -m pytest testing/test_remote.py -q
|
||||
```
|
||||
|
||||
Twenty unmodified runs passed during the review. To force the failing
|
||||
schedule, modify the generated crashing test in a disposable checkout to
|
||||
accept `worker_id`: make `gw3` wait for a marker created by `gw4`, and make
|
||||
`gw4` pause 0.1 seconds after creating the marker. Then both have in-flight
|
||||
tests when shutdown starts. Bound the marker wait so a reproduction failure
|
||||
cannot hang the suite. The original assertion fails on five reported
|
||||
failures; the patched test passes.
|
||||
|
||||
## Worker startup and outer concurrency
|
||||
|
||||
[`default.nix`](default.nix) runs the outer suite with one worker to limit
|
||||
nested process pools. This is a Nix test-runner setting; the source timeout
|
||||
change lives in [`worker-startup-timeout.patch`](worker-startup-timeout.patch).
|
||||
|
||||
A separate reproduction inserts an 11-second `pytest_sessionstart` delay
|
||||
into the child created by `test_basic_collect_and_runtests` in
|
||||
`testing/test_remote.py`. The original 10-second channel wait fails; the
|
||||
60-second wait passes. This bounds waits for test worker events, including
|
||||
startup, rather than changing a product deadline.
|
||||
|
||||
## Upstream status
|
||||
|
||||
These are standalone test patches for pytest-xdist 3.8.0. No upstream submission
|
||||
was made during this work. Recheck the allowed in-flight failures, replacement
|
||||
count, and remote-test wait when updating the scheduler or worker behavior.
|
||||
|
||||
## Local NixOS integration and build results
|
||||
|
||||
[`../default.nix`](../default.nix) loads `default.nix` through
|
||||
`pythonPackagesExtensions`. From the repository root:
|
||||
|
||||
```sh
|
||||
nix build --no-link -L .#nixosConfigurations.jeeves.pkgs.python314Packages.pytest-xdist
|
||||
```
|
||||
|
||||
After consolidating the settings in this directory, the full x86-64-v3 package
|
||||
build passed 185 tests, with 6 existing skips and 10 expected failures. Nix
|
||||
evaluation confirmed the same outer-worker limit and preserved existing
|
||||
patches, with the timeout now applied as a source patch.
|
||||
|
||||
The earlier forced concurrent-crash and delayed-startup reproductions passed
|
||||
after their fixes; the focused crash test also passed after formatting its
|
||||
assertion.
|
||||
@@ -0,0 +1,29 @@
|
||||
Subject: [PATCH] tests: count replacements when checking the worker restart limit
|
||||
|
||||
With two workers, another in-flight test may crash after the fourth
|
||||
crash requests shutdown. Either four or five failed tests is valid.
|
||||
Require exactly three replacements and the failed-tests exit status,
|
||||
while preserving the queued-work and no-internal-error assertions.
|
||||
|
||||
--- a/testing/acceptance_test.py
|
||||
+++ b/testing/acceptance_test.py
|
||||
@@ -1011,9 +1011,18 @@
|
||||
"worker*crashed while running*",
|
||||
"worker*crashed while running*",
|
||||
"* xdist: maximum crashed workers reached: 3 *",
|
||||
- "* 4 failed in *",
|
||||
]
|
||||
)
|
||||
+ # A second in-flight test may crash after shutdown is requested.
|
||||
+ # The restart limit constrains replacements, not concurrent failures.
|
||||
+ replacements = sum(
|
||||
+ line.startswith("replacing crashed worker ") for line in res.stdout.lines
|
||||
+ )
|
||||
+ assert replacements == 3
|
||||
+ failed = res.parseoutcomes()["failed"]
|
||||
+ assert failed in (4, 5)
|
||||
+ res.assert_outcomes(failed=failed)
|
||||
+ assert res.ret == pytest.ExitCode.TESTS_FAILED
|
||||
assert "INTERNALERROR" not in res.stdout.str()
|
||||
|
||||
def test_max_worker_restart_die(self, pytester: pytest.Pytester) -> None:
|
||||
@@ -0,0 +1,12 @@
|
||||
{ pytest-xdist }:
|
||||
pytest-xdist.overridePythonAttrs (old: {
|
||||
patches = (old.patches or [ ]) ++ [
|
||||
./concurrent-worker-crashes.patch
|
||||
./worker-startup-timeout.patch
|
||||
];
|
||||
|
||||
# The suite exercises its own worker pools. Limit the outer suite to one worker.
|
||||
preCheck = builtins.replaceStrings [ "--numprocesses=$NIX_BUILD_CORES" ] [ "--numprocesses=1" ] (
|
||||
old.preCheck or ""
|
||||
);
|
||||
})
|
||||
@@ -0,0 +1,19 @@
|
||||
Subject: [PATCH] tests: allow more time for remote worker events
|
||||
|
||||
Worker startup can exceed ten seconds on heavily loaded builders. Allow
|
||||
the remote-test helper to wait up to sixty seconds for worker events.
|
||||
The wait still returns as soon as an event arrives and remains bounded.
|
||||
Production worker timeouts and test assertions are unchanged.
|
||||
|
||||
--- a/testing/test_remote.py
|
||||
+++ b/testing/test_remote.py
|
||||
@@ -17,7 +17,8 @@
|
||||
from xdist.workermanage import WorkerController
|
||||
|
||||
|
||||
-WAIT_TIMEOUT = 10.0
|
||||
+# Allow worker events extra time on heavily loaded builders.
|
||||
+WAIT_TIMEOUT = 60.0
|
||||
|
||||
|
||||
def check_marshallable(d: object) -> None:
|
||||
Reference in New Issue
Block a user