INTER-2370: Deflake visitorId e2e

[erayaydin]

What

Rework the visitorId e2e test flow:

• runTest now reloads and retries the page up to MAX_PAGE_ATTEMPTS (3), polling both result blocks for RESULT_TIMEOUT_MS each load. It throws a clear error only after all attempts fail.
• The result poll now returns a boolean instead of throwing, so the loop decides when to reload.
• goto/reload use waitUntil: 'domcontentloaded' instead of 'networkidle'.

Why

The test was flaky and usually needed 4-5 reruns to pass.

Root cause: the test client page fetches the fingerprint result once per page load. When that single request hits a just-deployed Cloudflare worker route, the result element never populates, and the existing 30s poll can't recover it, since nothing on the page retries.

Reloading re-runs the agent, giving the warming worker another chance. networkidle is also ⚠️ ⚠️ discouraged by Playwright and unreliable here because the FPJS agent keeps the network busy; the poll already handles content readiness.

[github-actions]

Coverage report

St.❔	Category	Percentage	Covered / Total
🟢	Statements	99.63%	267/268
🟢	Branches	99.13%	114/115
🟢	Functions	100%	58/58
🟢	Lines	99.62%	262/263

Test suite run success

286 tests passing in 95 suites.

Report generated by 🧪jest coverage report action from 0499349

Show full coverage report

St	File	% Stmts	% Branch	% Funcs	% Lines	Uncovered Line #s
🟢	All files	99.62	99.13	100	99.61
🟢	src	100	100	100	100
🟢	config.ts	100	100	100	100
🟢	env.ts	100	100	100	100
🟢	handler.ts	100	100	100	100
🟢	index.ts	100	100	100	100
🟢	src/handlers	98.7	96.15	100	98.66
🟢	...ApiRequest.ts	96.42	87.5	100	96.15	79
🟢	...StatusPage.ts	100	100	100	100
🟢	src/utils	100	100	100	100
🟢	...ionHeaders.ts	100	100	100	100
🟢	...Monitoring.ts	100	100	100	100
🟢	cookie.ts	100	100	100	100
🟢	...orResponse.ts	100	100	100	100
🟢	...WithMaxAge.ts	100	100	100	100
🟢	...AgeIfLower.ts	100	100	100	100
🟢	proxyEndpoint.ts	100	100	100	100
🟢	...tpResponse.ts	100	100	100	100
🟢	routing.ts	100	100	100	100

[Copilot]

Pull request overview

This PR reworks the Playwright E2E visitorId flow to reduce flakiness by retrying page loads and switching navigation waits away from networkidle, addressing cases where a single per-load request can miss a just-deployed/cold worker route.

Changes:

• Add per-test CI retries and increase the Playwright per-test timeout to accommodate multiple reload/poll attempts.
• Refactor the visitorId E2E test to poll for both result blocks, reload up to MAX_PAGE_ATTEMPTS, and only fail after all attempts.
• Use waitUntil: 'domcontentloaded' for goto/reload rather than networkidle.

Reviewed changes

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

File	Description
playwright.config.ts	Increases per-test timeout and enables CI retries to reduce job-level flake from transient failures.
e2e/tests/visitorId.spec.ts	Implements multi-attempt reload + polling strategy and updates navigation wait strategy for reliability.

---

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

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

e2e/tests/visitorId.spec.ts:113

• If the first page.goto() attempt throws, the next loop iteration will call page.reload(), but the page may still be on about:blank (or another unexpected URL). That means subsequent attempts might never navigate to url at all. Track whether a successful navigation has happened and fall back to goto after any navigation failure.

    for (let attempt = 1; attempt <= MAX_PAGE_ATTEMPTS; attempt++) {
      console.log(`Running goto url (attempt ${attempt}/${MAX_PAGE_ATTEMPTS}): ${url}...`)
      try {
        // Navigation can itself fail, so retry it too rather than aborting the loop on the first error.
        if (attempt === 1) {
          await page.goto(url, { waitUntil: 'domcontentloaded' })
        } else {
          await page.reload({ waitUntil: 'domcontentloaded' })
        }

[github-actions]

⚠️ This PR doesn't contain any changesets. If there are user-facing changes, don't forget to run:

pnpm exec changeset

to create a changeset.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (3)

e2e/tests/visitorId.spec.ts:96

• waitForResults recreates the same locators on every poll iteration. Hoisting them once per call reduces repeated work and makes the polling loop easier to read.

  async function waitForResults(page: Page, timeout: number): Promise<boolean> {
    const deadline = Date.now() + timeout
    do {
      if (
        (await elementHasValidResult(page.locator('#result > code'))) &&
        (await elementHasValidResult(page.locator('#cdn-result > code')))
      ) {

e2e/tests/visitorId.spec.ts:87

• elementHasValidResult uses locator.textContent() without disabling Playwright’s built-in auto-wait. If the element detaches between isVisible() and textContent(), textContent() can wait up to the default timeout (often 30s), defeating the intended RESULT_TIMEOUT_MS budget and potentially reintroducing flakiness.

  async function elementHasValidResult(locator: Locator): Promise<boolean> {
    return (await locator.isVisible()) && hasValidResult((await locator.textContent()) ?? '')
  }

e2e/tests/visitorId.spec.ts:116

• PR description says runTest "reloads" between attempts, but the implementation always calls page.goto(url) (which is fine but not the same API). If the intent is to explicitly reload after the first successful navigation (and only goto when initial navigation failed), consider using page.reload() for subsequent attempts so the code matches the described behavior.

  async function runTest(page: Page, url: string) {
    for (let attempt = 1; attempt <= MAX_PAGE_ATTEMPTS; attempt++) {
      console.log(`Running goto url (attempt ${attempt}/${MAX_PAGE_ATTEMPTS}): ${url}...`)
      try {
        // Navigation can itself fail, so retry it too rather than aborting the loop on the first error.
        await page.goto(url, { waitUntil: 'domcontentloaded' })
      } catch (err) {
        console.log(`Navigation failed on attempt ${attempt}/${MAX_PAGE_ATTEMPTS}: ${String(err)}`)
        if (attempt === MAX_PAGE_ATTEMPTS) {
          throw err
        }
        continue
      }

[mcnulty-fp]

When that single request hits a just-deployed Cloudflare worker route, the result element never populates

I'm not sure that's the root cause. The test is already polling the /status endpoint to see if the worker is deployed and only proceeds with the actual test when the worker has been confirmed to be deployed.

If the page is not loading, it seems like it could be a different issue?

It could be useful to update the playwright test to upload the report to help us confirm that the page is not loading and what the browser is seeing when that happens. That'll help rule out a different root cause.