Skip to content

Browser test cross env#1888

Merged
jpolitz merged 9 commits into
brownplt:drydockfrom
jpolitz:browser-test-cross-env
Jun 28, 2026
Merged

Browser test cross env#1888
jpolitz merged 9 commits into
brownplt:drydockfrom
jpolitz:browser-test-cross-env

Conversation

@jpolitz

@jpolitz jpolitz commented Jun 26, 2026

Copy link
Copy Markdown
Member

No description provided.

jpolitz and others added 8 commits June 26, 2026 06:18
Add a strictly-additive cross-environment harness that runs the SAME mocha
assertions from code.pyret.org's suite against two other places the Pyret
editor renders, without modifying anything under code.pyret.org/.

All three environments render the same editor.html (CodeMirror, #runButton,
#output, .check-block, .testing-summary, "Looks shipshape"); only how the DOM
is reached differs.

- shared/load-cpo-specs.js: extracts the exact (program, expected) specs from
  the unmodified code.pyret.org/test/*.js by requiring them with recording
  shims (no copying -> no drift).
- embed/: reuses code.pyret.org/test-util/util.js UNCHANGED, focusing the
  Selenium driver inside the /embed/embed1.html iframe. 240 passing.
- shared/page-assertions.js + cpo-assertions.js: in-page port of the util.js
  predicates for the Playwright/vscode path; fidelity/run-cpo-fidelity.js
  proves it matches util.js against /editor (224 passing).
- vscode/run-vscode-tests.js: boots headless VS Code for the Web via
  @vscode/test-web + Playwright, opens the pyret-parley.cpo custom editor, and
  runs the same specs inside the webview frame. 232 passing (check-blocks,
  errors, all 10 charts).

Also symlinks vscode/build -> ../code.pyret.org/build so the extension builds
in this monorepo (per vscode/README.dev.md).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… too

The vscode custom editor starts with interactions hidden, but the CPO editor
removes the `hideInteractions` body class on the first run
(beforePyret.js:1594), so the REPL becomes usable. This brings the vscode
webview to full parity with embed (same suite set):

  vscode webview: 236 passing (check-blocks 29, errors 193, charts 10,
                                type-check 3, tables 1)

- Port testRunAndUseRepl + checkTableRendersCorrectly into the in-page
  assertions (REPL submit via .repl-prompt > .CodeMirror, read last #output
  child, table cell vs value HTML compare) -- faithful to util.evalPyret /
  evalPyretNoError. Validated against /editor by fidelity/run-repl-fidelity.js
  (4 passing).
- Fix a real interaction bug exposed by reusing one editor frame across suites:
  the run mode is sticky (cpo-main.js: currentAction), so after a type-check run
  the plain Run button keeps type-checking. PA.run() now selects the explicit
  "Run" dropdown item, resetting the mode. (Upstream never hits this -- a fresh
  browser per suite.)
- Match util.checkTableRendersCorrectly faithfully: its trailing
  checkAllTestsPassed(...) is unreachable dead code (after `return`), so we do
  the per-cell render comparison only.

CPO fidelity re-run after the PA.run() change: still 224 passing, 0 failing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…onments

Replace the dual-mechanism harness (Selenium + util.js for embed; Playwright +
in-page port for cpo/vscode) with a single runner that drives all three
environments through the in-page port of util.js:

    node run.js --env=cpo|embed|vscode [--suites=...] [--limit=N]

An env adapter (envs/<env>.js) returns a Playwright frame focused on the CPO
editor DOM; a shared findEditorFrame (the frame with #runButton) locates it for
cpo's main page, embed's #embed1 iframe, and vscode's webview alike. The port
(shared/page-assertions.js + cpo-assertions.js) then runs the same specs, loaded
from the unmodified code.pyret.org/test/*.js.

Result: each environment runs the same 5 suites and lands on the same total --
  cpo 236, embed 236, vscode 236 passing, 0 failing.

Why it's safe to drop the Selenium/util.js path: the two paths already ran and
agreed (prior commits: embed 240 via util.js; fidelity 224+4 and vscode 236 via
the port). --env=cpo now plays the reference role the fidelity check did. The
port keeps its line-by-line util.js provenance comments.

Removed: embed/*.spec.js, embed/embed-setup.js, fidelity/*, vscode/
run-vscode-tests.js, vscode/probe-vscode.js, the mocha dep (17 -> ~12 files).
Added: run.js, envs/{cpo,embed,vscode}.js, shared/{find-frame,browser}.js, and a
.github/workflows/browser-test.yml CI target (builds cpo + the extension, starts
the server, runs all three envs with Playwright's Chromium).

Hardening exposed by reusing one editor frame across suites:
- editorReady() now waits for CM non-empty (mirrors util.waitForPyretLoad), and
  setDefinitions is confirmed to stick before running, so a program can't race
  the vscode custom editor's async content push (was making `1 is 2` run the
  fixture's `1 is 1` and report Passed).
- browser launch falls back to Playwright's bundled Chromium when
  GOOGLE_CHROME_BINARY is unset (for CI).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Split the workflow into a `build` job (build code.pyret.org + the extension
once, upload code.pyret.org/build and vscode/dist as artifacts) and a `test`
job with strategy.matrix.env: [cpo, embed, vscode]. Each env runs on its own
runner, so chart rendering (CPU-bound headless Chrome) doesn't contend, and the
expensive cpo build happens once instead of three times.

cpo/embed restore the cpo build + start the server; vscode restores only
vscode/dist (self-contained) and needs no server. Tests use Playwright's
bundled Chromium (no GOOGLE_CHROME_BINARY).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the handrolled try/catch + return-true tally with a node:test suite, so
the assertions are real and the two failure kinds are legible:

- Content checks now use node:assert -> a wrong rendering fails as an
  AssertionError with a value diff (e.g. a check block that should contain
  "failed" but shows "Passed").
- Procedural problems (a program that wouldn't install, a value that never
  rendered, the REPL erroring) throw ProceduralError (shared/errors.js).
  The error class distinguishes "editor produced the wrong thing" from "the
  test couldn't be conducted." No more ambiguous bare `true`/`throw`.

tests/suite.test.js boots one environment (PYRET_ENV) once, then registers one
test() per spec loaded from the unmodified code.pyret.org/test/*.js.

run.js becomes a thin CLI over `node --test`: --env selects the environment,
--grep maps to --test-name-pattern (regex over suite + test names), so local dev
gets `npm run embed -- --grep tables` / `node run.js --env=cpo --grep is-not`.
Native node:test reporters (spec/tap/dot/junit) and exit codes.

shared/run-specs.js (the old tally loop) -> shared/dispatch.js (spec -> assertion).

All three environments still pass identically: cpo 236, embed 236, vscode 236.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The results/*.txt logs are regenerated on every run; ignore results/ wholesale
and drop the tracked copies so they don't clutter the PR. (Files kept on disk.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
vscode's devDeps pull @vscode/test-web -> @playwright/browser-chromium, whose
postinstall downloads a ~150MB Chromium. The build job only compiles the
extension (webpack), which needs no browser, so that download was pure waste and
could stall the job. Use `npm ci --ignore-scripts` for the vscode build.

In the test job, also `npm ci --ignore-scripts` and then install exactly one
Chromium explicitly with `npx playwright install --with-deps chromium` (avoids a
redundant implicit download; the vscode env runs @vscode/test-web in
browserType:'none' and launches that Chromium itself).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In CI the first test (check-blocks/simple) timed out: the first program run in a
freshly-loaded editor pays a large one-time cost (runtime warmup + first
check-results render) -- ~20s under CI's Chromium vs ~120ms once warm -- which
blew past testRunsAndHasCheckBlocks' tight 20s doneRendering wait. Every later
test passed. (The first chart/type-check also paid a cold cost but had generous
900s budgets, so only the check-block path, with its 20s internal wait, failed.)

Fix deterministically (no retries, no padded timeouts): run a trivial check
program once in the before() hook and wait for it to finish rendering, so the
warmup cost is absorbed in setup and every actual test runs warm.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@jpolitz

jpolitz commented Jun 26, 2026

Copy link
Copy Markdown
Member Author

Just confirming the witness for the new CI step running is at jpolitz#1:

image

Note the structure – one job does the shared build, then 3 other jobs run the headless tests on each platform.

@blerner blerner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nits but this looks reasonable

}

// mirrors util.setDefinitionsEvalAndWait (set definitions, run, wait for break btn)
async function setDefinitionsRunAndWait(page, code, options) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

amused that this got slightly renamed for no apparent reason...

// mirrors util.evalPyretNoError: submit code at the REPL, wait for a new result
// child of #output, and return its .replOutput/.replTextOutput texts. A result
// that isn't an echo-container/trace means the REPL errored -> procedural.
async function evalAtReplNoError(page, code) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also renamed?

Comment thread browser-test/RESULTS.md Outdated
@@ -0,0 +1,67 @@
# Results

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this file?

Comment thread browser-test/RESULTS.md Outdated
## Notes

- `cpo` and `embed` need the CPO server (they load `/editor`); `vscode` does not.
- Charts are slow (~38s each), so a full run is several minutes per environment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems so improbable, given that charts' performance for real-world use cases is pretty damn fast... are these tests loading external files or something?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's false, that's how long they take when they fail. We should remove this file, thanks.

@jpolitz jpolitz merged commit c997fb3 into brownplt:drydock Jun 28, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants