Skip to content

Commit 74fb02b

Browse files
committed
fix: addresses PR review findings in E2E test infrastructure
- extracts duplicated setupAuth to shared e2e/helpers.ts - removes ghu_ prefix from fake tokens to avoid secret scanning - fixes waitFor({state:'detached'}) with scoped expand assertion - adds data-presence wait before screenshot capture - adds authenticated 404 redirect E2E test - updates README E2E count to 13
1 parent 9f5627a commit 74fb02b

File tree

6 files changed

+79
-120
lines changed

6 files changed

+79
-120
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ src/
118118
worker/
119119
index.ts # OAuth token exchange endpoint, CORS, security headers
120120
tests/ # 1522 unit/component tests across 69 test files
121-
e2e/ # 12 E2E tests across 2 spec files
121+
e2e/ # 13 E2E tests across 2 spec files
122122
```
123123

124124
## Development

docs/dashboard-screenshot.png

7.54 KB
Loading

e2e/capture-screenshot.spec.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,7 @@ test("capture dashboard screenshot", async ({ page }) => {
341341
await page.addInitScript(() => {
342342
// Clear any stale view state (e.g. notification drawer open from a previous run)
343343
localStorage.removeItem("github-tracker:view");
344-
localStorage.setItem("github-tracker:auth-token", "ghu_fake_screenshot_token");
344+
localStorage.setItem("github-tracker:auth-token", "fake-screenshot-token");
345345
localStorage.setItem(
346346
"github-tracker:config",
347347
JSON.stringify({
@@ -463,11 +463,14 @@ test("capture dashboard screenshot", async ({ page }) => {
463463
await page.getByRole("tab", { name: /pull requests/i }).click();
464464
await page.getByRole("tab", { name: /pull requests/i, selected: true }).waitFor();
465465

466-
// Expand the first collapsed repo group by clicking its header button
467-
const firstCollapsedGroup = page.getByRole("button", { expanded: false }).first();
468-
if (await firstCollapsedGroup.isVisible()) {
469-
await firstCollapsedGroup.click();
470-
await firstCollapsedGroup.waitFor({ state: "detached" }).catch(() => undefined);
466+
// Wait for repo group headers to render (visible even when collapsed)
467+
await page.getByText("acme-corp/web-platform").first().waitFor();
468+
469+
// Expand a repo group by clicking its header button (scoped to avoid notification bell)
470+
const repoGroupBtn = page.getByRole("button", { expanded: false }).filter({ hasText: "acme-corp/web-platform" });
471+
if (await repoGroupBtn.isVisible()) {
472+
await repoGroupBtn.click();
473+
await page.getByRole("button", { expanded: true }).filter({ hasText: "acme-corp/web-platform" }).waitFor();
471474
}
472475

473476
await page.screenshot({ path: "docs/dashboard-screenshot.png" });

e2e/helpers.ts

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import type { Page } from "@playwright/test";
2+
3+
/**
4+
* Register API route interceptors and inject auth + config into localStorage BEFORE navigation.
5+
* OAuth App uses permanent tokens stored in localStorage — no refresh endpoint needed.
6+
* The app calls validateToken() on load, which GETs /user to verify the token.
7+
*/
8+
export async function setupAuth(page: Page) {
9+
// Intercept /user validation (called by validateToken on page load)
10+
await page.route("https://api.github.com/user", (route) =>
11+
route.fulfill({
12+
status: 200,
13+
json: {
14+
login: "testuser",
15+
name: "Test User",
16+
avatar_url: "https://github.com/testuser.png",
17+
},
18+
})
19+
);
20+
await page.route(
21+
"https://api.github.com/repos/*/*/actions/runs*",
22+
(route) =>
23+
route.fulfill({
24+
status: 200,
25+
json: { total_count: 0, workflow_runs: [] },
26+
})
27+
);
28+
await page.route("https://api.github.com/notifications*", (route) =>
29+
route.fulfill({ status: 200, json: [] })
30+
);
31+
await page.route("https://api.github.com/graphql", (route) =>
32+
route.fulfill({
33+
status: 200,
34+
json: {
35+
data: {
36+
issues: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
37+
prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
38+
prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
39+
rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" },
40+
},
41+
},
42+
})
43+
);
44+
45+
// Seed localStorage with auth token and config before the page loads
46+
await page.addInitScript(() => {
47+
localStorage.setItem("github-tracker:auth-token", "fake-token");
48+
localStorage.setItem(
49+
"github-tracker:config",
50+
JSON.stringify({
51+
selectedOrgs: ["testorg"],
52+
selectedRepos: [{ owner: "testorg", name: "testrepo", fullName: "testorg/testrepo" }],
53+
onboardingComplete: true,
54+
})
55+
);
56+
});
57+
}

e2e/settings.spec.ts

Lines changed: 2 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,5 @@
1-
import { test, expect, type Page } from "@playwright/test";
2-
3-
/**
4-
* Register API route interceptors and inject auth + config into localStorage BEFORE navigation.
5-
* OAuth App uses permanent tokens stored in localStorage — no refresh endpoint needed.
6-
* The app calls validateToken() on load, which GETs /user to verify the token.
7-
*/
8-
async function setupAuth(page: Page) {
9-
await page.route("https://api.github.com/user", (route) =>
10-
route.fulfill({
11-
status: 200,
12-
json: {
13-
login: "testuser",
14-
name: "Test User",
15-
avatar_url: "https://github.com/testuser.png",
16-
},
17-
})
18-
);
19-
await page.route(
20-
"https://api.github.com/repos/*/*/actions/runs*",
21-
(route) =>
22-
route.fulfill({
23-
status: 200,
24-
json: { total_count: 0, workflow_runs: [] },
25-
})
26-
);
27-
await page.route("https://api.github.com/notifications*", (route) =>
28-
route.fulfill({ status: 200, json: [] })
29-
);
30-
await page.route("https://api.github.com/graphql", (route) =>
31-
route.fulfill({
32-
status: 200,
33-
json: {
34-
data: {
35-
issues: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
36-
prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
37-
prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
38-
rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" },
39-
},
40-
},
41-
})
42-
);
43-
44-
await page.addInitScript(() => {
45-
localStorage.setItem("github-tracker:auth-token", "ghu_fake");
46-
localStorage.setItem(
47-
"github-tracker:config",
48-
JSON.stringify({
49-
selectedOrgs: ["testorg"],
50-
selectedRepos: [{ owner: "testorg", name: "testrepo", fullName: "testorg/testrepo" }],
51-
onboardingComplete: true,
52-
})
53-
);
54-
});
55-
}
1+
import { test, expect } from "@playwright/test";
2+
import { setupAuth } from "./helpers";
563

574
// ── Settings page renders ────────────────────────────────────────────────────
585

e2e/smoke.spec.ts

Lines changed: 10 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -1,60 +1,5 @@
1-
import { test, expect, type Page } from "@playwright/test";
2-
3-
/**
4-
* Register API route interceptors and inject auth + config into localStorage BEFORE navigation.
5-
* OAuth App uses permanent tokens stored in localStorage — no refresh endpoint needed.
6-
* The app calls validateToken() on load, which GETs /user to verify the token.
7-
*/
8-
async function setupAuth(page: Page) {
9-
// Intercept /user validation (called by validateToken on page load)
10-
await page.route("https://api.github.com/user", (route) =>
11-
route.fulfill({
12-
status: 200,
13-
json: {
14-
login: "testuser",
15-
name: "Test User",
16-
avatar_url: "https://github.com/testuser.png",
17-
},
18-
})
19-
);
20-
await page.route(
21-
"https://api.github.com/repos/*/*/actions/runs*",
22-
(route) =>
23-
route.fulfill({
24-
status: 200,
25-
json: { total_count: 0, workflow_runs: [] },
26-
})
27-
);
28-
await page.route("https://api.github.com/notifications*", (route) =>
29-
route.fulfill({ status: 200, json: [] })
30-
);
31-
await page.route("https://api.github.com/graphql", (route) =>
32-
route.fulfill({
33-
status: 200,
34-
json: {
35-
data: {
36-
issues: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
37-
prInvolves: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
38-
prReviewReq: { issueCount: 0, pageInfo: { hasNextPage: false }, nodes: [] },
39-
rateLimit: { limit: 5000, remaining: 4999, resetAt: "2099-01-01T00:00:00Z" },
40-
},
41-
},
42-
})
43-
);
44-
45-
// Seed localStorage with auth token and config before the page loads
46-
await page.addInitScript(() => {
47-
localStorage.setItem("github-tracker:auth-token", "ghu_fake");
48-
localStorage.setItem(
49-
"github-tracker:config",
50-
JSON.stringify({
51-
selectedOrgs: ["testorg"],
52-
selectedRepos: [{ owner: "testorg", name: "testrepo", fullName: "testorg/testrepo" }],
53-
onboardingComplete: true,
54-
})
55-
);
56-
});
57-
}
1+
import { test, expect } from "@playwright/test";
2+
import { setupAuth } from "./helpers";
583

594
// ── Login page ───────────────────────────────────────────────────────────────
605

@@ -79,7 +24,7 @@ test("OAuth callback flow completes and redirects", async ({ page }) => {
7924
route.fulfill({
8025
status: 200,
8126
json: {
82-
access_token: "ghu_fake",
27+
access_token: "fake-token",
8328
token_type: "bearer",
8429
scope: "repo read:org notifications",
8530
},
@@ -217,3 +162,10 @@ test("unknown path redirects to login when unauthenticated", async ({ page }) =>
217162
// catch-all → Navigate "/" → RootRedirect → validateToken() fails → Navigate "/login"
218163
await expect(page).toHaveURL(/\/login/, { timeout: 10000 });
219164
});
165+
166+
test("unknown path redirects to dashboard when authenticated", async ({ page }) => {
167+
await setupAuth(page);
168+
await page.goto("/this-path-does-not-exist");
169+
// catch-all → Navigate "/" → RootRedirect → validateToken() succeeds → Navigate "/dashboard"
170+
await expect(page).toHaveURL(/\/dashboard/, { timeout: 10000 });
171+
});

0 commit comments

Comments
 (0)