Skip to content

test: de-flake cart-drawer overflow spec via consent pre-seed#69

Merged
satra merged 1 commit into
mainfrom
026-deflake-cart-drawer-consent
Jul 1, 2026
Merged

test: de-flake cart-drawer overflow spec via consent pre-seed#69
satra merged 1 commit into
mainfrom
026-deflake-cart-drawer-consent

Conversation

@satra

@satra satra commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

De-flakes site/src/tests/e2e/cart_drawer_overflow.spec.ts. The test dismissed the first-visit analytics-consent banner reactively — clicking "decline" only if the banner happened to be visible at the moment it checked. On a fresh context the banner can render a beat after that check, stay up, overlay the cart drawer's footer buttons, and intercept pointer events, failing the test intermittently.

This pre-seeds the consent choice in localStorage via page.addInitScript before navigation, so the banner never renders. Deterministic, simpler, and faster.

Scope

Verification

  • Added lines pass prettier (the one remaining prettier warning on this file is a pre-existing long line I did not touch).
  • The behavioural check is the e2e run itself against the preview.

🤖 Generated with Claude Code

The cart-drawer footer test dismissed the first-visit analytics-consent
banner reactively (click "decline" if visible after load). On a fresh
context the banner could render a beat after the visibility check, stay
up, overlay the drawer footer, and intercept pointer events → flaky fail.

Pre-seed the consent choice in localStorage via addInitScript BEFORE
navigation so the banner never renders. Deterministic, simpler, faster;
scope is this one test — no product-code change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@satra
satra temporarily deployed to pr-preview-69 July 1, 2026 13:38 — with GitHub Actions Inactive
@satra satra added the deploy-production Trigger production deploy on merge label Jul 1, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the cart_drawer_overflow.spec.ts end-to-end test to pre-set the analytics consent choice to 'denied' in localStorage using page.addInitScript before navigating. This replaces the previous logic that attempted to dynamically find and click the decline button on the consent banner. The reviewer suggested also setting window.__ohbmAnalyticsConsent = 'denied' in the init script to ensure the banner is successfully blocked even in environments where localStorage is disabled or throws an error.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +29 to +35
await page.addInitScript(() => {
try {
localStorage.setItem('ohbm2026.analytics.consent.v1', 'denied');
} catch {
/* storage unavailable — banner will show; not this test's concern */
}
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

To make the consent suppression even more robust, we can also set window.__ohbmAnalyticsConsent = 'denied' directly in the init script.

Since ConsentBanner.svelte checks window.__ohbmAnalyticsConsent on mount and suppresses the banner if it is set to 'denied', doing this ensures the banner is successfully blocked even in test environments where localStorage is disabled or throws an error.

Suggested change
await page.addInitScript(() => {
try {
localStorage.setItem('ohbm2026.analytics.consent.v1', 'denied');
} catch {
/* storage unavailable — banner will show; not this test's concern */
}
});
await page.addInitScript(() => {
try {
localStorage.setItem('ohbm2026.analytics.consent.v1', 'denied');
} catch {
/* storage unavailable — banner will show; not this test's concern */
}
(window as any).__ohbmAnalyticsConsent = 'denied';
});

@satra

satra commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks @gemini-code-assist — I checked the bootstrap and am not taking this suggestion, because setting the global in addInitScript is a no-op here:

  • src/app.html's inline consent script runs on every load after Playwright's addInitScript, and unconditionally reassigns window.__ohbmAnalyticsConsent = state (app.html:96). It never reads a pre-existing global — it recomputes state from host / DNT / GPC / localStorage (:87-91). So a global set in the init script is overwritten before ConsentBanner mounts.
  • The correct lever is exactly the localStorage pre-seed this PR adds: the bootstrap reads ohbm2026.analytics.consent.v1 at app.html:88 and maps 'denied' → suppressed banner.
  • The localStorage-disabled edge doesn't benefit either — the global would still be clobbered by :96, and that path resolves via the bootstrap's own fallbacks, not the test.

(For transparency: on the CI preview host the path is /pr-<N>/…, which short-circuits to 'pr-preview' at app.html:74-76 and suppresses the banner regardless — so this change is a robustness fix for local-preview / production first-visit runs, not the CI flake. Keeping the diff minimal and localStorage-only.)

@gemini-code-assist

Copy link
Copy Markdown

Thanks for the detailed explanation, @satra. That makes perfect sense. I appreciate the clarification on how the app.html logic interacts with the addInitScript and why the localStorage pre-seed is the correct, robust approach for handling this in your test environment. Your reasoning for keeping the diff minimal and focused on the test's stability is sound.

@satra
satra merged commit 5ea0177 into main Jul 1, 2026
2 of 3 checks passed
@satra
satra deleted the 026-deflake-cart-drawer-consent branch July 1, 2026 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deploy-production Trigger production deploy on merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant