Skip to content

Commit a52bca0

Browse files
kruschekrusche
authored
Development: Fix the exam submission recovery e2e test reloading with a cold HTTP cache (#13573)
1 parent b1451d6 commit a52bca0

1 file changed

Lines changed: 89 additions & 37 deletions

File tree

‎src/test/playwright/e2e/exam/ExamSubmissionRecovery.spec.ts‎

Lines changed: 89 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { Exercise, ExerciseType } from '../../support/constants';
88
import { ExamAPIRequests } from '../../support/requests/ExamAPIRequests';
99
import { SEED_COURSES } from '../../support/seedData';
1010
import { POLLING_INTERVAL, RELOAD_RENDER_TIMEOUT } from '../../support/timeouts';
11+
import { Commands } from '../../support/commands';
1112

1213
/**
1314
* Regression test for silent exam answer loss after a failed save.
@@ -20,14 +21,12 @@ import { POLLING_INTERVAL, RELOAD_RENDER_TIMEOUT } from '../../support/timeouts'
2021
* both restored in the UI and successfully re-sent to the server.
2122
*/
2223
const course = { id: SEED_COURSES.examParticipation.id } as any;
23-
// Matcher for the quiz exam-save endpoint (PUT /api/quiz/exercises/{id}/submissions/exam), used to inject the failed
24-
// save below. A RegExp keeps the match unambiguous against the absolute request URL.
25-
const quizSaveUrl = /\/api\/quiz\/exercises\/\d+\/submissions\/exam/;
24+
// Chrome DevTools Fetch interception is used instead of Playwright routing below. Playwright routing disables the HTTP
25+
// cache for the page, which makes the production client reload unnecessarily expensive and can starve under parallel CI.
2626

2727
// Ceiling for the post-reload re-send. Generous on purpose, and it costs nothing when things are fast: expect.poll
28-
// returns as soon as the re-send lands, which locally is about two seconds. The ceiling only matters on a loaded CI
29-
// runner, where the re-send is preceded by a full client re-bootstrap with Playwright's per-context HTTP cache
30-
// disabled - bundle and lazy chunks re-fetched, then the exam re-fetched, then the answer restored and sent.
28+
// returns as soon as the re-send lands. The ceiling only matters on a loaded CI runner, where the re-send is preceded
29+
// by a full client bootstrap, exam fetch, and local-storage restoration.
3130
//
3231
// A caution for whoever reads this next: the observed "CI saw zero re-sends" was NOT this budget being too small. It
3332
// was the test reloading before the client had recorded the failed save, after which no re-send can ever happen - see
@@ -40,24 +39,19 @@ const RESEND_TIMEOUT = 4 * RELOAD_RENDER_TIMEOUT;
4039
// derived from it does not sit right on the measurement.
4140
const SETUP_AND_ASSERTION_ALLOWANCE = 120_000;
4241

42+
// Deadline for the forced save to reach the interception. Matches the bound the page.route version had on its
43+
// waitForResponse, so switching to CDP does not quietly turn a missed injection into a four-minute timeout.
44+
const FAILED_SAVE_TIMEOUT = 30_000;
45+
4346
test.describe('Exam submission recovery after a failed save', { tag: '@slow' }, () => {
44-
// Block the Angular service worker for this test. The production WAR registers ngsw-worker.js, which handles the
45-
// quiz exam-save fetch; Playwright's page.route does NOT intercept service-worker-handled requests, so the 503
46-
// outage we inject below was silently bypassed and the save reached the real server (200). Blocking the SW lets
47-
// page.route intercept the save directly; the answer-restore-on-reload logic under test lives in the client
48-
// (local storage), not the SW, so this does not change what the test verifies. serviceWorkers: 'block' is now
49-
// also the global default in playwright.config.ts; this test keeps its own declaration because its route-based
50-
// outage injection is correctness-critical, not merely flake mitigation.
47+
// Block the Angular service worker so the request reaches the page network target intercepted below. The
48+
// answer-restore-on-reload logic under test lives in the client (local storage), not the service worker.
49+
// serviceWorkers: 'block' is also the global default in playwright.config.ts; this test keeps its own declaration
50+
// because its outage injection is correctness-critical.
5151
test.use({ serviceWorkers: 'block' });
5252

53-
// The slow-tests project allows 90s per test, which this test cannot meet in CI: it was measured at ~113s there,
54-
// because the setup (start participation, navigate, tick, forced failed save) runs before a reload that has to
55-
// re-bootstrap the client with the HTTP cache disabled, re-fetch the exam and re-send the restored answer.
56-
//
57-
// Derived from RESEND_TIMEOUT rather than hard-coded, so the two cannot drift apart: RELOAD_RENDER_TIMEOUT is
58-
// overridable via RELOAD_RENDER_TIMEOUT_MS, and a fixed cap here would silently become smaller than the wait it
59-
// is supposed to contain. SETUP_AND_ASSERTION_ALLOWANCE covers everything outside that wait, generously - the
60-
// measured setup is roughly 50s.
53+
// The slow-tests project allows 90s per test. This test needs additional room because it deliberately performs a
54+
// failed save and a full client reload before waiting for the automatic recovery save.
6155
test.setTimeout(RESEND_TIMEOUT + SETUP_AND_ASSERTION_ALLOWANCE);
6256

6357
let exam: Exam;
@@ -76,22 +70,67 @@ test.describe('Exam submission recovery after a failed save', { tag: '@slow' },
7670
await examParticipation.startParticipation(studentTwo, course, exam);
7771
await examNavigation.openOrSaveExerciseByTitle(quizExercise.exerciseGroup!.title!);
7872

79-
// Simulate a failed save (as during an outage) BEFORE touching the answer: make the quiz exam save endpoint fail.
80-
// Installing it before the first answer change guarantees no save can succeed first (e.g. a coincidental 30s
81-
// autosave) and silently mark the answer synced, which would make the forced save below a no-op.
82-
await page.route(quizSaveUrl, (route) => route.fulfill({ status: 503, contentType: 'application/json', body: '{}' }));
73+
// Simulate a failed save (as during an outage) BEFORE touching the answer. CDP Fetch interception preserves the
74+
// HTTP cache; Playwright's page.route disables it for the page and made the production reload stall under
75+
// parallel CI load.
76+
//
77+
// Every matching save fails for as long as the outage lasts, not only the first one. Fetch.enable pauses every
78+
// request matching the pattern, so a single-shot handler answers one and leaves a second - the 30s autosave, or
79+
// a client retry after the 503 - paused with nothing to answer it. Lifting the outage releases that request,
80+
// which can then reach the server for real and mark the answer synced before the reload. The recovery under test
81+
// would have nothing left to restore, and the re-send awaited below could never happen.
82+
const cdpSession = await page.context().newCDPSession(page);
83+
let outageActive = true;
84+
const failedSave = new Promise<void>((resolveFailedSave, rejectFailedSave) => {
85+
// Bounded on purpose. Without a deadline, a save that never reaches the interception - a changed endpoint, a
86+
// pattern that stops matching - leaves this pending until the per-test timeout, which reports the re-send as
87+
// the failure and hides that no save was ever injected. The wait is for one request the click just issued,
88+
// so it is either answered promptly or something is wrong.
89+
const deadline = setTimeout(
90+
() => rejectFailedSave(new Error(`No save request was intercepted within ${FAILED_SAVE_TIMEOUT}ms, so the outage under test was never injected`)),
91+
FAILED_SAVE_TIMEOUT,
92+
);
93+
cdpSession.on('Fetch.requestPaused', async ({ requestId }) => {
94+
try {
95+
await cdpSession.send('Fetch.fulfillRequest', {
96+
requestId,
97+
responseCode: 503,
98+
responseHeaders: [{ name: 'Content-Type', value: 'application/json' }],
99+
body: 'e30=',
100+
});
101+
// Settles on the first failed save; a promise ignores every later call.
102+
clearTimeout(deadline);
103+
resolveFailedSave();
104+
} catch (error) {
105+
// Lifting the outage detaches the session, and a request paused at that moment is released by
106+
// Fetch.disable rather than answered here, so only a failure during the outage means the injection
107+
// itself is broken.
108+
if (outageActive) {
109+
clearTimeout(deadline);
110+
rejectFailedSave(error as Error);
111+
}
112+
}
113+
});
114+
});
115+
await cdpSession.send('Fetch.enable', {
116+
patterns: [{ urlPattern: `*://*/api/quiz/exercises/${quizExercise.id}/submissions/exam`, requestStage: 'Request' }],
117+
});
83118

84119
// Tick an answer option; the exercise becomes unsynced.
85120
await quizExerciseMultipleChoice.tickAnswerOption(quizExercise.id!, 0);
86121
await expect(getExercise(page, quizExercise.id!).locator('#answer-option-0')).toHaveClass(/selected/);
87122

88-
// Force a save attempt and wait deterministically for the failed (503) save instead of a fixed timeout.
123+
// Force a save attempt and wait deterministically until CDP has fulfilled it with 503.
89124
// The answer is written to local storage but the server submission stays empty.
90-
const failedSave = page.waitForResponse((response) => response.url().includes(`/quiz/exercises/${quizExercise.id}/submissions/exam`) && response.status() === 503, {
91-
timeout: 30000,
92-
});
93125
await getExercise(page, quizExercise.id!).locator('#save-exam').click();
94-
await failedSave;
126+
try {
127+
await failedSave;
128+
} catch (injectionFailed) {
129+
// Leave no interception behind on the way out, or the paused request outlives the test.
130+
outageActive = false;
131+
await cdpSession.detach().catch(() => undefined);
132+
throw injectionFailed;
133+
}
95134

96135
// Wait for the CLIENT to have recorded the failure, not merely for the 503 to appear on the wire.
97136
//
@@ -115,11 +154,9 @@ test.describe('Exam submission recovery after a failed save', { tag: '@slow' },
115154
// Record SUCCESSFUL re-sends, but only ones issued after the reload has committed.
116155
//
117156
// Both boundaries matter. The listener is attached before the outage is lifted so it cannot miss a re-send the
118-
// client fires during its own start-up, which a waitForResponse registered after page.reload() would never
119-
// see. But attaching it that early leaves a window between unroute() and reload() in which the existing
120-
// page's autosave could fire a successful PUT: that would satisfy the poll below while proving nothing, since
121-
// the reload would then restore an answer the server already had. Gating on the main-frame navigation closes
122-
// that window - the reload is the next main-frame navigation after this point.
157+
// client fires during its own start-up. Attaching it that early leaves a window between disabling interception
158+
// and reload in which the existing page's autosave could fire a successful PUT. Gating on the main-frame
159+
// navigation closes that window: the reload is the next main-frame navigation after this point.
123160
let reloadCommitted = false;
124161
page.on('framenavigated', (frame) => {
125162
if (frame === page.mainFrame()) {
@@ -137,8 +174,23 @@ test.describe('Exam submission recovery after a failed save', { tag: '@slow' },
137174
});
138175

139176
// Stop failing saves so the post-reload re-send can succeed.
140-
await page.unroute(quizSaveUrl);
141-
await page.reload();
177+
// The route to come back to after the reload, read before it can drift.
178+
const examUrl = page.url();
179+
180+
outageActive = false;
181+
await cdpSession.send('Fetch.disable');
182+
await cdpSession.detach();
183+
// Reload through the route-restoring helper rather than page.reload() directly.
184+
//
185+
// This is what the test was missing. A reload re-bootstraps the SPA, and when a lazy route chunk fails to
186+
// resolve behind the multi-node HTTPS load balancer - an intermittent module-fetch failure the suite already
187+
// recovers from elsewhere - the router drops to the /courses fallback and never returns. The exam is then not
188+
// on screen, so the client never restores the answer and never re-sends it, and the poll below waits out its
189+
// whole budget for something that can no longer happen. That is the failure this test kept hitting in CI, and
190+
// no timeout is large enough to fix it. Restoring the route explicitly turns that dead end into one more
191+
// attempt, and reports honestly when the SPA could not load the route at all.
192+
const restoredExamRoute = await Commands.reloadAndRestoreRoute(page, examUrl);
193+
expect(restoredExamRoute, 'the exam route did not survive the reload, so no recovery could be attempted').toBe(true);
142194

143195
// The client re-sends the restored answer by itself while starting up, so the only thing to do is wait for it.
144196
// Nothing is clicked here on purpose: by the time the exercise is on screen the save button is already

0 commit comments

Comments
 (0)