feat(express-sample): parallelize E2E tests and pre-cache CDN bundles#8612
Open
hectormmg wants to merge 6 commits into
Open
feat(express-sample): parallelize E2E tests and pre-cache CDN bundles#8612hectormmg wants to merge 6 commits into
hectormmg wants to merge 6 commits into
Conversation
- Split upgrade-downgrade.spec.ts into schema.spec.ts, upgrade.spec.ts, and downgrade.spec.ts to enable parallel CI job splitting - Update 3p-e2e.yml: replace single ExpressSample job with 3 parallel jobs using testFilter (schema\.spec, upgrade\.spec, downgrade\.spec) - Add CDN bundle pre-caching to server.js startup (NODE_ENV=test): downloads all cdn.jsdelivr.net bundles once at startup and rewrites availableVersions paths to serve from localhost, eliminating ~26 network CDN fetches per test run - Add NODE_ENV=test to .env.e2e to activate CDN caching in E2E mode - Add test/cdn-cache/ to .gitignore Expected improvement: ~3x reduction in CI wall-clock time Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets CI wall-clock time improvements for the ExpressSample E2E suite by splitting the previously-serial upgrade/downgrade tests across multiple pipeline jobs and reducing repeated CDN bundle downloads by introducing a local CDN cache served by the sample’s Express server.
Changes:
- Split the combined upgrade/downgrade/schema E2E spec into separate
schema,upgrade, anddowngradespec files. - Update the 3p E2E pipeline config to run ExpressSample tests in 3 parallel jobs via
testFilter. - Add a server-side startup step to pre-download jsDelivr bundles in E2E mode and serve them from a local static route; ignore the cache folder in git.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| samples/msal-browser-samples/ExpressSample/test/upgrade.spec.ts | Removes schema/downgrade coverage from the upgrade test file to enable job splitting. |
| samples/msal-browser-samples/ExpressSample/test/schema.spec.ts | New schema-version verification spec extracted to run independently. |
| samples/msal-browser-samples/ExpressSample/test/downgrade.spec.ts | New downgrade-focused spec extracted to run independently. |
| samples/msal-browser-samples/ExpressSample/server.js | Adds local CDN caching/downloading logic and defers server listen until cache init completes. |
| samples/msal-browser-samples/ExpressSample/.gitignore | Ignores the new local CDN cache directory. |
| samples/msal-browser-samples/ExpressSample/.env.e2e | Sets NODE_ENV=test to enable E2E-only caching behavior. |
| .pipelines/3p-e2e.yml | Splits ExpressSample E2E execution into 3 filtered jobs. |
Comment on lines
+202
to
+222
| const file = fs.createWriteStream(filePath); | ||
| https.get(cdnUrl, (response) => { | ||
| if (response.statusCode === 302 || response.statusCode === 301) { | ||
| // Follow redirect | ||
| file.close(); | ||
| fs.unlinkSync(filePath); | ||
| https.get(response.headers.location, (redirectResponse) => { | ||
| redirectResponse.pipe(file); | ||
| file.on('finish', () => { file.close(); resolve(localPath); }); | ||
| file.on('error', reject); | ||
| }).on('error', reject); | ||
| return; | ||
| } | ||
| response.pipe(file); | ||
| file.on('finish', () => { file.close(); resolve(localPath); }); | ||
| file.on('error', reject); | ||
| }).on('error', (err) => { | ||
| file.close(); | ||
| fs.unlink(filePath, () => {}); | ||
| reject(err); | ||
| }); |
Comment on lines
+203
to
+222
| https.get(cdnUrl, (response) => { | ||
| if (response.statusCode === 302 || response.statusCode === 301) { | ||
| // Follow redirect | ||
| file.close(); | ||
| fs.unlinkSync(filePath); | ||
| https.get(response.headers.location, (redirectResponse) => { | ||
| redirectResponse.pipe(file); | ||
| file.on('finish', () => { file.close(); resolve(localPath); }); | ||
| file.on('error', reject); | ||
| }).on('error', reject); | ||
| return; | ||
| } | ||
| response.pipe(file); | ||
| file.on('finish', () => { file.close(); resolve(localPath); }); | ||
| file.on('error', reject); | ||
| }).on('error', (err) => { | ||
| file.close(); | ||
| fs.unlink(filePath, () => {}); | ||
| reject(err); | ||
| }); |
Comment on lines
+235
to
+238
| console.log('E2E mode: pre-caching CDN bundles to eliminate network latency during tests...'); | ||
| const cdnEntries = Object.entries(availableVersions) | ||
| .filter(([, info]) => info.path && info.path.startsWith('https://')); | ||
|
|
Comment on lines
+239
to
+251
| const results = await Promise.allSettled( | ||
| cdnEntries.map(async ([key, info]) => { | ||
| try { | ||
| const localPath = await downloadCdnBundle(key, info.path); | ||
| availableVersions[key] = { ...info, path: localPath }; | ||
| console.log(` Cached ${key} -> ${localPath}`); | ||
| } catch (err) { | ||
| console.warn(` Failed to cache ${key}: ${err.message} (will use CDN fallback)`); | ||
| } | ||
| }) | ||
| ); | ||
|
|
||
| const failed = results.filter(r => r.status === 'rejected').length; |
| availableVersions[key] = { ...info, path: localPath }; | ||
| console.log(` Cached ${key} -> ${localPath}`); | ||
| } catch (err) { | ||
| console.warn(` Failed to cache ${key}: ${err.message} (will use CDN fallback)`); |
Comment on lines
+46
to
+48
| // Serve locally cached CDN bundles when running in E2E test mode | ||
| const CDN_CACHE_DIR = path.join(__dirname, 'test/cdn-cache'); | ||
| app.use('/lib/msal-cdn-cache', express.static(CDN_CACHE_DIR)); |
| }); | ||
|
|
||
| test("acquireTokenSilent can return tokens from the cache after downgrading back to v4", async () => { | ||
| const testName = "downgradeLocalToLatest"; |
Comment on lines
+201
to
+203
| return new Promise((resolve, reject) => { | ||
| const file = fs.createWriteStream(filePath); | ||
| https.get(cdnUrl, (response) => { |
- Refactor downloadCdnBundle() with inner fetchUrl() to fix reused closed write stream on redirects; create a fresh write stream per hop - Validate Location header presence before following a redirect - Reject on non-2xx HTTP status codes (e.g. 404) instead of silently piping error responses to disk - Add CDN_TIMEOUT_MS (30s) timeout on each https.get request - Gate /lib/msal-cdn-cache static route on NODE_ENV=test to avoid serving the test cache directory in production - Fix Promise.allSettled + inner try/catch anti-pattern in initializeCdnCache(): remove inner catch so rejections propagate, then log failures via result.reason - Add on-demand CDN caching in /api/version/switch for custom (pinned) versions so those also hit localhost instead of CDN - Fix duplicate testName 'downgradeLocalToLatest' in downgrade.spec.ts for the v4 downgrade test; rename to 'downgradeLocalToLatestV4' Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ting Change ref: dev -> ref: feat/express-sample-e2e-speedup so the 3P CI pipeline picks up the --testPathPatterns fix from the 1P feature branch. Revert before merging. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
server.js now calls dotenv.config() twice — first .env, then .env.e2e with override:true — so NODE_ENV=test and e2e credentials are picked up without needing env-cmd on the system PATH. __STARTCMD__ simplified to 'node server.js'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Upgrade jest ^29.5.0 → ^30.0.5, @types/jest ^29.5.0 → ^30.0.0, and ts-jest ^29.1.0 → ^29.4.1 (minimum with Jest 30 peer support). This aligns customizable-e2e-test with ExpressSample's Jest version so the e2e-tests.yml can use --testPathPatterns unconditionally without runtime version detection. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the ~2x CI wall-clock time for ExpressSample E2E tests.
Root causes: (1) No CI job splitting - all 13 tests ran serially in one job. (2) ~26 uncached CDN bundle downloads per run across isolated Puppeteer incognito contexts.
Fix 1 (parallel jobs): Split upgrade-downgrade.spec.ts into schema.spec.ts (1 test), upgrade.spec.ts (6 tests), downgrade.spec.ts (6 tests) and add 3 parallel ExpressSample-1/2/3 entries in 3p-e2e.yml with matching testFilter.
Fix 2 (local CDN): initializeCdnCache() in server.js pre-downloads all cdn.jsdelivr.net bundles at startup (NODE_ENV=test) and rewrites availableVersions paths to localhost, eliminating network latency during version switches.
Expected improvement: ~20-25 min -> ~3-4 min (~6x faster)