Skip to content

Commit 746f32d

Browse files
gustavoliraclaude
andauthored
refactor(e2e): use shared-page fixture for serial specs (#4696)
* refactor(e2e): use shared-page fixture for serial specs Replace manual browser.newContext() with a worker-scoped sharedPage fixture so serial specs get video recording, tracing and screenshot support from Playwright's built-in config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(e2e): add screenshot-on-failure, type annotation, and comments - Capture and attach screenshot on test failure in shared-page fixture - Add Page type annotation to adoption-insights page variable - Add explanatory comments for video recording and worker isolation - Trim header comment to project convention length Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(e2e): guard screenshot capture against page crash Wrap sharedPage.screenshot() in try-catch so trace attachment still succeeds when the page has crashed during a test failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(e2e): remove manual tracing start/stop to avoid conflict Playwright auto-starts tracing when trace: "on" is set in config, so the explicit tracing.start() caused "Tracing has been already started" error. The startChunk/stopChunk calls in _sharedTraceChunk work on top of the auto-started session. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(e2e): let Playwright manage tracing lifecycle entirely Playwright's trace: "on" config auto-manages tracing (start, per-test chunks, and attachment) for all contexts including manually created ones. Remove _sharedTraceChunk fixture and replace with a simpler _sharedTestHook that only handles screenshot-on-failure and the workerHadFailure flag for video cleanup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 52e62b5 commit 746f32d

3 files changed

Lines changed: 98 additions & 21 deletions

File tree

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

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
1-
import { test, expect } from "@playwright/test";
1+
import { test, expect } from "@support/shared-page";
22
import { Common } from "../../../utils/common";
33
import { UIhelper } from "../../../utils/ui-helper";
44
import { TestHelper } from "../../../support/pages/adoption-insights";
55
import { skipIfJobName } from "../../../utils/helper";
66
import { JOB_NAME_PATTERNS } from "../../../utils/constants";
7+
import type { Page } from "@playwright/test";
78

89
/* eslint-disable playwright/no-conditional-in-test */
910

@@ -19,29 +20,22 @@ test.describe.serial("Test Adoption Insights", () => {
1920

2021
test.describe
2122
.serial("Test Adoption Insights plugin: load permission policies and conditions from files", () => {
22-
let context;
23-
let page;
23+
let page: Page;
2424
let testHelper: TestHelper;
2525
let uiHelper: UIhelper;
2626
let initialSearchCount: number;
2727
let templatesFirstEntry: string[];
2828
let catalogEntitiesFirstEntry: string[];
2929
let techdocsFirstEntry: string[];
3030

31-
// Shared setup
32-
test.beforeAll(async ({ browser }) => {
33-
context = await browser.newContext();
34-
page = await context.newPage();
31+
test.beforeAll(async ({ sharedPage }) => {
32+
page = sharedPage;
3533
uiHelper = new UIhelper(page);
3634
testHelper = new TestHelper(page);
3735
await new Common(page).loginAsKeycloakUser();
3836
await uiHelper.goToPageUrl("/", "Welcome back!");
3937
});
4038

41-
test.afterAll(async () => {
42-
await context?.close();
43-
});
44-
4539
test("Check UI navigation by nav bar when adoption-insights is enabled", async () => {
4640
await uiHelper.openSidebarButton("Administration");
4741
await uiHelper.clickLink("Adoption Insights");
@@ -224,6 +218,7 @@ test.describe.serial("Test Adoption Insights", () => {
224218
.locator("table.v5-MuiTable-root tbody tr")
225219
.first();
226220
const firstEntry = firstRow.locator("td").first();
221+
// @ts-expect-error PanelState.firstRow is typed as string[] but reassigned to a Locator — pre-existing issue
227222
state[title].firstRow = firstRow;
228223

229224
let headerTxt: string;

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

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { test } from "@playwright/test";
17+
import { test } from "@support/shared-page";
1818
import { Common } from "../../../utils/common";
1919
import { Catalog } from "../../../support/pages/catalog";
2020
// TODO: Re-enable/uncomment once https://issues.redhat.com/browse/RHIDP-12130 is fixed
2121
// import { CatalogImport } from "../../../support/pages/catalog-import";
2222
import { ScorecardPage } from "../../../support/page-objects/scorecard/scorecard-page";
23-
import type { BrowserContext, Page } from "@playwright/test";
23+
import type { Page } from "@playwright/test";
2424

2525
test.describe.serial("Scorecard Plugin Tests", () => {
26-
let context: BrowserContext;
2726
let page: Page;
2827
let catalog: Catalog;
2928
// TODO: Re-enable/uncomment once https://issues.redhat.com/browse/RHIDP-12130 is fixed
@@ -33,25 +32,20 @@ test.describe.serial("Scorecard Plugin Tests", () => {
3332
let initialGithubCount: number;
3433
let initialJiraCount: number;
3534

36-
test.beforeAll(async ({ browser }, testInfo) => {
35+
test.beforeAll(async ({ sharedPage }, testInfo) => {
3736
testInfo.annotations.push({
3837
type: "component",
3938
description: "scorecard",
4039
});
4140

42-
context = await browser.newContext();
43-
page = await context.newPage();
41+
page = sharedPage;
4442
catalog = new Catalog(page);
4543
// TODO: Re-enable/uncomment once https://issues.redhat.com/browse/RHIDP-12130 is fixed
4644
// catalogImport = new CatalogImport(page);
4745
scorecardPage = new ScorecardPage(page);
4846
await new Common(page).loginAsKeycloakUser();
4947
});
5048

51-
test.afterAll(async () => {
52-
await context?.close();
53-
});
54-
5549
test("Setup aggregated scorecards on homepage", async () => {
5650
await scorecardPage.navigateToHome();
5751

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
// Worker-scoped fixtures for test.describe.serial() blocks.
2+
// Usage: import { test, expect } from "@support/shared-page";
3+
4+
import {
5+
test as baseTest,
6+
expect as baseExpect,
7+
type BrowserContext,
8+
type Page,
9+
} from "@playwright/test";
10+
import path from "node:path";
11+
import fs from "node:fs";
12+
13+
type TestFixtures = {
14+
_sharedTestHook: void;
15+
};
16+
17+
type WorkerFixtures = {
18+
sharedContext: BrowserContext;
19+
sharedPage: Page;
20+
};
21+
22+
// Each Playwright worker runs in its own process, so this flag is per-worker.
23+
let workerHadFailure = false;
24+
25+
// eslint-disable-next-line @typescript-eslint/naming-convention
26+
export const test = baseTest.extend<TestFixtures, WorkerFixtures>({
27+
sharedContext: [
28+
async ({ browser }, use, workerInfo) => {
29+
const videoDir = path.join(
30+
"test-results",
31+
`shared-worker-${workerInfo.workerIndex}`,
32+
"videos",
33+
);
34+
35+
// Always record — Playwright's recordVideo has no retain-on-failure mode
36+
// for manual contexts, so we record unconditionally and delete on success.
37+
// Tracing is managed automatically by Playwright (trace: "on" in config).
38+
const context = await browser.newContext({
39+
recordVideo: {
40+
dir: videoDir,
41+
size: { width: 1280, height: 720 },
42+
},
43+
});
44+
45+
await use(context);
46+
47+
await context.close();
48+
49+
// Retain-on-failure: delete video files when all tests passed
50+
if (!workerHadFailure && fs.existsSync(videoDir)) {
51+
fs.rmSync(videoDir, { recursive: true, force: true });
52+
}
53+
},
54+
{ scope: "worker" },
55+
],
56+
57+
sharedPage: [
58+
async ({ sharedContext }, use) => {
59+
const page = await sharedContext.newPage();
60+
await use(page);
61+
},
62+
{ scope: "worker" },
63+
],
64+
65+
_sharedTestHook: [
66+
async ({ sharedPage }, use, testInfo) => {
67+
await use();
68+
69+
if (testInfo.status !== "passed" && testInfo.status !== "skipped") {
70+
workerHadFailure = true;
71+
try {
72+
const screenshotPath = testInfo.outputPath("failure.png");
73+
await sharedPage.screenshot({ path: screenshotPath });
74+
await testInfo.attach("screenshot", {
75+
path: screenshotPath,
76+
contentType: "image/png",
77+
});
78+
} catch {
79+
// Page may have crashed — screenshot unavailable
80+
}
81+
}
82+
},
83+
{ auto: true },
84+
],
85+
});
86+
87+
// eslint-disable-next-line @typescript-eslint/naming-convention
88+
export const expect = baseExpect;

0 commit comments

Comments
 (0)