fix(test): address Copilot coverage test quality on #401 by ZappoMan · Pull Request #402 · Artificer-Innovations/BeakerStack

ZappoMan · 2026-06-04T00:02:51Z

Summary

Addresses all six Copilot review comments on the promotion PR #401. The coverage tests added in #400 exercised unmount/cancellation paths for coverage but lacked assertions that would catch regressions.

Changes

AdminDetailDrawer: Replace raw KeyboardEvent Tab dispatch with userEvent.tab() and assert focus advances from Notes → Save (verifies Tab is not trapped when focus is not on the last element).
useSignupMode: Spy on console.error and assert no unmounted-component warnings after resolving settings post-unmount.
BillingProvider: Add the same console.error unmount-warning assertions to four post-unmount async tests (plan query resolve/reject, RPC resolve/reject), using act() to flush updates.

Pattern matches ObservabilityProvider.web.test.tsx from #400.

Test plan

packages/admin — AdminDetailDrawer.web.test.tsx (17 tests)
packages/waitlist — useSignupMode.test.tsx (2 tests)
packages/billing — BillingProvider.test.tsx (24 tests)

Closes Copilot feedback on #401; merge this into develop before promoting to main.

Address Copilot review on #401: use userEvent.tab() to verify focus advances when Tab is not trapped, and assert no React unmounted warnings in post-unmount async tests for billing, waitlist, and admin packages.

github-actions · 2026-06-04T00:06:11Z

CI Coverage & Test Summary

Metric	Coverage	Covered / Total
Statements	99.42%	12578 / 12651
Branches	97.96%	5000 / 5104
Functions	97.58%	1291 / 1323
Lines	99.80%	12215 / 12239

Suites: 64 passed, 0 failed (64 total) · Tests: 666 passed, 0 failed (666 total)

✅ All reported test suites passed.

Coverage artifacts: coverage-summary, coverage-packages.

Updated at: June 3, 2026 at 5:14 PM PDT

Copilot

Pull request overview

Improves the quality of recently added “coverage-oriented” tests by turning them into regression-catching assertions, primarily around focus handling and post-unmount async behavior (ensuring no React “unmounted component” warnings).

Changes:

