Skip to content

Commit 1f59649

Browse files
authored
Move to loginRedirect from loginPopup, and update e2e tests (#293)
* Move to loginRedirect from loginPopup, and update e2e tests * Use e2e test sharding * Fix comments * Cleanup actions * Install playwright without deps * Use yarn cache for merge reports * Install playwright with deps
1 parent 4c8d1b6 commit 1f59649

19 files changed

+270
-100
lines changed

.github/workflows/deploy.yml

Lines changed: 57 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
- uses: actions/checkout@v4
2424
- uses: actions/setup-node@v4
2525
with:
26-
node-version: 24.x
26+
node-version: lts/*
2727
cache: 'yarn'
2828

2929
- name: Restore Yarn Cache
@@ -92,17 +92,23 @@ jobs:
9292
edit-mode: 'replace'
9393

9494
e2e:
95+
name: Run Playwright tests
9596
if: github.event_name == 'pull_request'
9697
needs: deploy
9798
runs-on: ubuntu-latest
9899
permissions:
99100
contents: read
100-
timeout-minutes: 10
101+
strategy:
102+
fail-fast: false
103+
matrix:
104+
shardIndex: [1, 2, 3, 4]
105+
shardTotal: [4]
106+
timeout-minutes: 20
101107
steps:
102108
- uses: actions/checkout@v4
103109
- uses: actions/setup-node@v4
104110
with:
105-
node-version: 24.x
111+
node-version: lts/*
106112
cache: 'yarn'
107113
- name: Validate deployment URL
108114
run: |
@@ -119,18 +125,21 @@ jobs:
119125
yarn-modules-${{ runner.arch }}-${{ runner.os }}-
120126
- name: Install dependencies
121127
run: yarn -D
122-
- run: npx playwright install --with-deps
123-
- run: yarn test:e2e
128+
- name: Install Playwright dependencies
129+
run: npx playwright install --with-deps
130+
- run: yarn test:e2e --shard=${{ matrix.shardIndex }}/${{ matrix.shardTotal }}
124131
env:
125132
PLAYWRIGHT_BASE_URL: ${{ needs.deploy.outputs.deployment-url }}
126-
- uses: actions/upload-artifact@v4
133+
- name: Upload blob report to GitHub Actions Artifacts
127134
if: ${{ !cancelled() }}
135+
uses: actions/upload-artifact@v4
128136
with:
129-
name: playwright-report
130-
path: playwright-report/
131-
retention-days: 7
137+
name: blob-report-${{ matrix.shardIndex }}
138+
path: blob-report
139+
retention-days: 1
132140

133141
check_formatting:
142+
name: Check code formatting
134143
if: github.event_name == 'pull_request'
135144
runs-on: ubuntu-latest
136145
permissions:
@@ -140,7 +149,7 @@ jobs:
140149
- uses: actions/checkout@v4
141150
- uses: actions/setup-node@v4
142151
with:
143-
node-version: 24.x
152+
node-version: lts/*
144153
cache: 'yarn'
145154
- name: Restore Yarn Cache
146155
uses: actions/cache@v4
@@ -151,3 +160,41 @@ jobs:
151160
yarn-modules-${{ runner.arch }}-${{ runner.os }}-
152161
- run: yarn -D
153162
- run: yarn format:check
163+
164+
merge-reports:
165+
name: Merge Playwright Reports
166+
if: ${{ !cancelled() }}
167+
needs: [e2e]
168+
runs-on: ubuntu-latest
169+
permissions:
170+
contents: read
171+
steps:
172+
- uses: actions/checkout@v5
173+
- uses: actions/setup-node@v5
174+
with:
175+
node-version: lts/*
176+
- name: Restore Yarn Cache
177+
uses: actions/cache@v4
178+
with:
179+
path: node_modules
180+
key: yarn-modules-${{ runner.arch }}-${{ runner.os }}-${{ hashFiles('**/yarn.lock') }}-dev
181+
restore-keys: |
182+
yarn-modules-${{ runner.arch }}-${{ runner.os }}-
183+
- name: Install dependencies
184+
run: yarn -D
185+
- name: Download blob reports from GitHub Actions Artifacts
186+
uses: actions/download-artifact@v5
187+
with:
188+
path: all-blob-reports
189+
pattern: blob-report-*
190+
merge-multiple: true
191+
192+
- name: Merge into HTML Report
193+
run: npx playwright merge-reports --reporter html ./all-blob-reports
194+
195+
- name: Upload HTML report
196+
uses: actions/upload-artifact@v4
197+
with:
198+
name: html-report--attempt-${{ github.run_attempt }}
199+
path: playwright-report
200+
retention-days: 14

.github/workflows/test.yml

Lines changed: 0 additions & 34 deletions
This file was deleted.

e2e/home.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,4 +29,10 @@ test.describe('Homepage', () => {
2929
await expect(page.locator(`img[src*="${sponsor}"]`)).toBeVisible();
3030
}
3131
});
32+
test('Copyright section is visible', async ({ page }) => {
33+
const currentYear = new Date().getFullYear();
34+
await expect(
35+
page.getByText(${currentYear} ACM @ UIUC. All rights reserved.`)
36+
).toBeVisible();
37+
});
3238
});

e2e/identitySync.spec.ts

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
test.describe('Identity Sync', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/admin/sync');
6+
});
7+
8+
test('Page renders with heading', async ({ page }) => {
9+
const heading = page.locator('h1');
10+
await expect(heading).toContainText('Sync Identity');
11+
const desc = page.locator('p').first();
12+
await expect(desc).toContainText(
13+
'Use this tool to setup your account, or update your ACM account with details from your Illinois NetID.'
14+
);
15+
});
16+
test('Page provides sync button which leads to login redirect', async ({
17+
page,
18+
}) => {
19+
await expect(
20+
page.getByRole('button', { name: 'Sync Identity' })
21+
).toBeVisible();
22+
await page.getByRole('button', { name: 'Sync Identity' }).click();
23+
await page.waitForURL(
24+
'https://login.microsoftonline.com/44467e6f-462c-4ea2-823f-7800de5434e3/oauth2/v2.0/authorize?client_id=d64e9c50-d144-4b4a-a315-ad2ed456c37**'
25+
);
26+
});
27+
test('Copyright section is visible', async ({ page }) => {
28+
const currentYear = new Date().getFullYear();
29+
await expect(
30+
page.getByText(${currentYear} ACM @ UIUC. All rights reserved.`)
31+
).toBeVisible();
32+
});
33+
});

e2e/membership.spec.ts

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
test.describe('Membership Purchase', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('/membership');
6+
});
7+
8+
test('Page renders with heading and perks', async ({ page }) => {
9+
const heading = page.locator('h1');
10+
await expect(heading).toContainText('Become a Paid Member');
11+
12+
const perksList = page.getByTestId('membership-perks');
13+
await expect(perksList).toBeVisible();
14+
const listItems = perksList.locator('li');
15+
expect((await listItems.all()).length).toBeGreaterThan(0);
16+
});
17+
test('Page provides purchase button which leads to login redirect', async ({
18+
page,
19+
}) => {
20+
await expect(
21+
page.getByRole('button', { name: 'Purchase Membership' })
22+
).toBeVisible();
23+
await page.getByRole('button', { name: 'Purchase Membership' }).click();
24+
await page.waitForURL(
25+
'https://login.microsoftonline.com/44467e6f-462c-4ea2-823f-7800de5434e3/oauth2/v2.0/authorize?client_id=d64e9c50-d144-4b4a-a315-ad2ed456c37**'
26+
);
27+
});
28+
});
29+
30+
test.describe('Membership Check', () => {
31+
test.beforeEach(async ({ page }) => {
32+
await page.goto('/membership/check');
33+
});
34+
35+
test('Page renders with heading', async ({ page }) => {
36+
const heading = page.locator('h1');
37+
await expect(heading).toContainText('Check Paid Membership');
38+
});
39+
test('Page provides check button which leads to login redirect', async ({
40+
page,
41+
}) => {
42+
await expect(
43+
page.getByRole('button', { name: 'Check Membership' })
44+
).toBeVisible();
45+
await page.getByRole('button', { name: 'Check Membership' }).click();
46+
await page.waitForURL(
47+
'https://login.microsoftonline.com/44467e6f-462c-4ea2-823f-7800de5434e3/oauth2/v2.0/authorize?client_id=d64e9c50-d144-4b4a-a315-ad2ed456c37**'
48+
);
49+
});
50+
test('Copyright section is visible', async ({ page }) => {
51+
const currentYear = new Date().getFullYear();
52+
await expect(
53+
page.getByText(${currentYear} ACM @ UIUC. All rights reserved.`)
54+
).toBeVisible();
55+
});
56+
});

e2e/navigation.spec.ts

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
import { test, expect } from '@playwright/test';
22

3+
const internalPages = [
4+
{ href: '/about', heading: /About/i },
5+
{ href: '/calendar', heading: /Calendar/i },
6+
{ href: '/store', heading: /All Products/i },
7+
];
8+
39
test.describe('Navigation', () => {
410
test.beforeEach(async ({ page }) => {
511
await page.goto('/');
@@ -24,21 +30,16 @@ test.describe('Navigation', () => {
2430
await expect(page.locator('nav a[href*="illinoiscs.wiki"]')).toBeVisible();
2531
});
2632

27-
test('internal nav links navigate to correct pages', async ({ page }) => {
28-
const pages = [
29-
{ href: '/about', heading: /About/i },
30-
{ href: '/calendar', heading: /Calendar/i },
31-
];
32-
33-
for (const p of pages) {
33+
for (const p of internalPages) {
34+
test(`Internal nav link navigates to ${p.href}`, async ({ page }) => {
3435
await page.goto('/');
3536
await page.locator(`nav a[href="${p.href}"]`).click();
3637
await expect(page).toHaveURL(new RegExp(p.href));
3738
await expect(
3839
page.getByRole('heading', { name: p.heading }).first()
3940
).toBeVisible();
40-
}
41-
});
41+
});
42+
}
4243

4344
test('Join Now button opens Join modal', async ({ page }) => {
4445
await page.getByRole('button', { name: /Join Now/i }).click();

e2e/resources.spec.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
test.describe('Resources', () => {
4+
test.beforeEach(async ({ page }) => {
5+
await page.goto('resources/');
6+
});
7+
8+
test('Heading renders with description', async ({ page }) => {
9+
const heading = page.getByTestId('content-title');
10+
const description = page.getByTestId('content-description');
11+
await expect(heading).toContainText('Resources');
12+
await expect(description).toContainText(
13+
'Learn about resources provided to ACM @ UIUC members'
14+
);
15+
});
16+
17+
test('Copyright section is visible', async ({ page }) => {
18+
const currentYear = new Date().getFullYear();
19+
await expect(
20+
page.getByText(${currentYear} ACM @ UIUC. All rights reserved.`)
21+
).toBeVisible();
22+
});
23+
});

