Repository navigation
Run every browser check in CI - #136
Merged
Merged
Conversation
A new Interface checks job runs each check script that only npm scripts reached, one step each, and the Node 22 job runs the website checks next to the blog check. CI passed requires the new job.
- check-skills still expected the commit form of tasks that accept straight into the manuscript, removed in #128; it now checks the save card of a deferred task. - check-review-pdf-appearance expected the phone decision dock at the bottom edge; since the mobile redesign it floats just above the marking tools, which the check now verifies. - check-fast-decisions pressed keys as soon as the current change was shown, before the viewer's key listener was attached; it waits a frame. - On Linux fonts, Spanish "Desactivado" overlapped "Pocas" under the note-alternatives slider at 390 px. On phones the labels are now spaced out instead of placed under the stops.
Member
Author
|
@codex review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
For over an hour the runners could not resolve Ubuntu's archive, so every Linux job failed while installing Chromium's system libraries (--with-deps) or the desktop smoke test's packages. The image already has the libraries Chromium needs, and most desktop packages: Chromium installs without --with-deps, and the desktop step asks the mirror only for packages the image lacks.
Placing each label under its stop overlapped Spanish "Desactivado" and "Pocas" on Linux fonts at 768 px too, not only on phones: the space depends on the settings card, not the window. The labels are now spaced out from end to end at every width.
The image lacks libnotify4, libfuse2 and openbox, so the desktop smoke test still needed Ubuntu's mirror while the runners could not resolve it. Their .deb files are now cached per image, as TeX's are: main saves them, and pull requests install from the cache without the mirror.
check-review-batching pressed Accept as soon as the current change was shown, before the viewer's key listener effect had run, and the key was lost now and then on CI. A shared afterRender wait in scripts/playwright.mjs now precedes the first key in both review keyboard checks.
On CI's Node 22 job, with every test file running at once, the first C could arrive before the viewer listened for keys; the composer never opened and the test waited out its 30 s. It now presses C until the composer shows (C does nothing once it is open) and allows 90 s.
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
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.
What and why
The last step of "every test runs in CI or is gone": 37 browser check scripts ran only through
npm run test:uiand similar npm scripts, which no workflow ran.Interface checksjob (.github/workflows/ci.yml): each of the 37 scripts as its own step (one failure does not hide the others), with real TeX and Chromium; failed runs keep their screenshots.CI passednow requires it (scripts/ci-passed.sh,tests/ci-passed.test.ts).check-site,check-site-analytics,check-site-privacy) run in the Node 22 job next tocheck-site-blog, after the site build.check-desktopandcheck-desktop-notificationsalready run in the desktop workflow;check-pdf-lifecycleruns fromcheck-playground.A trial run of the job (run) passed 33 of the 37 scripts. The four failures, fixed here:
check-skillsexpected the commit form of tasks that accept straight into the manuscript, removed in Drop tasks that accept straight into the manuscript #128; it now checks a deferred task's save card.check-review-pdf-appearanceexpected the phone decision dock at the screen's bottom edge; since the mobile redesign (Redesign the mobile UI with Apple-native hierarchy and touch ergonomics #118) it floats just above the marking tools, which the check now verifies.check-fast-decisionspressed keys as soon as the current change was shown, before the viewer's key listener effect ran, so the first key was sometimes lost; it now waits a frame.check-settings-layout). On phones (≤ 480 px) the labels are now spaced out instead of placed under the stops.How it was checked
check-skills,check-settings-layout(638 combinations),check-review-pdf-appearance,check-fast-decisions(3 consecutive passes)tests/ci-passed.test.ts; workflow YAML parses