test: a testing environment that can reach the add-on's actual behaviour - #11
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The runner was configured with no DOM on purpose, so that the code-block pipeline could not quietly start depending on one. That has held, and it also meant four modules could not be tested at all, because the only way to reach any of them is through a document. Keep the constraint and make it local instead of global. Two named projects. `node` keeps the existing tests and keeps having no document, and it claims any test file dropped straight into `tests/` that no other tier has taken, so the strict tier is what an unfiled test gets rather than something to remember. `dom` runs jsdom over `tests/dom/`, which is where a test that legitimately needs a document goes. `pnpm test` runs both; `pnpm test:node` runs the pure tier alone for an edit loop. The boundary is the directory rather than a per-file environment pragma: a pragma would satisfy the letter of the config while losing the property it exists for, since the test would still pass and the line that handed the pipeline a document would be one line in a header nobody reads twice. `tests/thunderbird/` is already declined by the node tier so that the real-Thunderbird tier can arrive as a project of its own without reopening this. Coverage becomes available and is never gated. No threshold is configured and none is meant to be: this project leaves whole modules uncovered on purpose, so a number here would be one somebody tunes down until it agrees with whatever the last commit did. The vendored highlight.js is left out, since the pipeline's tests import it directly and it would otherwise bury this project's own numbers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tiers are a decision with a rejected alternative that will otherwise be suggested again, and one of them is unsupported by the platform it drives, so both belong somewhere durable rather than in a config comment. ADR-0001 records the three tiers, why the boundary lives in the directory layout rather than in per-file environment pragmas, why no coverage threshold will ever be set, and that the real-Thunderbird tier is undocumented and unsupported by Thunderbird with its breakage a cost this project owns. The README's developing section now names the tiers and the commands and points at it. This is the first ADR, so it also opens `docs/adr/`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Generalises the stub the settings tests wrote by hand. Every namespace a test lets the code under test touch is stubbed per test, and anything reached that was not stubbed throws and names the property that was asked for, so a new browser API call becomes a failing test rather than an undefined that something further down interprets. The ruled-out storage area keeps its old shape - present and throwing, naming the rule - and is now enforced for every test rather than for the settings ones alone, including against a test that tries to stub it. Built here rather than taken from a package: the maintained WebExtension mocks know Chrome and Firefox and none of them knows Thunderbird's namespaces. It lives under tests/helpers/ because the pure tier and the simulated-DOM tier are separate runner projects and both need it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The one property the release checklist cannot see: with two compose windows open, a snippet parked by a right-click in one never surfaces in the other. Covered in the pure tier through the fake browser, since the background registers listeners at module scope and exports nothing to call. The rules pinned are parking against the originating tab, clearing a stale park when the next right-click carries no selection, handing the selection over once, dropping it when the popup fails to open, keeping nothing across a wake of the event page, offering the menu in the compose body and nowhere else, swallowing the duplicate-id error a second creation produces by reading the last error, and declining a message the extension does not own rather than answering it. Each test wakes a fresh instance of the module scope, because a cached one carries the previous test's parked selections and every take-once assertion would then depend on the order the file runs in. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The release notes are the only changelog this project has, and a `refactor` produced a Changed entry in them. A refactor by definition changes nothing anyone using the add-on can observe, so that entry told a reader waiting to hear what the add-on now does about a file move instead. It joins the internal-only types; `perf` keeps Changed to itself, because a faster add-on is something a user experiences. A cycle whose every commit is internal now generates empty notes far more often than before, so the prose that claimed the release workflow refuses to publish on them is corrected rather than the behaviour: commit 3057179 removed that hard failure on purpose, and the README and the workflow comment had not caught up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both workflows pinned Node 22 while everyone working on this is on 24, so green locally said nothing about green in CI. The package declared no floor at all, which meant the only way to find out was a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
addons-linter is Mozilla's and knows Firefox, so the `compose` permission and every compose, composeAction, menus and scripting call read to it as an unsupported API. Its warning list was this add-on's whole reason for existing, which is why the warnings could never be made fatal and the README told you to skim them. Thunderbird's own webext-linter matches those calls against Thunderbird's annotated schemas and passes every one of them, so the exit code is worth something and CI now fails the build on it. It publishes no tags and is not on npm yet, so scripts/lint.sh pins the commit whose package.json reads 1.9.0 and fetches it into an ignored directory. It bootstraps with its own npm in there; pnpm stays this repo's package manager. CI caches the schema zips on a weekly-rotating key, because they come from branch heads and a permanent cache would freeze the schema train. Two checks are skipped and no more: update-url, which is the replacement for addons-linter's --self-hosted, and unused-files, which reads the extension of `vendor/highlight.js/LICENSE` as `.js/license` and so reports a licence that has to ship as dead weight. Before it was skipped, that check found cliff.toml riding along in the archive, which is now excluded from the package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The function handed to the compose sandbox already reports which of its three paths ran, precisely so the mechanism is observable from outside the sandbox, and nothing read that report until now. It is unchanged: self-contained by requirement, taking only its argument object, so the simulated-DOM tier can call it with no seam to build. With the editor command reporting failure, a caret inside the body splices the block in and leaves the caret after it - asserted by typing at the caret afterwards rather than by pinning a node and an offset - a body with no caret anywhere appends rather than appearing to do nothing, and content that turns out to produce an empty fragment does not throw while the caret is repositioned. A plain-text composer's source arrives as text on both DOM paths, with no markup parsed out of it and its indentation intact. One test asserts the only thing this tier can say about the preferred path: a command that answers yes is the end of it, and neither fallback touches the document. The gap is deliberate rather than incidental. No simulated DOM implements the editor command, so every test says out loud what the command answered instead of relying on its absence, and the claim that a real editor action joins the undo stack stays with the real-Thunderbird tier. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The page gets no entry point: the shipped options.html is loaded into the simulated-DOM tier and the module imported over it, which is how it runs when Thunderbird opens it, and loading the real file is what makes a renamed field fail here rather than in someone's settings page. What is pinned is only the correction notice, which is the part worth pinning: an out-of-range value is stored clamped, the field shows the clamped value, and the page says it adjusted rather than reporting a plain save; an emptied field is reported as a correction, and it is also the only shape a nonsense value can arrive in, since a number field hands over an empty string for one; and a leading zero typed over the same number is not reported, because that is the field's own formatting and not a correction anyone needs telling about. The bounds are read from the settings module and the confirmation wording off the page itself, so retuning either stays a one-line change. The settings tests said the options page was verified by hand because the runner had no DOM. The DOM was never the whole reason and it is no longer the situation, so that comment now says which tier decides the numbers and which one shows them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rule was two module variables in popup.js, read and written by four places that each knew one of its three rules, so the rule as a whole was only ever stated in a comment. It is now a closure that answers one question - what the pipeline should be told about the language - with the dropdown's value passed in, which is what lets the three rules be driven without a document at all. A closure and not a reducer: what the popup does with the answer is two DOM writes and a timer rather than data, so an effects vocabulary would be a second thing to keep correct for no gain. The precedent is snippet-size.js, pure for the same reason and stopping at the same edge. No behaviour changes. Which edits count as wholesale stays in the popup, where the event is, and so does the dropdown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The wiring was this module's side effects, so an import was the only way
in: it could not be run twice and it took its clock from whatever global
was around it. `startPopup({ document, setTimeout, clearTimeout })` now
takes both, each defaulting to the page's own, and the page's script is a
two-line main.js that makes the one call.
The debounce and the staleness guard stay exactly where they are. A
debounce extracted into a module is a module that tests a timer, and the
guard's property is about two renders overlapping, which can only be
asserted by making them overlap.
Otherwise nothing moved: the order of the dropdown, the settings and
theme reads, the load-time render, the prefill and the four listeners is
the order the page ran before. The dropdown's options are built through
the supplied document rather than the `Option` constructor, and the
popup is closed through the document's own window.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The type string the popup asks with and the background answers was a literal at both ends, and the background's own test had restated it as a third copy with a comment saying so. Both ends now import it, and so does that test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The latch's three rules go in the pure tier, one assertion each, where they need no document: an override outliving ten later pastes, one guess owed for a burst of them, and a caller that only asks leaving the guess standing. The wiring goes in the simulated tier, through `startPopup` against the shipped popup.html, the strict browser fake and a clock this file holds still. What it covers is the paths that actually broke: paste and confirm inside the debounce window inserting under the detected language rather than the last one displayed, the prefill announcing a change through the one entry point instead of replaying listeners, two renders overlapping where only the newest writes to the screen, a failed insert leaving confirmation usable with the error visible, and the size warning appearing and clearing from the same entry point. Each popup gets a document of its own in a frame, because the shortcut is bound to the document: sharing one would leave every earlier popup in the file listening, and confirming once would confirm as many times as there had been tests. One fixture was replaced during the work: a shell snippet detects as the language the dropdown happens to open on, so the overlap assertion passed for a source it had never rendered. The guard that would have caught that is now in the test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The theme reduction said it was untested because the runner has no DOM by design, which stopped being true when the simulated tier landed. The honest reason is narrower: the claim the module is built around is that the CSS parser expands shorthands into longhands while parsing, and that is precisely where a simulated object model is least faithful, so a passing test there would pin the fake's behaviour and call it the platform's. Two neighbouring comments said the popup is verified by hand for the same reason. It is not any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One command provisions the Thunderbird the manifest's floor promises, starts it headless on a profile that cannot update itself, temp-installs this checkout unsigned, opens a compose window and finds the add-on's button in the format toolbar. It needs nothing installed: the build and a matching geckodriver are fetched into an ignored directory and verified against published checksums. `pnpm test` now names its projects, so the default run stays offline and fast while the tier has a command of its own. Two things the research had wrong, both found by running it. geckodriver is pinned to 0.36.0 rather than anything newer because from 0.37.0 the driver hands the add-on to the application as base64, which Marionette only learned to read long after 128, so a temporary install against the pinned build answers with a bare InvalidArgumentError. And a compose window does need an identity, so the profile carries Mozilla's own prefs-only dummy account rather than creating one from chrome context: an account that exists before the first paint is also what keeps the account setup tab shut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The README gains what the tier needs, what the three environment variables do and the fact that Thunderbird supports none of it, so that whoever finds it broken after an update knows that was always the deal. The decision record's reason for keeping the tier out of the default command was wrong once the harness fetched its own build: the suite is green on a machine with no Thunderbird either way, and the real reasons are that the default run must not need the network or two minutes, and that an unsupported harness should never be why a change cannot be tested. It also now records the pile of external pins the tier turned out to own, the dummy account among them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tier could find the add-on's button; now it uses it. A snippet is typed into the popup and a block comes out in the message body - through the toolbar button, through the shortcut's key element, through a right-click carrying a selection, and into a plain-text composer - with the insertion function's own report of which path it took read back off the console, so "through the editor command rather than a fallback" is asserted rather than assumed. One Ctrl+Z takes the block out again, which is the editor's side of the same claim. The popup's document is still unreachable. What turned out to be reachable is the popup itself: focusing its browser element hands it the keyboard, so it can be typed into and confirmed from outside while the message body says what happened. A focus failure would otherwise read as a working insert, so every test reads the body once before confirming and expects it untouched - broken on purpose once to watch all eight fail with that message. Two things these tests cannot have, both said where they bite: the key press itself, which synthesised input cannot deliver to a letter-key shortcut, and the popup's textarea, which nothing can read. So the shortcut is driven at the key element Thunderbird built from the manifest, and the prefill is observed through what it inserts. And one finding: the popup cannot be opened in a plain-text composer at all, because Thunderbird hides the toolbar the button sits in and the popup is anchored to that button. That is issue #12. The insert itself is correct, which is what the test asserts, with the toolbar unhidden for the length of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… needs eyes Thirty-one items done by hand become nineteen. The file now opens with three commands and a list of what running them stands in for, so a failure in the tier is recognisable as a checklist item failing rather than as a test being fussy; what follows is what no driver can claim - the button on a dark appearance, the icon rather than a puzzle piece, where cloud attachment links land, the absence of spell-check underlines, the archive installed into a clean profile, and the key press behind Ctrl+Shift+C. Nothing was deleted that nothing verifies. The two items superseded by unit coverage rather than by the tier, the large-snippet threshold and the live preview, are listed with the tests that own them; the shortcut stays, because what is unverified there is exactly the delivery. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`tests/thunderbird/pins.test.js` said out loud that its assertions "would pass" in the node tier and kept them in the Thunderbird tier by topic. That is the one thing ADR-0001 rules out: which tier a test belongs in is answered by where it can be written, and nothing else. The cost was not theoretical. `pnpm test` and CI run the node and dom tiers only, so the guard that fails when `manifest.json`'s floor outgrows the pinned build, and the whole of the `THUNDERBIRD_BINARY` resolution, ran in no pipeline at all - raising the floor would have broken the harness with everything green. Six of the seven assertions need no Thunderbird and move to `tests/node/thunderbird-harness.test.js`. The seventh needs the download, so it stays in the tier as `tests/thunderbird/provision.test.js` and says why. The floor guard also grew a real comparison: it claimed "at or above the exact version promised" while comparing majors, so a floor of `128.15` - a build that does not exist - would have passed it. While in there, two things the tier's tests were getting from the wrong place. `harness/index.js` is documented as the harness's interface and was imported by nobody; every test file in the tier now goes through it, and the names no test asked for have come out of it. And `THUNDERBIRD_ENV` is exported from `provision.js`, which owns it, rather than re-declared in the test. The two literals that cannot be read from the module that owns them are the insertion function's mechanism strings, because that module is handed to the compose sandbox and has to stay self-contained. Those stay written out, with the reason next to them rather than left to look like an oversight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Everything removed here was read by nothing. `ComposeWindow.format` was written and never looked at, and the only caller that had to invent a value for it was `composeWindows()`, which no test ever called. `Session` copied the button and toolbar ids onto itself alongside the resolved binary and the driver path, and all four were write-only: the tests that want the ids import them. `closeActionPopup()` had no callers either - a test that wants the popup gone closes the window, which is what every one of them does. The two provisioning functions were the same sequence twice, differing in the compression flag: fetch once, verify against a published digest, extract into a directory that is only moved into place when the extraction finished. That sequence now lives in one place, which also gives geckodriver the extract-and-rename the Thunderbird half already had. The digest arrives as a function rather than a value because resolving Thunderbird's costs a request, and a warm cache must ask nothing of the network. The geckodriver archive's name moves to `pins.js`, which claims to hold everything this tier pins: built inline where it was downloaded, the platform and the compression suffix were two claims about the outside world sitting where nobody would look for them. `provision.js` also called its logging function `describe`, which is what `session.js` calls the thing it is waiting for. One name for two ideas across two files of the same harness; the logger is now `report`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four claims that had drifted, and one section that was filed under the wrong heading. The release checklist's Updates section asserted the rule the split was supposed to satisfy - every item left is a claim about Thunderbird - while holding three claims about GitHub and this repo's own release workflow. It is now "After publishing", which says what those items are and why they survive: not because a test could not make them, but because there is nothing to make them against until the workflow has published something. The items about a real Thunderbird checking for and installing an update move out into a section of their own, where they are what the rest of the file is. One item is retired rather than reframed. The "Update manifest did not contain an entry for …" line in the Error Console is the sole symptom of a mismatch between the update manifest's key and the id this add-on declares, and `tests/node/updates.test.js` pins exactly that; the item above it already fails if a check does not upgrade. What is left of the `updates.json` item is the part nothing verifies - whether the published release actually carries the asset - because the file's contents are pinned and CI checks that it builds, but neither can see a release. The README said two checks are skipped when the lint script also turns off the CDN library lookup, said the checklist starts with two commands when it lists three, and left `test:watch` out of the command list. And `.gitignore` pointed at "issue 12" for the packaged artifacts, which predates all of this work and now names an unrelated bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Schleuse
marked this pull request as ready for review
September 9, 2026 10:16
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.
Implements the spec in #10 across all seven of its tickets, on one branch.
Three named test tiers, with the boundary in the directory layout rather than in per-file pragmas:
Alongside the tiers: the popup's override rule comes out into its own module, the Firefox-oriented linter is replaced by one that knows Thunderbird and CI fails on its findings, and
refactorstops reaching the release notes.Closes #10
Closes #3
Closes #4
Closes #5
Closes #6
Closes #7
Closes #8
Closes #9
🤖 Generated with Claude Code