refactor(e2e): use shared-page fixture for serial specs

[gustavolira]

Summary

• Introduced a worker-scoped sharedPage fixture (e2e-tests/playwright/support/shared-page.ts) that provides a persistent BrowserContext + Page with video recording, per-test tracing (startChunk/stopChunk), and retain-on-failure semantics
• Refactored adoption-insights.spec.ts and scorecard.spec.ts to use sharedPage instead of manual browser.newContext(), which was bypassing Playwright's built-in video/trace/screenshot config
• Added @support/* TypeScript path alias for cleaner imports from playwright/support/

Test plan

• npx tsc --noEmit passes (verified locally)
• yarn lint:check passes — no new warnings (verified locally)
• yarn prettier:check passes (verified locally)
• Run adoption-insights and scorecard specs against a deployed RHDH instance to verify traces and video artifacts are produced
• Verify trace files are attached per-test in the Playwright report
• Verify video files are retained on failure and cleaned up on success

🤖 Generated with Claude Code

[rhdh-qodo-merge]

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. Per-worker video only 🐞 Bug ◔ Observability

Description

Video is recorded into a worker-level directory while a single worker-scoped Page stays open across
all tests, producing one long video finalized at context close instead of per-test videos. This
makes it difficult to correlate a specific failure to the right video segment and won’t integrate
with Playwright’s per-test video artifacts.

Code

e2e-tests/playwright/support/shared-page.ts[R42-74]

+      const context = await browser.newContext({
+        recordVideo: {
+          dir: videoDir,
+          size: { width: 1280, height: 720 },
+        },
+      });
+
+      await context.tracing.start({
+        screenshots: true,
+        snapshots: true,
+        sources: false,
+      });
+
+      await use(context);
+
+      await context.tracing.stop();
+      await context.close();
+
+      // Retain-on-failure: delete video files when all tests passed
+      if (!workerHadFailure && fs.existsSync(videoDir)) {
+        fs.rmSync(videoDir, { recursive: true, force: true });
+      }
+    },
+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐⭐ Medium

Team values artifacts (traces/videos), but worker-scoped context/page reuse seems intentional;
per-test video may be acceptable.

PR-#4248
PR-#3893
PR-#4437

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The fixture enables recordVideo on the worker-scoped context and creates exactly one worker-scoped
page that is not closed per test. As a result, video output is produced for the lifetime of the
worker context, not aligned to individual tests. The repo’s Playwright config also documents that
the built-in per-test video behavior applies only when using the built-in { page } fixture.

e2e-tests/playwright/support/shared-page.ts[34-74]
e2e-tests/playwright.config.ts[69-84]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The current implementation records one continuous per-worker video because the page is worker-scoped and never closed between tests. This reduces the usefulness of videos for debugging individual failures.

### Issue Context
Playwright’s config is set up for per-test `video: { mode: 'retain-on-failure' }`, but that automatic behavior applies to the built-in `{ page }` fixture.

### Fix Focus Areas
- Consider making `sharedPage` test-scoped (new page per test) while keeping a shared authenticated context via `storageState`.
- Or manually create/close a per-test page and attach its video to `testInfo` (closing the page per test is typically required to finalize video files).

- e2e-tests/playwright/support/shared-page.ts[34-74]
- e2e-tests/playwright.config.ts[69-84]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Retries reuse dirty state 🐞 Bug ☼ Reliability

Description

Because sharedContext/sharedPage are worker-scoped, a retried test will run in the same
BrowserContext/Page state as the failed attempt, undermining Playwright retry isolation. With
retries enabled in the global Playwright config, this can cause false passes/fails and hard-to-debug
flakiness.

Code

e2e-tests/playwright/support/shared-page.ts[R65-74]

+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐ Low

Repo already reuses context/page via beforeAll in serial suites; retries already existed with same
dirty-state issue.

PR-#3893
PR-#3232
PR-#3860

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The sharedContext and sharedPage fixtures are explicitly worker-scoped, so they are created once per
worker and reused for all tests executed by that worker. The repo configuration enables retries on
CI, which means failed tests will be re-run without recreating those worker-scoped fixtures, so
browser state (cookies, localStorage, current URL, listeners, etc.) carries into the retry attempt.

e2e-tests/playwright/support/shared-page.ts[34-75]
e2e-tests/playwright.config.ts[53-61]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`sharedContext`/`sharedPage` are `scope: "worker"`, so Playwright retries re-run in the same context/page state as the failed attempt. This breaks retry isolation and can hide/mask flaky failures.

### Issue Context
The repo enables retries on CI (`retries: process.env.CI ? 2 : 0`). Worker-scoped fixtures do not get recreated for retries.

### Fix Focus Areas
Pick one of these approaches:
- Disable retries for suites using `@support/shared-page` (recommended if shared mutable state is intentional): add `test.describe.configure({ retries: 0 })` in the serial specs importing the shared fixture.
- Or change the fixture strategy to be retry-safe: make the context/page test-scoped (or re-created per test attempt) and preserve login via `storageState`/API login rather than a shared page.

- e2e-tests/playwright/support/shared-page.ts[34-75]
- e2e-tests/playwright.config.ts[53-61]
- e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts[1-36]
- e2e-tests/playwright/e2e/plugins/scorecard/scorecard.spec.ts[17-47]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Auth popup can hang 🐞 Bug ☼ Reliability

Description

With a shared worker-scoped BrowserContext, authentication state can leak across suites/files and
make subsequent login flows behave differently. Common.logintoKeycloak waits on a 'popup' event
without a timeout, so if the popup doesn't appear due to existing session state, the suite can hang
indefinitely.

Code

e2e-tests/playwright/support/shared-page.ts[R65-74]

+    { scope: "worker" },
+  ],
+
+  sharedPage: [
+    async ({ sharedContext }, use) => {
+      const page = await sharedContext.newPage();
+      await use(page);
+    },
+    { scope: "worker" },
+  ],

Relevance

⭐ Low

Speculative: Keycloak flow/popup behavior unclear; team previously rejected popup-flow robustness
suggestions, implying low appetite.

PR-#3797
PR-#4549
PR-#4315

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The new fixture keeps a single BrowserContext alive for the whole worker (scope: "worker"). The
Keycloak login helper resolves only when a popup event fires; if the session is already
authenticated (possible when reusing context across suites), the popup may not open and the Promise
never resolves.

e2e-tests/playwright/support/shared-page.ts[34-75]
e2e-tests/playwright/utils/common.ts[99-128]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Common.logintoKeycloak()` awaits a Promise that only resolves on a `page.once('popup')` event. If the popup never opens (e.g., because the shared worker context is already logged in), the test run can hang.

### Issue Context
The new `@support/shared-page` fixture is worker-scoped, so auth/session state can persist beyond a single suite.

### Fix Focus Areas
- Add a bounded timeout around waiting for the popup event and fail fast with a clear error.
- Or make `loginAsKeycloakUser()` detect already-authenticated state (e.g., sidebar already visible / sign-in button absent) and return early.
- Optionally, add an explicit logout/state reset in suites using `sharedPage`.

- e2e-tests/playwright/utils/common.ts[99-128]
- e2e-tests/playwright/support/shared-page.ts[34-75]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[rhdh-qodo-merge]

Review Summary by Qodo

Refactor e2e tests to use shared-page fixture with video and tracing

✨ Enhancement 🧪 Tests

Walkthroughs

Description

• Introduced worker-scoped sharedPage fixture for persistent browser context
• Integrated video recording, tracing, and screenshot support automatically
• Refactored adoption-insights and scorecard specs to use shared fixture
• Added @support/* TypeScript path alias for cleaner imports

Diagram

flowchart LR
  A["Manual browser.newContext()"] -->|Replace with| B["sharedPage fixture"]
  B -->|Provides| C["Video Recording"]
  B -->|Provides| D["Trace Chunks"]
  B -->|Provides| E["Screenshots"]
  C -->|Retain on| F["Test Failure"]
  D -->|Attach to| G["Playwright Report"]

Loading

File Changes

1. e2e-tests/playwright/support/shared-page.ts ✨ Enhancement +99/-0

New shared-page fixture with video and tracing

• Created new worker-scoped fixture providing persistent BrowserContext and Page
• Implemented video recording with 1280x720 resolution and automatic cleanup on success
• Added per-test tracing with startChunk/stopChunk and trace attachment to reports
• Tracks worker failures to retain video artifacts on test failures

e2e-tests/playwright/support/shared-page.ts

---

2. e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts Refactoring +3/-10

Migrate to shared-page fixture

• Changed import from @playwright/test to @support/shared-page
• Replaced manual browser.newContext() with sharedPage fixture parameter
• Removed manual context cleanup in test.afterAll()
• Removed unused context variable declaration

e2e-tests/playwright/e2e/plugins/adoption-insights/adoption-insights.spec.ts

---

3. e2e-tests/playwright/e2e/plugins/scorecard/scorecard.spec.ts Refactoring +4/-10

Migrate to shared-page fixture

• Changed import from @playwright/test to @support/shared-page
• Removed BrowserContext type import, kept only Page type
• Replaced manual browser.newContext() with sharedPage fixture parameter
• Removed manual context cleanup in test.afterAll() and context variable

e2e-tests/playwright/e2e/plugins/scorecard/scorecard.spec.ts

---

View more (1)

4. e2e-tests/tsconfig.json ⚙️ Configuration changes +5/-1

Add TypeScript path alias for support imports

• Added baseUrl compiler option set to current directory
• Added paths mapping for @support/* alias pointing to playwright/support/*

e2e-tests/tsconfig.json

---

[github-actions]

The container image build workflow finished with status: cancelled.

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4696
• quay.io/rhdh-community/rhdh:pr-4696-11c997b3

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4696
• quay.io/rhdh-community/rhdh:pr-4696-addfc8bb

[github-actions]

Image was built and published successfully. It is available at:

• quay.io/rhdh-community/rhdh:pr-4696
• quay.io/rhdh-community/rhdh:pr-4696-fdeec7ee

[github-actions]

The container image build and publish workflows were skipped (either due to [skip-build] tag or no relevant changes with existing image).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[openshift-ci]

@gustavolira: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-ocp-helm	b5281b2	link	true	/test e2e-ocp-helm

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.