Repository navigation
Run Desktop's tests after the browser tests in CI - #1132
Merged
thesiti92 merged 0 commit intoOct 14, 2026
Merged
Conversation
sidkmenon
approved these changes
Oct 14, 2026
thesiti92
force-pushed
the
ci/desktop-tests-after-browser
branch
from
October 14, 2026 20:22
f2401f8 to
a5efe1c
Compare
thesiti92
added this pull request to stack #1131
October 14, 2026 20:22
thesiti92
removed this pull request from stack #1131
October 14, 2026 20:44
thesiti92
force-pushed
the
remote/12-host-down-surfaces
branch
from
October 14, 2026 20:44
a420031 to
13f0aef
Compare
thesiti92
force-pushed
the
ci/desktop-tests-after-browser
branch
from
October 14, 2026 20:44
a5efe1c to
4689f14
Compare
thesiti92
added a commit
that referenced
this pull request
Oct 14, 2026
Stacked PR 8 of 9 for remote reviews (base: `remote/7-remote-ask`; #934, the rename, is merged). Review in order; each PR builds and passes its suites on its own. ## Manual test plan **Purpose.** Builds what a remote Source window needs before anything opens one: the `whiteboard+<serverId>` resolver over the `review` channel, the read-only pin and title for every Source window, and their registration in the navigator. A Source window layers settings as stock VS Code does, but no workspace file, folder `.vscode/settings.json` or host machine setting can lower its read-only rule. There is no UI entry point for a remote Source window yet (PR 9), so the remote half of the read-only check runs at PR 9's head. **Setup.** Worktree at `origin/remote/8-source-windows-trust`. No host is needed for steps 1–3. **Steps.** 1. `(cd apps/review-desktop && TSX_TSCONFIG_PATH=tsconfig.test.json node --import tsx --test --test-force-exit code-oss/src/vs/review/services/remote/reviewWindowAuthorityResolver.test.ts code-oss/src/vs/review/services/configuration/reviewSourceWindowConfiguration.test.ts code-oss/src/vs/review/electron-main/remote/reviewRemoteHosts.test.ts)` → all pass. 2. Local read-only pin. The window shows the review's own checkout of the reviewed commit, so the settings file must be committed: `W='{"files.readonlyInclude":{"**/*":false},"files.readonlyExclude":{"**/*":true},"window.title":"wb hostile title","editor.fontSize":40}'`; `awk '/^const fixture = String.raw`/{f=1;next} /^`;/{f=0} f' apps/review-desktop/scripts/e2e/journeys/remote-source-window.mjs | sed "s#^ git add \.\$# mkdir -p .vscode; echo '$W' > .vscode/settings.json; git add .#" > /tmp/wb-source.sh`; `mkdir -p $H/laptop $H/bin; printf '#!/bin/sh\nexec node %s/packages/review/dist/cli.js "$@"\n' "$PWD" > $H/bin/whiteboard; chmod +x $H/bin/whiteboard`. With Desktop running: `HOME=$H/laptop DEV_REVIEW_HOME=$H DEV_FAST_REVIEW_CLI_NO_DELEGATE=1 PATH=$H/bin:$PATH bash -s -- Local-source Local-second < /tmp/wb-source.sh` → a laptop review whose committed `.vscode/settings.json` tries to lift read-only. 3. Open Local-source → Diff → **Open file** on f.ts → Source window, not titled `wb hostile title`; typing in f.ts changes nothing and shows the read-only notice. 4. With a host online with language features, `api /reviews-api` → its entries still have `available.sourceWindows: false`, and the remote review's Diff view offers no **Open file**. 5. At `origin/remote/9-source-windows` (needs PR 9's entry point), the same against a host, driven as `remote-source-window` drives it: `S wb-test-a 'bash -s -- Remote-source Second-checkout' < /tmp/wb-source.sh` (the step 2 script, so the checkout commits the same `.vscode/settings.json`), and `echo "$W" | S wb-test-a 'mkdir -p ~/.dev/whiteboard-remote/server/data/Machine && cat > ~/.dev/whiteboard-remote/server/data/Machine/settings.json'` for the host's machine settings. Add the host; **Open file** on f.ts → the window is titled `Remote-source — Source — Whiteboard`, its explorer shows `.vscode`, f.ts is read-only and typing shows the notice. Other settings layer as stock VS Code's: the host's `editor.fontSize` 40 applies. **Covered by unit tests.** - `reviewWindowAuthorityResolver.test.ts`: waits up to 60 s for a `connecting` host, then fails `<alias> is offline`; re-resolves a moved forward; other authorities pass through. - `reviewSourceWindowConfiguration.test.ts`: the workspace file, a folder's settings and the host's machine settings cannot lower `files.readonlyInclude`/`files.readonlyExclude`. - `reviewRemoteHosts.test.ts`: removing a machine's last alias closes only its windows. **Cleanup.** Quit Desktop; `rm -rf $H /tmp/wb-source.sh`; if steps 4–5 used a host, common cleanup. ## Reviewer notes - Remote hosts are trusted the way VS Code Remote trusts them: a Source window layers settings and registers host extension contributions as stock VS Code does; only the read-only pin and the title stay Whiteboard's (`docs/superpowers/plans/2026-10-05-adopt-vscode-trust-model.md`, rulings T2, T3, T5). - Not a defect: after a first success the resolver waits on a host that is `connecting` (the stage-4 ruling); removing the host closes its windows, which ends the wait. - Not a defect: the resolver does not override `getConnectionData`; the base service caches it per authority. <details><summary>Common setup for all plans</summary> ## Common setup (read once) - Test each PR in a worktree at its head branch: `git -C <core> worktree add --detach ../review-prN origin/<head>`, then `pnpm install` there. Run everything below from that worktree root. - Helpers (bash or zsh): ```sh R() { node apps/review-desktop/scripts/e2e/remote/remote.mjs "$@"; } export WB_TEST_RUN=manual-$$ # before the first `R up` S() { ssh -F /tmp/wbt.$WB_TEST_RUN/ssh_config "$@"; } # host aliases are wb-test-<name> H=/tmp/wb-home-$WB_TEST_RUN # isolated Desktop home mkdir -p $H/review-desktop/state/user-data/User echo '{"review.experimental.remoteHosts.enabled": true}' > $H/review-desktop/state/user-data/User/settings.json awk '/createRemoteReview = String.raw`/{f=1;next} /^`;/{f=0} f' apps/review-desktop/scripts/e2e/journeys/remote-host.mjs > /tmp/wb-review.sh sed -e 's/open\\":false/open\\":true/' -e 's/wbrepo/wbrepo2/g' /tmp/wb-review.sh > /tmp/wb-push.sh edit() { printf '{"sessionId":"%s","edit":{"type":"insert","content":{"type":"markdown","markdown":"%s"}}}' "$2" "$3" | S "$1" 'whiteboard api session_edit -'; } ``` - Launch (dev build only; a packaged build ignores both env vars): `DEV_REVIEW_HOME=$H DEV_FAST_REVIEW_SSH_CONFIG=/tmp/wbt.$WB_TEST_RUN/ssh_config pnpm dev` (prefix `DEV_FAST_REVIEW_DESKTOP_BACKGROUND=1` to keep it from taking focus). - Desktop API: `D=$(ls $H/review-desktop/instances/dev-*.json); URL=$(node -p "require('$D').url"); TOK=$(node -p "require('$D').token"); api() { curl -s -H "x-review-token: $TOK" "$URL$1"; }` - A remote review: `ID=$(S wb-test-a 'bash -s -- Remote-order' < /tmp/wb-review.sh | head -1)` (repo `~/wbrepo`, review "Remote-order" with a prose line and a code peek of f.ts). `/tmp/wb-push.sh` is the same with `open: true` in `~/wbrepo2`. - Settings: ⌘, → region **Remote hosts** → type the alias in **SSH alias** → **Add**. - Fork unit tests: `(cd apps/review-desktop && TSX_TSCONFIG_PATH=tsconfig.test.json node --import tsx --test --test-force-exit <files>)`. Package tests: `pnpm --filter @dev.fast/whiteboard test <files>`. - Journeys: stage a runtime per `apps/review-desktop/scripts/e2e/TESTING.md` ("Staging the runtime"), then `node apps/review-desktop/scripts/e2e/run.mjs --runtime "$REVIEW_E2E_RUNTIME" --journey <name>`. - The harness has `pause`/`resume` (there is no `unpause`). - Cleanup, every plan: quit Desktop first (its `ssh` masters count as leftovers), then `R down --all; R verify-clean; rm -rf $H /tmp/wb-*.sh`. Without an `aws sso login` session `verify-clean` also prints `AWS could not be checked` and exits 1; no other line may appear. </details> Also runs Desktop's CI tests after the other packages: #939's remote tarball test starved the concurrent browser tests of CPU, so test files failed to import or hung until the 60-minute timeout (was #1132).
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
2 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.
Since #939,
build-remote-tarball.test.mjsbuilds Whiteboard, bundles, installs and xz-compresses (2–3 min, load average ~10 on the 4-vCPU runner) while the Chromium browser tests run beside it. Starved of CPU, Chromium drops a test file's module fetch. The file then fails withFailed to fetch dynamically imported module(#942, #973), or its iframe never reports ready, and the job hangs until the 60-minute timeout (#940, #1110, and two other branches today). Timing-sensitive canvas tests also fail under the same load.This runs
apps/review-desktop's tests after the other packages instead of alongside them. The Test step gets about 3–4 minutes longer.