AdminDetailDrawer test now uses userEvent.tab() and asserts focus moves from “Notes” → “Save”.
useSignupMode test now spies on console.error to assert no unmounted-component warnings after resolving settings post-unmount.
BillingProvider tests add similar console.error assertions across multiple post-unmount async scenarios and use act() to flush microtasks.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File	Description
packages/admin/src/components/AdminDetailDrawer.web.test.tsx	Replaces manual Tab key dispatch with `userEvent.tab()` and adds a concrete focus assertion.
packages/waitlist/src/hooks/useSignupMode.test.tsx	Adds console warning assertions for late async resolution after unmount.
packages/billing/src/BillingProvider.test.tsx	Adds post-unmount console warning assertions to multiple async resolution/rejection paths, flushing with `act()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ZappoMan · 2026-06-04T00:11:39Z

+    const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) =>
+      String(message).toLowerCase().includes('unmounted')
+    );
+    expect(unmountedWarnings).toHaveLength(0);
+
+    consoleSpy.mockRestore();
    vi.restoreAllMocks();


Fixed in f7e2714.

Wrapped the spy + test body in try/finally so consoleSpy.mockRestore() runs even when the assertion throws — no spy leakage into later tests.

Added expectNoUnmountedConsoleWarnings() that scans every argument on each console.error call for "unmounted", not just the first arg.

Intent unchanged: we still fail the test if React logs an unmounted-component warning after resolving settings post-unmount. The assertion is stricter (all args) and the setup is safer (guaranteed cleanup).

ZappoMan · 2026-06-04T00:11:40Z

+    const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) =>
+      String(message).toLowerCase().includes('unmounted')
+    );
+    expect(unmountedWarnings).toHaveLength(0);
+    consoleSpy.mockRestore();


Fixed in f7e2714.

This test (plan query resolve after unmount) now uses try/finally around the spy so cleanup always runs.

expectNoUnmountedConsoleWarnings() checks all console.error arguments per call.

We still assert no React unmounted warnings after resolvePlan fires post-unmount — same regression guard from #401, just with leak-proof cleanup and broader argument matching.

ZappoMan · 2026-06-04T00:11:41Z

+    const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) =>
+      String(message).toLowerCase().includes('unmounted')
+    );
+    expect(unmountedWarnings).toHaveLength(0);
+    consoleSpy.mockRestore();


Fixed in f7e2714.

RPC resolve-after-unmount test: spy wrapped in try/finally.

Warning detection uses args.some(...) across the full argument list.

Still validates that a late ensure_billing_subscription resolution does not trigger setState-on-unmounted warnings — the postcondition is the same, only the harness is more reliable.

ZappoMan · 2026-06-04T00:11:41Z

+    const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) =>
+      String(message).toLowerCase().includes('unmounted')
+    );
+    expect(unmountedWarnings).toHaveLength(0);
+    consoleSpy.mockRestore();


Fixed in f7e2714.

Plan query rejection after unmount: try/finally ensures the spy is restored on failure.

All console.error args are scanned for unmounted warnings.

Intent preserved: a late plan rejection must not produce React unmounted warnings. We did not remove or soften the assertion — only made cleanup unconditional and matching more complete.

ZappoMan · 2026-06-04T00:11:43Z

+    const unmountedWarnings = consoleSpy.mock.calls.filter(([message]) =>
+      String(message).toLowerCase().includes('unmounted')
+    );
+    expect(unmountedWarnings).toHaveLength(0);
+    consoleSpy.mockRestore();


Fixed in f7e2714.

RPC rejection after unmount: same try/finally + expectNoUnmountedConsoleWarnings() pattern as the other three billing tests.

Still enforces that rejectRpc after unmount does not log unmounted-component warnings. Broader arg matching makes the check more likely to catch real regressions, not less.

Wrap console.error spies in try/finally so failed assertions cannot leak mocks into later tests, and scan all console.error arguments when detecting React unmounted-component warnings.

diego-artificerinnovations

All six Copilot comments addressed correctly. Approving.

AdminDetailDrawer Tab test ✓ — Replacing the raw KeyboardEvent dispatch with userEvent.tab() and asserting document.activeElement === save is the right fix. The old test only confirmed focus was on notes after a synthetic event the component may have silently ignored; the new test verifies Tab actually advances focus to the next element, which is the real invariant.

BillingProvider four unmount tests ✓ — The consoleSpy + expectNoUnmountedConsoleWarnings pattern is correct:

vi.spyOn(console, 'error').mockImplementation(() => {}) captures React's unmounted-component warnings without polluting test output.
act(async () => { await Promise.resolve(); }) correctly flushes pending microtasks so post-unmount state updates resolve before the assertion.
consoleSpy.mockRestore() in finally guarantees cleanup even on test failure.
Filter on 'unmounted' (case-insensitive) matches React's warning copy.

useSignupMode unmount test ✓ — Same pattern; bare await Promise.resolve() is fine for the renderHook path. Moving vi.restoreAllMocks() into finally is correct.

Pattern is consistent across all three files and matches the ObservabilityProvider precedent from #400.

fix(test): strengthen unmount and Tab-trap coverage assertions

199499b

Address Copilot review on #401: use userEvent.tab() to verify focus advances when Tab is not trapped, and assert no React unmounted warnings in post-unmount async tests for billing, waitlist, and admin packages.

ZappoMan requested a review from Copilot June 4, 2026 00:05

Copilot started reviewing on behalf of ZappoMan June 4, 2026 00:05 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

fix(test): harden console.error unmount warning assertions

f7e2714

Wrap console.error spies in try/finally so failed assertions cannot leak mocks into later tests, and scan all console.error arguments when detecting React unmounted-component warnings.

diego-artificerinnovations approved these changes Jun 4, 2026

View reviewed changes

ZappoMan merged commit 255860f into develop Jun 4, 2026
62 checks passed

diego-artificerinnovations mentioned this pull request Jun 4, 2026

release: promote develop to main (router security, coverage) #401

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(test): address Copilot coverage test quality on #401#402

fix(test): address Copilot coverage test quality on #401#402
ZappoMan merged 2 commits into
developfrom
fix/copilot-coverage-test-quality

ZappoMan commented Jun 4, 2026

Uh oh!

github-actions Bot commented Jun 4, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

ZappoMan Jun 4, 2026

Uh oh!

ZappoMan Jun 4, 2026

Uh oh!

ZappoMan Jun 4, 2026

Uh oh!

ZappoMan Jun 4, 2026

Uh oh!

ZappoMan Jun 4, 2026

Uh oh!

diego-artificerinnovations left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ZappoMan commented Jun 4, 2026

Summary

Changes

Test plan

Uh oh!

github-actions Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI Coverage & Test Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

ZappoMan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ZappoMan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ZappoMan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ZappoMan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

ZappoMan Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

diego-artificerinnovations left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jun 4, 2026 •

edited

Loading