Skip to content

test(e2e): stop main's static analysis failing on the tab strip test - #24052

Merged
brennanb2025 merged 1 commit into
mainfrom
brennanb2025/e2e-tab-strip-typed-global
Sep 30, 2026
Merged

brennanb2025 merged 1 commit into
mainfrom
brennanb2025/e2e-tab-strip-typed-global

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 30, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 1 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​9 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​5
Prod 0 0 0 0

ELI5

Since #24010 merged into main, every open PR fails CI's static analysis check, so their unit tests never run. The cause is one line in an end-to-end test that #24010 added. This PR rewrites that line the way the lint rule asks. No user-facing change: only a test file changes.

What Changed

tests/e2e/tab-strip-active-dock.spec.ts stored its scroll samples on window with Reflect.set and read them back with Reflect.get. The low-evidence lint (anti-slop(no-reflect-get), run by pnpm run audit:anti-slop) rejects Reflect.get at line 145.

The samples are now a typed Window field, declared with declare global as other e2e specs already do (for example github-url-smart-input-transition.spec.ts). The test writes and reads window.__tabStripDockSamples directly. The assertions are unchanged.

Why

Before: static analysis fails on main's merge ref for every PR (seen on #23898, #23681 and #23704), which also skips unit tests, relay and package, and fails verify.

After: the check passes again, and PRs get their full CI back.

A typed global is the pattern the codebase already uses for values that have to live on window between page.evaluate calls. It's also what the rule's message asks for. Silencing the rule would hide the untyped read it exists to catch.

Linked Issue

Follow-up to #24010 (no issue: main's CI regression).

Visual Proof

N/A: only an end-to-end test file changes. The app is untouched.

Testing

  • oxlint --config config/oxlint-anti-slop.json tests/e2e/tab-strip-active-dock.spec.ts --deny-warnings: fails on main's copy (145:7 anti-slop(no-reflect-get)), passes with this change.

  • oxlint and oxfmt --check on the file: clean.

  • tsc -p config/tsconfig.e2e.json: no errors in this file. The e2e project has unrelated type errors on main already, and it isn't a CI gate.

  • The e2e spec itself was not run locally; it isn't a required check.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

Agent skill upstream boundary

  • Not applicable, or this change follows docs/reference/agent-skill-sharing-upstream-boundary.md and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

…ield

#24010's spec read them with Reflect.get, which the low-evidence lint rejects, so every PR's static analysis now fails on main.
@coderabbitai

coderabbitai Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 581d3c8b-75de-4ae0-b210-150b0251c81b

📥 Commits

Reviewing files that changed from the base of the PR and between fceca5c and 2e84693.

📒 Files selected for processing (1)
  • tests/e2e/tab-strip-active-dock.spec.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Low risk] Fixes static analysis warnings in an end-to-end test.

The PR appears safe to merge.

Summary

Replaces reflective access to the tab-strip test’s scroll samples with a typed Window field so static analysis accepts the spec. The test assertions are unchanged.

Reviews (1) · Last reviewed commit: "test(e2e): read the tab strip's dock sam..."

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Typed Window field replaces Reflect.get/Reflect.set — tests/e2e/tab-strip-active-dock.spec.ts now declares window.__tabStripDockSamples through declare global, the same pattern the neighbouring e2e specs use, clearing the anti-slop(no-reflect-get) rule that was failing main's static analysis gate.

The rewrite is a mechanical lint-compliance change with no behavioral surface: the write and read sites are equivalent, the assertions are untouched, and the ?? [] fallback matches the prior undefined-when-unset behavior. The disable comment for consistent-type-definitions mirrors the existing five call sites, so it is not a new suppression style.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@brennanb2025
brennanb2025 merged commit 7afa4ee into main Sep 30, 2026
39 checks passed
brennanb2025 added a commit that referenced this pull request Sep 30, 2026
brennanb2025 added a commit that referenced this pull request Sep 30, 2026
Brings in main at 707d3dc, including #24052, which fixes main's
static-analysis failure in the tab strip e2e test. No conflicts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant