test(addon): add integration tests for background-restart and disable-mid-upload bugs - #1041
Merged
Merged
Conversation
rwood-moz
previously approved these changes
Jul 23, 2026
rwood-moz
left a comment
rwood-moz
left a comment
Contributor
There was a problem hiding this comment.
Nice! I pulled this branch down on top of your previous branch with the test harness (from #1033) and was able to run the tests successfully, and I see that the two new tests also ran successfully:
✓ src/test/integration/b5-background-restart-loses-bookkeeping.test.ts (1 test) 18ms
✓ src/test/integration/c1-disable-mid-upload-orphans-popup.test.ts (2 tests) 37ms
Test Files 13 passed | 1 skipped (14)
Tests 54 passed | 1 skipped (55)
Start at 16:57:13
Duration 1.09s (transform 922ms, setup 161ms, import 1.19s, tests 1.13s, environment 3.33s)
Code LGTM.
Adds the two High-priority (per ADDON-BUG-REPORTS-2026-07-22.md) specs on top of the fake Thunderbird host harness: - B5: background restart mid-upload loses uploadPromiseMap bookkeeping (background page recycling drops in-flight upload tracking with no persistence/rehydration path). - C1: no add-on disable/suspend lifecycle hook exists anywhere in this codebase, so a mid-upload disable orphans the popup with no cleanup signal. Both assert CONFIRMED BUG behavior against the real background.ts/ init.ts modules via the harness landed in the prior PR; they are expected to start failing (by design) once the corresponding fix lands, at which point they should be updated to assert the fixed behavior rather than deleted.
aaspinwall
force-pushed
the
test/addon-integration-high-priority-bugs
branch
from
July 23, 2026 21:29
8a7c0b5 to
bb2d6b8
Compare
… across contexts (#1042) * test(addon): add fake Thunderbird host harness for integration tests Introduces the Tier 1 integration-test harness used by the addon's sync/race-bug specs: - fakeThunderbirdHost.ts: simulates the three JS-only WebExtension execution contexts (background/popup/web), a shared browser.storage.local backing store with real storage.onChanged fan-out, and controllable browser.windows.create() resolution for deterministic check-then-act interleavings. - testHelpers.ts: shared setup/teardown glue (setupHost/teardownHost/ stubContext/createDeferred/ADDON_ROOT) so individual spec files can stay focused on the race they're proving. No specs land yet in this PR; harness-only so it can be reviewed in isolation before any bug-report assertions are argued over. * test(addon): add integration specs for High-priority sync bugs B5, C1 Adds the two High-priority (per ADDON-BUG-REPORTS-2026-07-22.md) specs on top of the fake Thunderbird host harness: - B5: background restart mid-upload loses uploadPromiseMap bookkeeping (background page recycling drops in-flight upload tracking with no persistence/rehydration path). - C1: no add-on disable/suspend lifecycle hook exists anywhere in this codebase, so a mid-upload disable orphans the popup with no cleanup signal. Both assert CONFIRMED BUG behavior against the real background.ts/ init.ts modules via the harness landed in the prior PR; they are expected to start failing (by design) once the corresponding fix lands, at which point they should be updated to assert the fixed behavior rather than deleted. * test(addon): add integration specs for A-series sync bugs (A1, A2, A5, A6, A7) Adds the five Medium-priority A-series specs (auth/logout sync races, per ADDON-BUG-REPORTS-2026-07-22.md) on top of the fake Thunderbird host harness: - A1: SIGN_OUT in a web tab doesn't abort or notify an in-flight popup upload. - A2: token-bridge origin mismatch means logout never reaches the add-on. - A5: menuLogout()'s blanket storage.local.clear() wipes unrelated in-flight data. - A6: no push-based refresh path exists for SIGN_OUT/OIDC_USER into the web UI. - A7: AccountHub add-on login races a concurrent web login for the same account. Each asserts CONFIRMED BUG behavior against the real production modules and is expected to start failing once the corresponding fix lands (see harness README's regression-test contract). * test(addon): add integration tests for upload popup dropping or mishandling attached files (#1043) * test(addon): add integration specs for B-series sync bugs (B1, B2, B3, B4) Adds the four remaining Medium-priority B-series specs (popup upload-queue/window races, per ADDON-BUG-REPORTS-2026-07-22.md) on top of the fake Thunderbird host harness and the already-landed B5: - B1: rapid onFileUpload bursts drop a second batch silently. - B2: popup close mid-upload aborts background bookkeeping but not the underlying network request. - B3: openUnifiedPopup()'s check-then-act race can open two popups. - B4: a file attached while the popup is already open is dropped, not merged into the existing upload. Each asserts CONFIRMED BUG behavior against the real production modules and is expected to start failing once the corresponding fix lands (see harness README's regression-test contract). * test(addon): add integration tests for stale-login-on-re-enable and the per-context init guard (#1044) * test(addon): add integration specs for C2, D3 sync bugs Adds the final two remaining Medium-priority specs (per ADDON-BUG-REPORTS-2026-07-22.md) on top of the fake Thunderbird host harness: - C2: add-on re-enable trusts stale STORAGE_KEY_AUTH without backend revalidation. - D3: init.ts's single-flight guard only coordinates within a single execution context, not across background/popup/web. Together with the already-landed A-series, B-series, B5, and C1 specs, this completes the 13 confirmed sync/race bugs covered by Tier 1 of the integration-test plan. Each asserts CONFIRMED BUG behavior against the real production modules and is expected to start failing once the corresponding fix lands (see harness README's regression-test contract). * docs(addon): add integration harness README and wire test:integration script (#1045) - Adds src/test/integration/README.md: explains the Tier 1 fake-host harness (what it simulates, what it deliberately stubs out, the regression-test contract for CONFIRMED BUG specs, and current coverage status/gaps). - Adds packages/addon/package.json's test:integration script (VITE_TESTING=true VITE_SEND_CLIENT_URL=... VITE_SEND_SERVER_URL=... vitest run src/test/integration --silent) so the harness can be run scoped, without needing the full addon .env. - Adds the two source-of-truth e2e docs the harness/specs trace back to: ADDON-BUG-REPORTS-2026-07-22.md (concise, spec-file-referenced bug list) and ADDON-INTEGRATION-TEST-ENV-PLAN-2026-07-22.md (the Tier 1/2/3 environment plan this harness implements Tier 1 of). No CI changes in this PR -- see PR description / reviewer notes for why the existing addon-changes job in .github/workflows/validate.yml already covers this (it's path-filtered on packages/addon/** and already runs `lerna run test --scope=addon`, i.e. `pnpm test`, which picks up every spec under src/test/integration/ automatically since vitest's default include glob is **/*.test.ts with no directory exclusion). The new test:integration script is a convenience for local/targeted runs; it does not need to be wired into CI separately. Co-authored-by: cagebot <cagebot@cagebots-Virtual-Machine.local> * fix(addon): cross-context lock for init.ts default-folder recreate (#1046) init.ts's single-flight guard (added for #930) is a module-scope variable, so it only dedups concurrent calls within one JS context. Background, popup, and any web-app tab each load their own copy of the module, so none of them can see another context's in-flight delete+recreate of the same account's default folder -- reintroducing #930's race, just moved across contexts instead of within one. Add a short-TTL lock in browser.storage.local (the one thing every context actually shares) around just the delete+recreate branch, keyed by account id. A context that can't acquire the lock re-syncs and trusts whatever the lock holder leaves behind instead of racing it. No-ops in any context without browser.storage.local (e.g. a plain web-app tab with no sibling context to race against), so behavior there is unchanged. Updates the D3 integration test (packages/addon) to actually exercise the fake host's shared browser.storage.local via two real fake-host contexts, replacing the version that only proved the bug existed. Adds a second spec asserting a racing context defers instead of running its own delete+recreate while another context holds the lock -- confirmed this fails against the pre-fix init.ts and passes with the fix. Fixes #1032 Co-authored-by: cagebot <cagebot@cagebots-Virtual-Machine.local> --------- Co-authored-by: cagebot <cagebot@cagebots-Virtual-Machine.local> --------- Co-authored-by: cagebot <cagebot@cagebots-Virtual-Machine.local> --------- Co-authored-by: cagebot <cagebot@cagebots-Virtual-Machine.local>
This was referenced Jul 28, 2026
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
2 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.
Depends on #1033 (needs the fake-host harness to be merged first).
Tracks #1036. Part of the overall test coverage effort in #1034.
What
Adds two integration tests on top of the fake-host harness, covering the two highest-priority add-on sync bugs found in the July bug hunt:
Both tests currently assert the buggy behavior, since neither bug has been fixed yet. Once someone fixes #1017 or #1018, its test should start failing until the fix is reflected in the test's expectations — that's intentional, it's how we'll know the fix actually worked.
Why
These were the two highest-priority add-on-client findings from the triage (both flagged as bugs a real user could actually hit, not just theoretical races), and neither had any test coverage before this.
Testing
Next steps
This is PR 2 of the stack. Next up: login/logout sync bugs (#1037), then the upload popup bugs (#1038), then the re-enable/init-guard bugs (#1039), then docs (#1040).