acc: run cmd/workspace/apps/run-local locally - #6169
Open
radakam wants to merge 4 commits into
Open
Conversation
Collaborator
Integration test reportCommit: 7da0161
11 interesting tests: 4 RECOVERED, 4 SKIP, 3 flaky
Top 25 slowest tests (at least 2 minutes):
|
radakam
marked this pull request as ready for review
August 5, 2026 11:37
denik
approved these changes
Aug 5, 2026
radakam
enabled auto-merge
August 5, 2026 12:12
The test was disabled in #4765 for using fixed ports and being slow, so apps run-local has had no coverage on PRs since. Both problems come from the fixture rather than the command: the ports were hardcoded, and the test asked for --prepare-environment, which installs the seventeen default app libraries via uv. That install can no longer work at all, because acceptance tests now run with UV_OFFLINE and only a handful of vendored wheels. Ports are now allocated per run from a new acceptance/bin/free_port.py, and the app is a standard-library HTTP server started directly instead of Flask under a freshly built venv, so the run needs no package index. The test covers the same ground otherwise: valueFrom rejection, --entry-point, app.yml env vars, and the proxy's identity headers. The --debug variant is dropped because debugpy is only reachable from PyPI. The debugpy invocation it exercised is asserted in libs/apps/runlocal unit tests.
The fixture answered /shutdown and then called os._exit(0) from inside the handler, so the response reached the client only because the kernel drained the socket buffer on exit. An abortive close would surface as a curl failure, and the script no longer tolerates one: unlike the version this test was modelled on, it has neither "|| true" on the shutdown request nor on the final wait. Stopping the serve loop from a second thread closes the connection cleanly and lets the process exit on its own, which also makes "wait $PID" assert that the CLI exited zero. The two ports now go through add_repl.py, so the output tells the proxy port and the app port apart instead of rendering both as $(port); that makes the generic 127.0.0.1 replacement dead, so it goes. With a stable placeholder for the proxy port the test can assert the "To access your app go to" line, which the previous version only waited for. Both run-local invocations are traced so each LOG file records the command that produced it.
The trap killed $PID, which is not the CLI: backgrounding "trace" forks a subshell, so the CLI and the Python app it starts are grandchildren and survive the script. A failure anywhere after the app comes up left both processes running, reparented to init, holding the two ports and an open handle on LOG.run. Windows reaches the same state by another route, because kill terminates the CLI outright instead of giving it the chance to stop the app, and there a leaked handle also blocks the temp directory cleanup. Cleanup now falls back to kill_port.py, which is what this script used before and what run-local-node still does. It runs only when the script failed: on success both processes have exited through /shutdown already, and the ports are free and may have been handed to another test by the time the trap fires.
The trap explained that $PID is a subshell whose CLI and app survive being killed, then killed $PID anyway. Reaping by port already covers both, so the kill goes and the comment says why the port is the handle we have. Also reach the server through self.server in the fixture rather than the module global declared below the handler.
radakam
force-pushed
the
acc-run-apps-run-local-locally
branch
from
August 5, 2026 16:53
7da0161 to
e1cf319
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Changes
Re-enable
acceptance/cmd/workspace/apps/run-local, disabled since #4765.acceptance/bin/free_port.pyand registered as output replacements.run-local-nodeuses the helper too, replacing its inline copy; that test is disabled everywhere (Badness: needs NPM registry access), so CI does not exercise the change there./shutdownstops the serve loop from a second thread so the response is delivered before the connection closes.kill $PIDcannot: backgroundingtraceforks a subshell, so the CLI and the app it starts are grandchildren of the script and outlive it.Timeout/TimeoutWindowsoverrides, so the test takes the root defaults (60s, 90s on Windows, doubled in CI) rather than 2m/10m. They existed for the package installs that are gone; the test now runs in well under a second.--debugvariant: it needsdebugpyfrom PyPI. The invocation it exercised is asserted inlibs/apps/runlocalunit tests.Why
Both reasons the test was disabled — fixed ports and slowness — came from the fixture.
--prepare-environmentinstalls the seventeen default app libraries viauv, which can no longer work now that acceptance tests run withUV_OFFLINE. Without it the test covers the same ground in under a second:valueFromrejection,--entry-point,app.ymlenv vars, the proxy URL reported for--port, and the proxy's identity headers.