Repository navigation
Fix/release pipeline - #10
Merged
Merged
Conversation
Two gaps left when the device list went in, neither of which fires today and both of which block a release later. `pnpm devices` ran nowhere in the release workflow. It downloads the exact playwright-core that versions.json names as `playwrightBundled` and reads its device descriptors, and `test/devices.spec.ts` asserts the manifest and that version agree. So the first release that moves Playwright fails `pnpm verify` in CI and opens no pull request at all — the test doing its job against a pipeline with no way to satisfy it. The step now runs after the version numbers, which is what it reads. And the release commit staged `content public/avatars app/generated/versions.json`, which does not include app/generated/devices.json. Regenerating it would have changed nothing that got committed. 20.4.0 still depends on playwright ^1.62.1, so neither gap has bitten yet. Separately, a guard for the fault behind #8: a Markdown table row is split on its unescaped pipes before anything else is parsed, code spans included, so | `|=` | contains hyphenated word | `role=button[name|="DARK"]` | parsed as five cells in a three-column table and shipped as a cell reading "`" beside one reading "=`". Nothing failed. Counting cells against the row's own header catches it, and the message names the fix. `\|` is the only escape that works — `|` is left standing as literal text inside a code span; both were run through @nuxtjs/mdc to check rather than assumed.
The 20.4.0 import failed `pnpm verify` on three assertions in test/libdoc.spec.ts, none of which described a defect on this side. Two counted keywords and had 151 written into them. 20.4.0 adds `Set Storage State`, so 152 — and the release stopped, on a number that was only ever a description of one version. What those assertions protect is that no keyword is lost between the spec and the transform, and that the group counts add up to the whole; both now read the total off the spec, which is the thing that knows it. A floor stays, so an empty transform still fails. The third looked for the two "Examples" sections that Browser's introduction happened to contain, and failed with "expected 0 to be greater than 1" — a message about nothing. 20.4.0 rewrote that introduction, 33 headings down to 18, both "Examples" among the ones that went. De-duplicating repeated heading slugs is our behaviour; the prose it was being read from lives in another repository and is free to change. It is now asserted against a two-heading spec written in the test, and also checks that both ids reach the rendered intro, which the old version never did. Verified against both versions: 35 pass with LATEST at 20.3.0, and 35 pass with the real 20.4.0 libdoc generated by scripts/fetch-libdoc.sh. The 20.4.0 content itself is not committed here — the release pull request brings it. Still pinned, and not touched because they pass today: the first two group counts, 32 and 28, in "orders groups by the mapping". They will fail the same way on the first release that adds an Interaction keyword, and the ordering they exist to prove does not need them.
…ainst Three more assertions in test/libdoc.spec.ts described Browser 20.3.0 rather than anything that has to be true. Measured across every committed libdoc first, because "will break soon" was worth checking rather than asserting. Over 19.12.4 → 20.4.0 the keyword count moved five times (147, 148, 147, 151, 152) and the group count once, 19 → 20. The two group counts flagged as risky — Interaction 32, Getters & Assertions 28 — did not move once in twelve releases. They are still the library's numbers sitting inside a test about ordering, so they go, but the honest reason is that they do not belong there, not that they were about to fail. - "no group falls back" also asserted twenty groups. `transform` omits a group no keyword landed in, so the same mapping yields 19 over 19.12.4 and 20 over 20.3.0: a fixed number there is a statement about the version. It now asserts what must hold at every version — nothing fell back, and every group produced is one we mapped. - "orders groups by the mapping" compared the first two names, each with a count. It now compares the whole sequence against the mapping's own order, which catches a group moving into the middle where naming two never would, plus a check that the order is not simply alphabetical — otherwise the comparison would pass against a sorted implementation whenever the mapping happened to be sorted. - "gives the introduction headings ids" matched on `id="finding-elements"`. That is the library's prose, and the release that broke this file rewrote the introduction from 33 headings to 18. What the h2 check guards is the sanitizer stripping those headings outright, so it asserts the shape and leaves the titles to the loop below, which already covers every section. Verified against six versions — 19.12.4, 19.13.0, 20.0.0, 20.1.0, 20.3.0 and the generated 20.4.0: 35 pass on each. The previous assertions fail on four of those six. The ordering check was mutation-tested: alphabetical order fails it, and so does moving one group into the middle.
Contributor
Previewhttps://rfbrowser-pr-10.surge.sh
Rebuilt on every push to this branch, and taken down when it closes. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the site’s release/import pipeline by removing brittle version-specific test assertions, adding a missing generation step for Playwright device descriptors, and introducing a content guard to prevent silently broken Markdown tables from shipping.
Changes:
- Updated libdoc transform tests to assert against the current spec/mapping rather than hard-coded counts and headings tied to a specific upstream Browser release.
- Added a CI content guard that checks Markdown tables remain “square” (row cell counts match the header), catching unescaped pipe issues.
- Updated the library release workflow to generate and commit
app/generated/devices.jsonduring automated release imports.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| test/libdoc.spec.ts | Makes tests resilient to upstream libdoc/spec changes by asserting invariants instead of fixed counts/strings. |
| test/content-mdc.spec.ts | Adds a repo-wide check to catch malformed Markdown tables caused by unescaped pipes. |
| .github/workflows/library-release.yml | Ensures device descriptors are generated after version updates and included in the release import commit. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
No description provided.