Run smoke tests against a built quarto (binary mode) - #14706
Draft
cderv wants to merge 63 commits into
Draft
Conversation
Collaborator
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
cderv
force-pushed
the
test/smoke-tests-built-version
branch
from
July 17, 2026 18:54
f1ca881 to
64a0f89
Compare
cderv
force-pushed
the
test/smoke-tests-built-version
branch
from
July 17, 2026 19:39
12d6e7e to
f8368cf
Compare
cderv
added this pull request to stack #14874
September 9, 2026 13:59
Analysis of the current test harness (in-process quarto() invocation, json-stream log capture, smoke-all/_quarto.tests dispatch, ff-matrix bucket routing, release CI artifacts) plus a proposal: - Add a subprocess 'binary mode' seam (QUARTO_TEST_BIN) in tests/test.ts reusing the existing --log/--log-format json-stream contract - Pair built binaries with a repo checkout at the matching v<version> tag to avoid harness/binary version skew - Parameterize test-smokes.yml (dev | release | artifact install modes) - Mode A: scheduled workflow testing the GitHub prerelease - Mode B: smoke a built artifact inside create-release.yml - Phased roadmap starting with the feature-format-matrix bucket Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- Weekly schedule plus manual workflow_dispatch trigger - Scope is full smoke-all (ff-matrix bucket as optional extra) - Primary mode is preventive build-then-test: the workflow builds the linux-amd64 dist itself (same configure.sh + quarto-bld prepare-dist steps as create-release.yml make-tarball) and tests that artifact in the same run, so harness and binary share one commit - Published-(pre)release testing becomes a secondary dispatch input for Windows coverage and version backfill - create-release.yml gating deferred until the mode has a track record Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Full sweep of tests/smoke/**/*.test.ts: 107 compatible as-is, 24 adaptable through four mechanical patterns (shared runQuarto dispatch helper, quartoDevCmd honoring QUARTO_TEST_BIN, consolidating hardcoded package/dist/bin spawns, two semantic one-offs), and only 2 genuinely dev-only yaml-intelligence unitTest files that should move to tests/unit/ instead of carrying a requiresDevQuarto flag. Also audits smoke-all documents: docs are binary-clean except ten fixture extensions pinned to quarto-required '>=99.9.0', which hard-error against a real-version binary; recommend relaxing to '>=1.9'. Records a no-reorganization verdict with two greppable lint rules to keep future tests binary-compatible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Incorporates four verification passes with file:line evidence: - Git archaeology: the ten quarto-required '>=99.9.0' fixtures are scaffolding artifacts (create-extension template computes the value from the running dev version); no test exercises version gating; all ten are safe to relax to '>=1.9'. QUARTO_FORCE_VERSION=99.9.9 is rejected (masks version gates, flips quarto check into dev mode). - Log pipeline: --log/--log-format json-stream confirmed global and per-record flushed; render failures write ERROR before exit 1; pandoc/typst/run passthroughs are the documented exception. - Env hygiene: full strip/allow/overlay contract with clearEnv; DENO_DIR/QUARTO_DENO added to strip list; dev-mode trap documented (in-repo package/dist/bin/quarto runs TS sources — dist must be extracted outside the checkout, guarded by a 99.9.9 version check). - CI: quarto-dev action must run in every mode (harness runtime); built binary is a PATH override on top, not a replacement; prepare-dist writes only to package/pkg-working so build+test can share a checkout. Windows built layout ships both quarto.cmd (prepare-dist) and the Rust launcher quarto.exe (installer job); release testing targets quarto.exe. Includes concrete seam spec (runQuarto helper), parameterized test-smokes.yml step diff, test-smokes-built.yml sketch, and phased roadmap with acceptance criteria. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Five-lens adversarial review (19 agents, every critical/major finding independently verification-checked); 32 findings stood, none refuted. Design changes: - Version marker must use semver BUILD metadata (+test.YYYYMMDD): the vendored semver throws on the 4-component string the v2 sketch produced, and any '-prerelease' marker fails all >=X.Y ranges (verified by executing deno.land/x/semver@v1.4.0), which would have broken every quarto-required gate including the freshly relaxed fixtures. - Silent-green invariant: a child failing before logger init (deno startup, bundle load, missing share) or via commandFailed paths exits non-zero with an empty log that vacuously passes noErrorsOrWarnings (~23% of the 1424-doc corpus is log-only); runQuarto must synthesize an ERROR record for record-free non-zero exits. - Keep QUARTO_SHARE_PATH exported for the harness process in all modes (getenv throws when unset, killing smoke-all at module load); child isolation via spawn-time strip only. - Env mechanism flipped from clearEnv+allowlist to inherit+strip (Windows system-var surface too large to enumerate safely); QUARTO_DENO_DOM corrected as the real leak vector. - runQuarto gains throwOnFailure (direct/setup call sites keep throw semantics; test.execute uses no-throw), process-tree timeout kill (launchers spawn-and-wait, so killing the child orphans deno and corrupts the two-writer log), piped+drained stdout/stderr with stderr tail in failure reports, and logger-lifecycle branching in test(). - CI: runners input to restrict artifact mode to linux (Windows leg would download a Linux tarball), buckets becomes optional, resolve job for release mode, fork guard, Windows setup-step gates extended to non-dev full runs, release mode scoped to post-Phase-1 tags (older tags lack harness plumbing and local composite actions). - Corrected citations: nightly create-release runs are build-only (publishing comes from dispatch), prepare-dist also regenerates src/ artifacts (separate build/test jobs are a requirement), dev-symlink location, local configure-test-env PATH-quarto nuance. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ten fixture extensions declaring quarto-required '>=99.9.0' got that value from create-extension scaffolding run on a dev build (the template computes major.minor.0 from the running version, which is the 99.9.9 dev sentinel). No test exercises version gating, and sibling fixture extensions use realistic constraints, so relax them to '>=1.9' to make the fixtures loadable by a real-version built quarto. No behavior change under dev (99.9.9 satisfies >=1.9). Also convert drafts-env.test.ts from a process-global Deno.env.set (flagged in .claude/rules/testing/test-anti-patterns.md as a parallel test race) to the sanctioned per-test TestContext.env channel. Part of the built-version smoke testing plan (dev-docs/smoke-tests-built-version-plan.md). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduces tests/quarto-cmd.ts as the single dispatch point for invoking the quarto under test. Without QUARTO_TEST_BIN nothing changes: quarto() runs in-process exactly as before. With it, quarto is spawned as a subprocess with --log/--log-format json-stream so the existing log-record verifiers work unchanged against a built distribution. Seam design (see dev-docs/smoke-tests-built-version-plan.md): - runQuarto() dev branch preserves the historical in-process call and timeout semantics byte-for-byte; the binary branch spawns with inherit+strip env (dev-tree identity vars like QUARTO_SHARE_PATH, DENO_DIR, QUARTO_DEBUG never reach the child; per-test env overlays last), pipes and drains stdout/stderr, and kills the whole process tree on timeout (the launcher spawn-and-waits on deno, so killing only the direct child would orphan the renderer). - Silent-green guard: a child can exit non-zero without writing any ERROR record (failures before logger init, quarto add/remove commandFailed paths). runQuarto appends a synthetic ERROR record (exit code + stderr tail) whenever a non-zero exit leaves the log record-free, so log-only verifiers like the default noErrorsOrWarnings cannot pass vacuously. - throwOnFailure defaults to true so direct call sites (module-level project pre-render in smoke-all, context.setup pre-renders) keep fail-loudly semantics; testQuartoCmd passes false and lets verifiers consume the log records. - test() skips initializeLogger/cleanupLogger/flushLoggers in binary mode (the child owns the log file; cleanupLogger would tear down the default handlers for subsequent tests) and appends harness-side failures to the log file directly. - TestContext.requiresDevQuarto marks in-process tests to ignore in binary mode (currently unused; escape hatch). - quartoDevCmd() returns QUARTO_TEST_BIN when set, migrating the subprocess-based tests (run/, lua-unit, logging, create) wholesale. - run-tests.sh/.ps1 verify the binary before running (fail on the 99.9.9 dev sentinel, which means the launcher resolved to dev mode) and default to smoke/ in binary mode (unit/ and integration/ are dev-only). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replaces every 'import { quarto } from src/quarto.ts' in smoke tests
with runQuarto from tests/quarto-cmd.ts so these tests work in both
dev mode (unchanged in-process behavior) and binary mode
(QUARTO_TEST_BIN subprocess):
- Setup pre-renders keep fail-loudly semantics (throwOnFailure
defaults to true): render-freeze, render-format-extension,
render-output-file-collision, extension-render-journals,
extension-render-typst-templates, self-contained/stdout,
issues/9133 (parallel renders; no shared log file since two
concurrent children would interleave one json-stream).
- Hand-rolled execute() bodies now accept the harness log file and
pass throwOnFailure:false, mirroring testQuartoCmd: crossref/syntax,
convert/issue-12318, jupyter/cache.
- jupyter/issue-10097 and issue-12374 rewritten as plain testQuartoCmd
calls (log-only verifiers).
- engine/invalid-engine-in-project: the previous assertRejects was
never awaited, so the test passed regardless of outcome. Rewritten
as testQuartoCmd + printsMessage on the specific engine error, which
actually asserts the failure and works in both modes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… weekly workflow test-smokes.yml gains workflow_call inputs (quarto-install: dev|release|artifact, quarto-version, quarto-artifact-name, ref, runners; buckets becomes optional) while keeping every default-path behavior identical — the quarto-dev action still runs unconditionally in all modes because run-tests.sh hardcodes the harness Deno runtime it provisions. Non-dev modes add: release install via quarto-actions/setup, artifact download + extraction OUTSIDE the checkout (the installed launcher enters dev mode when a sibling src/quarto.ts exists), and a pin-and-verify step that fails on the 99.9.9 dev sentinel, validates the version marker shape (build metadata only — prerelease suffixes fail every semver range in the vendored library), checks the checkout has binary-mode support, and exports QUARTO_TEST_BIN. Windows playwright/node setup gates also fire for full non-dev runs. test-smokes-built.yml (weekly Monday + workflow_dispatch) implements the preventive build-then-test mode: build the linux-amd64 dist from the current commit with the same configure.sh + quarto-bld prepare-dist recipe as create-release.yml make-tarball, versioned with semver build metadata (+test.YYYYMMDD), then run the full smoke suite against it at the same SHA on ubuntu only. A dispatch-only source=release path resolves pre-release/release/explicit versions from quarto.org and tests the published binary at its matching v<version> tag (scoped to releases that contain binary-mode support; preflighted via the contents API). Validated with actionlint 1.7.7 (zero findings) and yaml parse; not yet exercised on CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> # Conflicts: # .github/workflows/test-smokes.yml
Tests that spawn quarto themselves (via execProcess or Deno.Command) previously inherited the harness's full environment — including QUARTO_SHARE_PATH, QUARTO_DEBUG and DENO_DIR exported by run-tests.sh, which the installed launcher inherits when set, so in binary mode the spawned binary would silently use dev-tree resources. Adds quartoSpawnEnvOptions() to tests/quarto-cmd.ts: in binary mode it returns the ambient env with the dev-tree strip list applied (per-test overlay merged last) plus clearEnv, and in dev mode today's inherit-and-merge behavior exactly. execProcess forwards clearEnv to Deno.Command untouched, so no src/ change is needed. Applied at every quarto spawn site in run/, lua-unit, logging, create, filters/ editor-support and typst-gather; the hardcoded 'quarto' and package/dist/bin paths in stdlib-run-version, editor-support and typst-gather now resolve through quartoDevCmd() (which honors QUARTO_TEST_BIN). Semantic one-offs: env/check.test.ts asserts a real semver version in binary mode instead of the 99.9.9 dev sentinel; inspect-standalone-rstudio passes RSTUDIO=1 via per-test env in binary mode (the in-process _setIsRStudioForTest hook cannot cross the process boundary and remains the dev-mode mechanism). The two in-process yaml-intelligence tests were moved to tests/unit/ in the previous commit; with that, zero smoke tests need the requiresDevQuarto escape hatch today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Correctness fixes from an 8-angle review of the binary-mode diff: - Shared-logfile truncation: the built quarto opens --log in truncate mode, so a test running two runQuarto calls against one log file (crossref/syntax, convert/issue-12318, jupyter/cache) would have the second invocation erase the first's records, including the synthetic ERROR guard. Each child now writes its own temp log which runQuarto merges into the caller's log file after exit; the record-free check now inspects only that invocation's output. - drafts-env: restore the module-load QUARTO_PROFILE set alongside context.env. src/project/project-profile.ts caches the base profile from the env on the first render in the process, so a per-render override is ignored whenever another test rendered first — the process-global set (before any test runs) is what makes the drafts profile apply in dev mode; context.env is what the spawned binary sees in binary mode. Documented in llm-docs/testing-patterns.md. - Timeout kill portability: process-tree walk now uses pgrep -P (Linux + macOS/BSD) instead of the procps-only ps --ppid, which on macOS silently killed only the launcher and orphaned the renderer. - Dev-branch timer leak: runQuarto's in-process timeout timer is now cleared after the race; migrated direct call sites previously had no timer and must not gain a dangling 10-minute one. - Windows path resolution in CI pin-and-verify: 'command -v quarto' under git-bash yields an extensionless /c/... path that Deno.Command cannot spawn; resolve quarto.exe/quarto.cmd explicitly and convert with cygpath -w. Also export QUARTO_TEST_EXPECTED_VERSION when the caller pinned a concrete version so every spawn is verified against it (previously an unwired guard). - editor-support and typst-gather were pinned to the locally built package/dist/bin/quarto before the migration; quartoDevCmd() would have silently retargeted them to PATH quarto in local dev runs. New quartoDevBinCmd() preserves the dev-tree pinning (QUARTO_BIN_PATH) while honoring QUARTO_TEST_BIN. - Reuse: dev sentinel now imported from src/core/quarto.ts kLocalDevelopment instead of a private literal copy. - Docs: testing-patterns.md env-var section updated to reflect the drafts-env resolution and the TestContext.env channel; testing rules core-file tables list tests/quarto-cmd.ts. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…input An empty buckets input (the default, and what schedule runs use) keeps the full smoke run; a dispatch can pass a JSON list of bucket globs (e.g. the feature-format-matrix qmd glob) for a cheaper validation run before committing to the full 1400-document suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The configure.sh + prepare-dist + tarball recipe existed three times: create-release.yml make-tarball, make-arm64-tarball, and the new test-smokes-built.yml build job. Extract it into .github/actions/build-dist-tarball (inputs: version, arch, tarball-name, artifact-name) and use it from all three call sites. Behavior-preserving for create-release.yml: same commands, same --owner/--group root ownership, same tarball top-level directory (quarto-<version>), same artifact names; the only mechanical change is tar -czf instead of the equivalent tar cvf + gzip two-step. The prevent-rerun and version_commit checkout steps stay in the workflow. This also guarantees the weekly built-version smoke run exercises exactly the release build recipe by construction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…lease run The signing steps in make-installer-win run unconditionally, so every create-release run - including the nightly no-publish schedule - already produces a signed Windows Zip (the real quarto.exe) and the linux Deb Zip tarball as workflow artifacts. source=nightly reuses them instead of building anything: resolve a create-release run (input run-id, or latest successful), check out its exact commit for the harness, download its artifacts cross-run (test-smokes.yml gains a quarto-artifact-run-id input backed by download-artifact's run-id/github-token support), and run the smokes on both OSes. This is the preventive Windows coverage path: signed shipped binaries, zero extra build or signing infrastructure, harness at the same commit by construction. The artifact install step now handles both layouts (linux tarball with a quarto-<version>/ top dir, Windows zip with bin/share at the root, extracted via bsdtar as test-zip-win does). Only works once the commit create-release built from contains the binary-mode harness (post-merge); the pin-and-verify preflight fails with a clear message otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
binaryMode() returned the binary path, which made boolean call sites read awkwardly (binaryMode() !== undefined). Split the API: quartoTestBin() returns the path for spawn sites; isBinaryMode() is the boolean predicate for mode checks in test.ts and the two mode-aware tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
First tranche of the documentation audit for the QUARTO_TEST_BIN binary mode: debugging-flaky-tests gains a binary-mode isolation tip (a flake vanishing in binary mode implicates in-process harness state), the prerelease checklist gains an optional post-publish validation dispatch of Smoke Tests (Built Version), llm-docs/testing-patterns.md gains staleness frontmatter and a Dev Mode vs Binary Mode section with the authoring rules, and the testing rules overview points at binary mode. Remaining audit edits (tests/README.md et al.) follow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Four mermaid diagrams in tests/README.md (GitHub renders them natively): the runQuarto dispatch seam (dev vs binary mode), the lifecycle of one binary-mode test (per-invocation child log, merge, synthetic-ERROR guard, tree kill), the CI workflow map (which workflow tests what, and how the modes relate to create-release and the shared build action), and the QUARTO_TEST_BIN handoff chain from workflow input to per-test dispatch. All four validated with mermaid-cli. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…CLI resolution The weekly schedule previously rebuilt a dist that create-release had already built hours earlier from nearly the same commit - unsigned and linux-only, where the nightly artifacts are signed and include the Windows quarto.exe. The schedule now resolves to source=nightly; source=build becomes dispatch-only, keeping its unique job: testing an arbitrary ref (PR branches, forks, pre-merge validation) without needing signing secrets or existing create-release runs. Also replaces the curl/jq run-resolution with the preinstalled gh CLI (one readable line per lookup), and documents the on-demand signed-build composition: dispatch create-release with publish-release=false, then source=nightly with that run id. Mode resolution is one expression repeated in job conditions: schedule => nightly; dispatch => the source input (default build). CI diagram and plan updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Implements the conclusions of two studies (create-release branch dispatch; daily dev-vs-built migration): - test-smokes-built.yml now triggers via workflow_run after every completed nightly create-release build instead of a weekly cron: once per build, right after it, no stale-artifact risk (the previous cron resolved 'latest successful run', silently testing yesterday's commit when a build failed or was slow). A failed nightly build means a visibly skipped day, not a false green. - New macOS smoke leg from the nightly signed+notarized Mac Zip - the only macOS smoke coverage in CI (previously the Mac build only ever got quarto check). test-smokes.yml gains a macOS system-deps step (brew poppler/librsvg) and the artifact install step now detects layout instead of hardcoding strip-components (the linux tarball has a version top dir; the Mac tarball and Windows Zip are flat). - create-release.yml gains a smoke-artifacts-only dispatch input that skips source/arm64 tarballs, deb/rpm installers and the macOS build, making 'signed build of a branch tip' cheap (verified safe: all publish/tag/docker/cloudsmith paths are input-gated, no protected environments restrict secrets on branches, version_commit resolves empty so jobs build the dispatched ref); publish-release is guarded against partial builds. - The daily dev-mode test-smokes.yml schedule is deliberately left untouched: the plan records the remaining migration decisions (dedicated daily unit/integration job, then downgrade daily dev to weekly) as maintainer calls after a ~2-week green track record. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
macOS smoke runs are reserved for the scheduled built-version path (test-smokes-built.yml nightly mode, once per nightly build); it must not be added to per-commit callers, which need to stay fast. Encodes maintainer intent so the constraint survives as policy rather than just current wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The trigger for test-smokes-built.yml changed from a weekly Monday cron to workflow_run on each nightly Build Installers run, and the nightly mode gained a macOS leg. The mermaid diagrams were updated at the time but three prose spots still described the old design: - tests/README.md CI section (weekly on Monday; nightly = Linux+Windows) - test-smokes-built.yml source input description and nightly job comment - plan doc §5.2 trigger sketch, §5.2b framing, and Phase 2 acceptance Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
project-prepost.test.ts checks that i-was-created.txt (extension) and input-files.txt/output-files.txt (issue-10828) exist after render, but only removed them in teardown. Those are fixed repo paths, so a run that crashes between creating them and teardown leaves a stale copy — after which a regression that stops the render creating them still passes verifyPath. Add setup cleanup (matching the safeRemoveIfExists the file already uses in teardown) so each verifyPath proves this render wrote the file, not a leftover. Also clears i-exist.txt up front so pre-render's "must not exist" guard starts clean after a prior crashed run.
Cleanups surfaced while reviewing the binary-mode test harness. runQuarto had grown into one 125-line function interleaving the in-process dev path with the spawn/timeout/log-merge binary path. Split into runDevQuarto, runBinaryQuarto, and a mergeChildLog helper so each mode's mechanics read on their own; behavior is unchanged. Three extension tests chdir'd into a working dir inside setup() and restored cwd only on the success path. When "use/update template" throws (the network step), cwd leaked into later tests, which then ran in — and whose teardown deleted — a directory they never meant to be in. withCwd() restores cwd in a finally, matching the existing withTempDir idiom. Also corrected a run-tests.sh comment that claimed its --version env strip list matched quarto-cmd.ts's kStripEnvVars; it is deliberately the subset that affects version resolution.
Temporary so the built-version smoke workflow runs on this PR branch before it lands on the default branch (workflow_dispatch is not available until then). Push trigger + a single-file bucket keep the run cheap. Both are reverted before merge.
The push trigger and the hardcoded single-bucket override existed only to exercise this workflow on the feature branch during development. Restore the buckets input passthrough (empty = full run) and drop the branch push trigger so the workflow fires only via workflow_run/dispatch as designed.
test.ts treats a missing log target as a hard failure ("test log file is
missing"). mergeChildLog only wrote the caller log when the child produced
output, so a quiet successful command (empty child log, exit 0, no synthetic
ERROR) left no file at all. Harmless for the always-created temp log, but a
smoke test pinning a custom logConfig.log for a quiet command would fail
spuriously. Ensure the caller log exists (empty) in that case so verifiers
read an empty record array instead of the harness reporting a missing file.
killProcessTree runs fire-and-forget inside the timeout callback (child.output() resolves once the kill lands). The POSIX path guards every syscall, but the Windows path awaited an unguarded taskkill Deno.Command: if taskkill can't be spawned that rejection had no handler and could surface as an unhandled rejection under Deno's default. Guard the taskkill call and attach a catch at the call site so a failed kill degrades to a timeout rather than a crash.
readExecuteOutput called bare JSON.parse and threw on the first unparseable line, while hasErrorRecordText already tolerates partial lines. A timeout that kills a built quarto mid-write leaves a torn record; mergeChildLog appends a clean synthetic timeout ERROR after it, but the throw fired before any verifier saw that record, so the test failed with a JSON parse error instead of the timeout. Skip unparseable lines so the synthetic ERROR surfaces.
The Windows catch swallowed a failed taskkill and returned, killing nothing; runBinaryQuarto then keeps awaiting child.output(), so a hung child could block the test process instead of degrading to a timeout. Best-effort Deno.kill on the direct child so output() can resolve. This does not reach an orphaned grandchild renderer, but is strictly better than killing nothing on the near-impossible case that taskkill itself is unavailable.
…eader readExecuteOutput was made to skip every unparseable line, which also disabled the malformed-JSON detection log-level-and-formats.test.ts relies on (it treats a parse throw as a failed "valid JSON output" assertion). Restore the strict reader and instead drop a torn trailing line in mergeChildLog, which is the only place that knows a timeout kill happened and could have interrupted the child mid-write. The synthetic timeout ERROR still reaches the verifiers, and genuine malformed output is caught again.
test-smokes-built.yml becomes resolvers + a scheduler fanning out to three independent legs per source mode: smoke (also the general bucket runner when the buckets dispatch input is set), playwright, and the feature-format matrix. test-ff-matrix.yml gains a workflow_call interface forwarding to test-smokes.yml so it stays the single owner of the ff-matrix bucket glob; its schedule/push/PR triggers keep today's dev defaults via inputs fallbacks. Fixes surfaced by the design review: - playwright-tests.test.ts now passes quartoSpawnEnvOptions() to its render spawns so the built quarto does not inherit the dev-tree env (QUARTO_SHARE_PATH etc.) and silently render with dev resources - the playwright report artifact is named per runner OS: the nightly linux and mac playwright legs share one workflow run and duplicate artifact names fail the upload even on green tests - no windows playwright leg: the browser assertions are hard-ignored on Windows CI (playwright-tests.test.ts ignore gate), so a leg there would render-then-skip and report a misleading green Docs: revise D9 (integration/ff-matrix are no longer dev-only in built mode), document the scheduler layout and the reusable ff-matrix interface, refresh tests/README.md and the testing rules binary-mode notes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s described Add the suites column to the mode table and the legs to the architecture one-paragraph summary, update the CI topology diagram in tests/README.md (ff-matrix legs route through the reusable test-ff-matrix.yml), refresh the stale "integration/ is dev-only" note in testing-patterns.md, and record the binary-mode constraints (quartoSpawnEnvOptions render spawns, windows ignore gate) in the playwright testing rule. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n CI Blocker fix: test-ff-matrix.yml's top-level concurrency group is evaluated in the CALLER's context under workflow_call, so the nightly ff-matrix legs of one test-smokes-built.yml run shared a single cancel-in-progress group and could cancel a sibling leg. The group now carries a per-call suffix (github.run_id + inputs.runners); dev triggers keep their dedup groups via a constant -dev suffix. Hardening and precision: - release legs pass runners explicitly so per-leg OS scope really is tuned in one place - resolve-nightly ignores expired artifacts so a re-test dispatch of an old run-id skips instead of failing at download time - the stale "playwright only on Linux" comment on the Windows ignore gate now states the actual semantics and its coupling to the missing windows leg; wrapper file header fixed (said smoke-all.test.ts) - scheduler header states the per-mode OS scope accurately - test-ff-matrix.yml push/PR paths-ignore excludes the built-only scheduler Docs: correct the "integration/ is not dev-only" overclaim (only the playwright suite runs in built mode; the two other integration tests stay dev shards only), record the residual coverage gaps (preview/serve, publish, installer packages, arm64, visual snapshots) in D9, note in D10 that refs predating the quartoSpawnEnvOptions fix run the old env-leaking playwright wrapper (first post-merge nightlies included), fix the concurrency/nesting claims, bump testing-patterns.md frontmatter, and add a discovery-pointer rule for the built-version CI workflow files. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The julia-engine subtree tests spawn quarto via raw Deno.Command with the fully inherited environment, so in binary mode the built quarto inherits the harness dev env (QUARTO_DEBUG, QUARTO_SHARE_PATH, ...) and crashes in checkReconfiguration reading <root>/configuration outside the checkout. The spawn sites live in the PumasAI/quarto-julia-engine subtree and cannot be fixed in this repo, so merge-extension-tests now skips copying them when QUARTO_TEST_BIN is set (dev shards unchanged) and emits a ::notice::. Adds dev-docs/ci-julia-engine-binary-mode-followup.md with the upstream sanitization plan (mirror kStripEnvVars, then pull-git-subtree and remove the gate), and records the temporary exclusion in the built-version architecture llm-doc and the log-grouping design doc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WFvZcK71eJ3gmmDT6RcFDd
The built dist tarball is intra-run transport only: the smoke, playwright, and ff-matrix legs download it within the same run and nothing reads it afterward. Default 90-day retention just accumulates large stale tarballs, and upload-artifact's default zip compression wastes CPU re-compressing an already-gzipped .tar.gz.
The previous commit put retention-days: 1 in the shared build-dist-tarball action, but create-release.yml uses the same action to produce the "Deb Zip" / "Deb Arm64 Zip" release tarballs, and test-smokes-built.yml re-tests older create-release runs via source=nightly / run-id. A 1-day cap there expires the "Deb Zip" after a day, so re-testing a run older than that resolves has-linux=false and silently skips the Linux smoke/playwright/ff-matrix legs while Windows/Mac artifacts (uploaded elsewhere, still 90 days) keep the run green. Make retention a composite-action input defaulting to the repo default, and cap it to 1 day only at the test-smokes-built build-mode call site, the sole true intra-run transport. compression-level: 0 stays in the action - every caller uploads an already-gzipped .tar.gz.
cderv
force-pushed
the
test/smoke-tests-built-version
branch
from
September 9, 2026 14:36
91ada48 to
f49f957
Compare
readarray is a bash4 builtin; macOS's system bash is 3.2 and the new macOS playwright leg runs this same code path.
… back A nonzero exit does not throw, so the Deno.kill fallback was unreachable when taskkill ran but failed to touch the tree.
create-release.yml skips both DigiCert steps on github.event_name == 'schedule', so the daily workflow_run leg tests an unsigned quarto.exe. Signed Windows coverage only comes from a create-release dispatch. Recorded as D11 in the architecture doc.
axe-exit-codes.test.ts resolved its subprocess quarto via QUARTO_BIN_PATH (always set by run-tests.sh/.ps1) before QUARTO_TEST_BIN, and never sanitized the spawn env - both silently defeated binary mode. Replaced the hand-rolled resolver with the existing quartoDevBinCmd() + quartoSpawnEnvOptions() (same pattern as typst-gather.test.ts). shared.ts rendered its fixtures via a direct in-process import of src/quarto.ts, the documented anti-pattern for binary-mode tests. Switched to runQuarto().
Trivial workflow-YAML tweaks were triggering a full read of the built-version-testing-architecture deep dive. Point at the relevant section instead, and note the grep-check needed when a referenced heading gets renamed.
Git for Windows' bash resolves `tar` to GNU tar, which has no zip support, and the Windows leg's artifact is always a zip. Both nightly Windows smoke legs died at the install step as a result. macOS and Linux stay on the bash/tar path since bsdtar and tar.gz both work there; create-release.yml's own Windows extraction step already dodges this by omitting `shell:` so it runs under pwsh, which is the precedent followed here. Folded in while touching this step: dotfiles from a nested archive root were previously left behind by a bare `mv "$top"/*`, and a find-then-head-1 silently picked one directory when more than one was present instead of failing loudly.
`${{ }}` expressions are substituted into the shell script text before
the shell ever parses it, so a dispatch value containing shell
metacharacters would execute rather than just being read as a string.
Route the two workflow_dispatch inputs used inside run: blocks through
step-level env: instead.
The release preflight collapsed every gh api failure - a missing tag,
an auth problem, a network blip - into the same "predates binary-mode
harness support" message, which sends whoever is troubleshooting a
transient failure down the wrong path. Branch on the actual HTTP 404
and surface anything else verbatim.
The nightly-run fallback had no --branch filter, so it could adopt
artifacts from a branch build instead of main. The macOS system-deps
step ran brew's full auto-update on every invocation for no reason
relevant to installing poppler/librsvg. The smoke-artifacts-only
input description didn't mention that the linux/Windows installer
verification jobs keep running under this mode - they're cheap and
validate the very artifacts it builds, so they were never meant to be
gated.
context.setup() and the context.cwd() chdir ran before the try/finally that owns teardown and the cwd restore. A throwing setup skipped both, leaking the process cwd into every later test in the file. Binary mode widened the blast radius: runQuarto() throws on any non-zero exit, and several setups (axe shared.ts, render-output-file-collision, issue-9133) now call it bare. Moving setup inside the try means teardown can run on paths where setup never created its artifacts, so render-freeze.test.ts and render-site-themes.test.ts switch their teardown removals to safeRemoveIfExists/safeRemoveSync. RunQuartoResult.code was hardcoded to 0 in dev mode, which reads as a success claim quarto() cannot back: it either resolves or rejects, and on CommandError/commandFailed() it calls exitWithCleanup(1) and Deno.exits the process before runDevQuarto could return anything. Making code optional and leaving it undefined in dev mode stops a future `if (result.code !== 0)` from silently passing there while correctly failing in binary mode. Also make the QUARTO_TEST_BIN version probe in run-tests.sh/.ps1 check its own exit code, matching what assertTestBinary() already enforces for binary-mode test runs, and treat an empty-string QUARTO_TEST_BIN as unset to match quartoTestBin()'s own check.
…ale date The create.test.ts rewrite dropped the sentence recording that the writability assertion passes in dev environments even without ensureUserWritable, because resource files there are already 0o644. That's the most useful fact about the step - this branch exists to find exactly this kind of vacuous-in-dev assertion - so restore it alongside the newer explanation of what the smoke test checks. typst-gather.test.ts declared a local runQuarto() with a different signature in a file that also imports quartoDevBinCmd/quartoSpawnEnvOptions from quarto-cmd.ts, shadowing the harness dispatcher's name. Rename it to execTypstGather to remove the ambiguity. The architecture doc's D10 cited a branch commit date as the boundary for the quartoSpawnEnvOptions() playwright fix; that date won't mean anything once the branch merges. Point at the PR instead.
TestContext.requiresDevQuarto was declared, merged in mergeTestContexts, and honored in the ignore computation, but no test in the suite ever set it - the migration to binary mode found no case that needed to skip a test rather than branch on isBinaryMode(), the way inspect-standalone-rstudio.test.ts already does for its analogous problem. Documented dead machinery is worse than no machinery: it reads as a supported pattern to reach for, so remove the field, its merge and ignore branches, and the three doc references that described it.
The version/quarto-version env-binding fix in the prior commit missed two
more dispatch-controlled values in the same run: blocks: the nightly
run-id fallback and the buckets JSON used by both the Linux and Windows
bucket-runner steps. Same substitution hazard, same fix - route each
through a step-level env: var instead of interpolating ${{ }} straight
into the shell/pwsh source.
The finally block ran test.context.teardown() and then Deno.chdir(wd) as sibling statements, so a throwing teardown skipped the restore and left the process cwd pointing at the test's own directory for every later test in the file. That is reachable today, not hypothetical: tests/smoke/extensions/ install.test.ts sets cwd and tears down with a bare Deno.removeSync of "_extensions" (and one case chdirs before removing). When the install or add itself fails there is no _extensions to remove, teardown throws NotFound, and the restore never runs. The preceding commit widened the window by moving setup() inside the same try, so teardown now also runs on paths where setup never created what it wants to clean up. Wrapping teardown in its own try/finally keeps the failure behaviour identical - a throwing teardown still fails the test, with the same error - it just restores the cwd first.
This reverts commit 7b9bf48. Zero users wasn't new information: the July 17 binary-mode classification sweep (commit a465af2) already read all 133 smoke test files, found exactly two genuinely dev-only cases, moved those to tests/unit/ per its own recommendation, and explicitly concluded the flag should stay as an escape hatch for tests not yet written. "Documented dead machinery" was the wrong frame - the machinery was never meant to have a resident today.
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.
Note
Stacked on #14705 (base branch
test/verify-vacuous-pass). Review/merge that first; this branch includes its commit and will rebase cleanly once it lands.Smoke tests only ever run against the in-process dev TypeScript sources (version
99.9.9). Bugs that live in the shipped product — bundling, the packaged Lua filter set, the launcher, signing/notarization side effects — have no CI coverage, because the code path that exhibits them is never executed by the harness.This adds a binary mode so the same smoke suite can run against a built quarto distribution, with no change to how the verifiers work.
Harness
Every
testQuartoCmd()-based test now invokes quarto through one dispatch point,runQuarto()intests/quarto-cmd.ts. In dev mode (default) it calls the in-processquarto()entry point exactly as before. WhenQUARTO_TEST_BINpoints at a built quarto extracted outside the checkout, it spawns that binary as a subprocess with--log <file> --log-format json-stream, merges the child log into the test log, and the verifiers run unchanged — they only ever see log records and rendered output.Dev callers are untouched; binary mode is opt-in via one env var. Around 30 smoke tests that spawned quarto directly or imported
src/quarto.tswere migrated onto the dispatch helper so they work in both modes; arequiresDevQuartoescape hatch skips the few that exercise in-process internals.CI
test-smokes.ymlgains aquarto-install: dev | release | artifactinput (dev callers unchanged). A newtest-smokes-built.ymlorchestrates three sources for the binary under test:nightly— reuses the signed artifacts of a nightlycreate-releasebuild, triggered automatically viaworkflow_run. Tests exactly what the release pipeline produces, on all three OSes, before anything is published.build— builds a fresh linux dist from the current ref; works on forks and PR branches; manual dispatch.release— installs a published (pre-)release at its tag, for post-publish verification.workflow_runwas chosen over modifying the release pipeline so smoke failures can never redden — or block — a real publish. The shared build step is extracted into abuild-dist-tarballcomposite action, andcreate-release.ymlgains asmoke-artifacts-onlyfast path for cheap branch builds.Design decisions and their rationale are recorded in
llm-docs/built-version-testing-architecture.md.Test hardening
Migrating tests onto the dispatch seam exposed several that were silently vacuous — assertions the harness swallowed, or fixtures that never existed. Fixed here: a
writer.releaseLock()-before-close()bug in the editor-support helper, and two missing fixtures (convert/issue-12318,editor-support/all.qmd). The sharedverify.tsvacuous-pass fixes these rely on are in #14705, which this stacks on.Testing
Test-infrastructure only — no user-facing change, no changelog entry. Dev-mode CI exercises the refactored dispatch path on this PR. Binary mode is verified by dispatching
test-smokes-built.ymlwithsource=build, and after merge by the automatic nightlyworkflow_run.