acc: run cmd/workspace/apps/run-local-node locally - #6172
Merged
Conversation
Collaborator
Integration test reportCommit: 75b8383
8 interesting tests: 4 RECOVERED, 4 SKIP
Top 3 slowest tests (at least 2 minutes):
|
radakam
force-pushed
the
acc-run-apps-run-local-locally
branch
from
August 5, 2026 16:53
7da0161 to
e1cf319
Compare
radakam
force-pushed
the
acc-run-apps-run-local-node-locally
branch
from
August 6, 2026 08:09
13a315f to
649f5ae
Compare
The test has been disabled since #4875, which took NPM registry access away from the runners. It needed the registry only for the app fixture: express, and the --prepare-environment run that installs it. The fixture is now a standard-library HTTP server started directly with node, so nothing is installed at test time. /shutdown answers with Connection: close so server.close() can complete and the app exits on its own, which lets wait $PID assert that the CLI exited zero. The app throws when PORT is unset rather than letting listen() pick a random port and failing fifteen seconds later as a proxy timeout; PORT is read in preference to DATABRICKS_APP_PORT because it is what Node apps read and what GetBaseEnvVars sets for that reason, so it covers an alias the python fixture cannot. Ports come from free_port.py, and the Timeout overrides go away because the run takes a few seconds rather than 2m. --prepare-environment comes back with it. The flag had no test left anywhere: the python sibling dropped it for needing PyPI, and neither PrepareEnvironment has a unit test. npm install only reaches the registry when there is something to install, so the fixture keeps an empty dependency list and gains a build script, and a single run covers both halves of NodeApp.PrepareEnvironment: the install has to succeed for the build script to run at all. The build script is a file rather than an inline command because npm's shell quotes inline arguments differently on Windows, and the assertion greps for the exact output. With nothing to install the one request npm still makes on its own is the check for a newer npm, which the rejecting proxy blocks, so app/.npmrc turns that check off. --debug stays too, unlike in the python sibling where it needs debugpy from PyPI: node debugging is only NODE_OPTIONS=--inspect. The test asserts both the port the CLI reports and that node listens on it, so --debug-port keeps its coverage. The CLI prints its line only after the proxy accepts connections, so the curl that reaches the app can beat it and that grep is retried. A failed run reaps the app, proxy and debug ports. node binds the inspector port before it runs app.js, so the debug port is the only handle on an app that died before it listened.
radakam
force-pushed
the
acc-run-apps-run-local-node-locally
branch
from
August 6, 2026 14:13
00640ba to
75b8383
Compare
radakam
marked this pull request as ready for review
August 6, 2026 14:25
denik
approved these changes
Aug 6, 2026
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
Aug 6, 2026
Collaborator
Integration test reportCommit: b664bb6
17 interesting tests: 6 FAIL, 4 flaky, 4 RECOVERED, 2 SKIP, 1 KNOWN
Top 50 slowest tests (at least 2 minutes):
|
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-node, disabled since #4875 for needing NPM registry access. The fixture is now anode:httpserver run directly withnode, soexpressand the 2m/10m timeouts are gone. Ports come fromfree_port.py,/shutdownlets the app exit on its own sowait $PIDasserts the CLI's exit code, and a failing run reaps the app, proxy and debug ports.--prepare-environmentand--debugstay. The app has no dependencies, sonpm installfetches nothing, andapp/.npmrcturns off npm's check for a newer npm, which is the one request it still makes on its own; one run covers both halves ofNodeApp.PrepareEnvironmentand the--entry-pointoverride. The build script is a file rather than an inline command because npm quotes inline arguments differently on Windows and the assertion greps for its exact output. Node debugging is onlyNODE_OPTIONS=--inspect, so the test asserts the port the CLI reports and that node listens on it.Why
Only the fixture needed the registry, for
express.--prepare-environmentis untested anywhere else: the python sibling dropped it for needing PyPI, and neitherPrepareEnvironmenthas a unit test.