playwright.config.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export default defineConfig({
66
forbidOnly: !!process.env.CI,
77
retries: process.env.CI ? 2 : 0,
88
workers: process.env.CI ? 1 : undefined,
9-
reporter: process.env.CI ? 'html' : 'list',
9+
reporter: process.env.CI ? 'blob' : 'list',
1010
timeout: 30_000,
1111
use: {
1212
baseURL: process.env.PLAYWRIGHT_BASE_URL ?? 'http://localhost:3000',

src/authConfig.ts

Lines changed: 9 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
import {
2-
BrowserAuthError,
32
createStandardPublicClientApplication,
43
InteractionRequiredAuthError,
54
type IPublicClientApplication,
6-
type PopupRequest,
5+
type RedirectRequest,
76
} from '@azure/msal-browser';
87

98
export const loginRequest = {
@@ -27,54 +26,29 @@ export const initMsalClient = async () => {
2726
};
2827

2928
export const getUserAccessToken = async (
30-
pca: IPublicClientApplication
29+
pca: IPublicClientApplication,
30+
returnPath?: string
3131
): Promise<string> => {
3232
const account = pca.getActiveAccount() || pca.getAllAccounts()[0];
33-
const request: PopupRequest = {
33+
const request: RedirectRequest = {
3434
...loginRequest,
3535
account: account || undefined,
3636
redirectUri: '/auth-redirect',
37+
state: returnPath ?? window.location.pathname + window.location.search,
3738
};
3839

3940
if (!account) {
40-
try {
41-
const response = await pca.loginPopup(request);
42-
pca.setActiveAccount(response.account);
43-
return response.accessToken;
44-
} catch (error) {
45-
if (
46-
error instanceof BrowserAuthError &&
47-
error.errorCode === 'popup_window_error'
48-
) {
49-
alert(
50-
'Your browser is blocking popups. Please allow popups for this site and then try logging in again.'
51-
);
52-
}
53-
console.error('MSAL login failed:', error);
54-
throw error;
55-
}
41+
await pca.loginRedirect(request);
42+
throw new Error('Redirecting to login');
5643
}
5744

5845
try {
5946
const response = await pca.acquireTokenSilent(request);
6047
return response.accessToken;
6148
} catch (e) {
6249
if (e instanceof InteractionRequiredAuthError) {
63-
try {
64-
const response = await pca.acquireTokenPopup(request);
65-
return response.accessToken;
66-
} catch (popupError) {
67-
if (
68-
popupError instanceof BrowserAuthError &&
69-
popupError.errorCode === 'popup_window_error'
70-
) {
71-
alert(
72-
'Your browser is blocking popups, which are required for this action. Please allow popups for this site and try again.'
73-
);
74-
}
75-
console.error('MSAL popup token acquisition failed:', popupError);
76-
throw popupError;
77-
}
50+
await pca.acquireTokenRedirect(request);
51+
throw new Error('Redirecting to login', { cause: e });
7852
}
7953
throw e;
8054
}

0 commit comments

Comments
 (0)