Test coverage that replaces manual testing (plan 005) - #7
Draft
hatton wants to merge 11 commits into
Draft
Conversation
Nothing ran either suite automatically, so a release depended on someone remembering three commands. Several e2e specs also passed while proving nothing: one asserted on a literal object it had just written, two logged their findings and asserted only that a table exists, and the undo test was skipped with a comment that undo is unsupported, which it no longer is. A GitHub Actions workflow now runs typecheck, the unit suite and the Playwright suite on every push and pull request, and uploads the Playwright report on failure. The screenshot baselines are all -chromium-win32 and this machine cannot render the Linux ones, so playwright.config.ts ignores snapshots when CI is set; the plan records the debt. @vitest/coverage-v8 is installed at the version vite-plus bundles, with a test:coverage script and thresholds a point below the measured baseline of 83.52 statements, 72.56 branches, 81.89 functions, 86.52 lines. coverage.include names src/** instead of excluding demo/ and tests/, because an exclude pattern is matched against the absolute path and a checkout directory whose name ends in "tests" then excludes the whole tree and reports zero of zero. Every spec now takes test and expect from tests/e2e/utils/strict-page.ts, which fails a test that produced a pageerror or a console.error. A silent exception was invisible before. resize-rows-and-columns.spec.ts keeps one real page.mouse row drag and gains the undo test, which now runs through the history manager the harness publishes. borders-changing.spec.ts drops "(BUG)" from titles that describe behaviour that works. table-menu.spec.ts waits on conditions instead of 26 fixed delays. Local retries go from 0 to 1, so a flake reports as flaky. Unit tests: 532 in 37 files. End-to-end: 65, none skipped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1 of plans/005-test-coverage-for-shipping.md. Each module named there gains one test file beside it, written against what a caller of the module can see rather than against how it works. The two React context files carry the tests that matter most to Bloom, which renders the panel in a different realm from the tables: defaultTableApi maps each member to the function it wraps and carries nothing the interface does not name, an injected api decides what the panel does, and an injected colour picker replaces the built-in one and receives each control's label. Three assertions in menu-widgets.test.ts read the box around a diagonal rather than the diagonal itself, because the happy-dom CSS parser drops a linear-gradient containing calc(). The drawn line stays covered by the e2e screenshots. Coverage rises to 84.42 statements, 73.54 branches, 83.51 functions and 87.32 lines, and the four thresholds move to one point below each. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 2 of plans/005-test-coverage-for-shipping.md. One data table lists all ninety-six controls the three pill menus render: the label the popup shows, the cell the case selects, the model the item must change, how many undo entries it must add, and the condition under which it renders disabled. Each item runs twice, against a top-level table and against a nested one, and the nested run also checks that the outer table comes through untouched. A completeness test compares the labels each popup renders with the labels described here, so an item added later fails until it is described. The two spacing sliders in the Table menu are it.fails: they write data-gap-x and data-gap-y with no history entry, so they cannot be undone while every other item can. table-size-buttons.test.ts gains the geometry the add previews and the affordances are placed by, and the clamp that keeps a menu opened near the window's edge inside it. Coverage rises to 87.23 statements, 75.51 branches, 87.71 functions and 90.36 lines, and the four thresholds move to one point below each. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The two spacing sliders in the Table menu wrote data-gap-x and data-gap-y straight into the table, so Ctrl+Z after moving one reverted an earlier operation and left the new spacing in place. A range input reports every step of a drag, and one history entry per step would be as bad as none. So the slider row now also reports the end of a gesture, and the spacing sliders write freely while the thumb moves and record a single entry when the pointer is released, whose snapshot is the table as it stood before the drag began. A drag that ends where it started records nothing. Playwright addressed the dev server as localhost, which resolves to ::1 first. Another checkout's dev server bound there answers 404 for the harness page, so the probe timed out and the suite never started. It names 127.0.0.1 now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four new files cover the panel section by section: each Row, Column and Cell control, and the border selector on its own. The panel reaches a table only through the injected TableApi, so a BloomTable subclass that notes each call and then does the real work shows which method ran and with which index. Two end-to-end checks only ever read a menu button's title, which happy-dom answers just as well. They move down beside the components, and the spec that held nothing else is gone. What is left in e2e is the dashed border a real browser has to compute. Two it.fails record a defect: the panel never disables Delete Row on a one-row table, or Delete Column on a one-column table, so the click reaches removeRowAt or removeColumnAt and throws. The pill menu guards both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Delete Row and Delete Column buttons stayed enabled on a table that had nothing left to give. The click reached removeRowAt or removeColumnAt, whose assertion threw out of the React handler. The pill menu has always disabled both commands in that state. The buttons carry the assertion's own wording as their tooltip, because the panel had no text of its own for it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ten new specs on the UI harness, each taking library objects from window.bloomTableTestHooks: drag-resize, cell-menu, pill-menus, save-round-trip, text-editing, scroll-container, host-injection, row-growth, container-narrowing and visual-samples. The pill-menus spec clicks every Row, Column and Table item where the browser draws it and checks one undo step each; the menu labels move to src/test-support/menu-item-labels.ts so the unit suite and the e2e suite read one list. The harness publishes removeTableEditingArtifacts, setContent and a stub-api call log, and mounts a stub table api and color picker under ?stubs=1. The visual-samples spec snapshots every sample and fixture through tests/samples/_harness.html at 1000x800. Two test.fixme cases record known gaps for plan 004: a nested table's rows do not grow with their text, and a table does not refit when its container narrows. resize-rows-and-columns.spec.ts and undo-resize-column.spec.ts are replaced by drag-resize.spec.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
tests/package/dist-smoke.test.ts imports the built bundle, attaches a table, adds a row and undoes it, checks the three stylesheets exist, checks React, React DOM and MUI stay external, and snapshots the 61 exported names from the type file. It runs under its own vitest config (vite.package.config.ts, `pnpm test:package`) so `pnpm test` does not need dist. CI builds and runs it after the unit tests. Coverage thresholds rise to 89/78/91/92 (statements, branches, functions, lines). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The workflow pinned Node 22.12 and the runner failed before the install step: pnpm 11.0.3 refuses Node earlier than 22.13. The engines field and the toolchain note now say the same. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
| Filename | Overview |
|---|---|
| src/table-size-buttons.ts | Adds history-backed spacing sliders, but the pre-operation snapshot retains the newly rendered inline gap and produces an incomplete visual undo. |
| src/menu-widgets.ts | Adds an optional range-input commit callback used to distinguish live updates from completed gestures. |
| src/components/RowSection.tsx | Correctly disables row deletion when the canonical table metadata reports only one row. |
| src/components/ColumnSection.tsx | Correctly disables column deletion when the canonical table metadata reports only one column. |
| .github/workflows/ci.yml | Adds broad CI execution, but omits the coverage command and uses mutable action tags. |
| playwright.config.ts | Improves local retry behavior and server addressing but disables all screenshot assertions in CI. |
| demo/ui-harness.tsx | Adds host-injection and persistence hooks, though its synchronous two-update reset cannot force an equal-content remount under React batching. |
| tests/package/dist-smoke.test.ts | Adds useful consumer-facing checks for bundle behavior, stylesheets, external dependencies, and exported API declarations. |
| vite.config.ts | Defines focused source coverage and explicit thresholds, which are effective when the coverage-specific command is run. |
| tests/e2e/visual-samples.spec.ts | Adds comprehensive fixture and sample screenshots, but their assertions are globally skipped by CI configuration. |
Reviews (1): Last reviewed commit: "Test the package as Bloom receives it (p..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 |
There was a problem hiding this comment.
The workflow executes checkout, pnpm setup, Node setup, and artifact upload through mutable major-version tags. If an upstream tag is repointed after a compromise, that code can run inside this repository’s CI with the workflow token’s configured access. Pin each action, including the third-party pnpm setup action, to a reviewed full commit SHA.
How this was verified: Each newly introduced uses entry references a mutable @v4 tag and the workflow provides no immutable commit reference.
…erage When a spacing drag ended, the slider put the data attribute back before the history entry captured the table, but not the inline gap the render had painted from the dragged value. The snapshot therefore kept the dragged spacing and Undo left the table visibly spaced. The slider now renders the pre-drag state before the entry captures it, and the unit test checks the painted style as well as the attribute. CI runs the unit tests with coverage, so the thresholds in vite.config.ts fail the run instead of only the local report. The harness's setContent hook remounts MainContent through a key, so markup equal to the current content still mounts fresh. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
hatton
commented
Sep 6, 2026
| // The screenshot baselines in *-snapshots/ are all "-chromium-win32". Linux | ||
| // renders different fonts, and no Linux baseline exists yet, so a CI run | ||
| // compares nothing rather than failing on every snapshot. | ||
| ignoreSnapshots: !!process.env.CI, |
Member
Author
There was a problem hiding this comment.
[Devin] Investigate: Linux visual regressions remain unchecked
CI disables every screenshot comparison through ignoreSnapshots. The new visual suite therefore provides no automated regression gate until Linux baselines are committed.
(playwright.config.ts:11. The baselines in the repo were taken on Windows; a Linux runner renders fonts differently, so the comparison is off on CI until Linux baselines exist.)
Member
Author
|
[Claude Fable 5.1 from Hatton's machine during preflight] Consulted Devin on 2026-09-05 up to commit e956d2f. Devin raised two bugs, both fixed in e956d2f (Undo of a spacing drag kept the painted gap; CI ran the unit tests without coverage so the thresholds never applied), and one Investigate flag about screenshot comparison being off on CI, which is mirrored as a review thread and waits for a decision. CI is green. |
With one local retry, a new sample or fixture wrote its baseline on the first attempt, failed, and passed on the retry. The run reported flaky and exited 0, so an unreviewed baseline landed green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
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
1 participant
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.
Problem
bloom-table is about to ship inside Bloom, and until now a release needed a manual pass through the editor. Twelve source modules had no unit tests, no CI ran any suite on push, and the Playwright specs covered only a handful of flows. A regression in a menu command, a drag handle, the saved HTML, or the published bundle could reach Bloom without any test noticing.
What the PR does
dist/bloom-table.mjs, edits a table through it, checks React and MUI stay external, and snapshots the exported names.test.fixmefor plan 004: a nested table's rows do not grow with their text, and a table does not refit when its container narrows.Plan:
plans/005-test-coverage-for-shipping.md.Devin review