From d34067caa3b021fb0b6c9a48064d91bf1725596d Mon Sep 17 00:00:00 2001 From: Kevin Jahns Date: Mon, 31 Aug 2026 20:31:32 +0200 Subject: [PATCH] =?UTF-8?q?=E2=9C=85(e2e)=20stop=20two=20tests=20relying?= =?UTF-8?q?=20on=20removed=20api=20endpoints?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The export regression tests injected their fixture by intercepting `GET /documents/{id}/content/`, and the 401 routing test intercepted the `PATCH .../content/` a document used to send on the way out. Both endpoints went with the yhub migration, so the interceptions matched nothing: the exported document was whatever the two images added to an empty page, and the 401 page was never reached. The fixture now goes to the collaboration server, which is the only thing that holds document content. It is seeded before the editor mounts: its first block carries the id BlockNote gives the empty paragraph it writes into any document it opens empty, so seeding an already open document would leave a stray paragraph in front of the fixture and shift every page. The reference PDFs are unchanged and still match, page count, text and pixels. The 401 test reloads rather than navigating to another document, nothing being sent on the way out any more. Signed-off-by: Kevin Jahns --- .../__tests__/app-impress/doc-routing.spec.ts | 57 ++++++------- .../e2e/__tests__/app-impress/utils-export.ts | 85 +++++++++++++------ 2 files changed, 85 insertions(+), 57 deletions(-) diff --git a/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts b/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts index 1dda69e55..528bdf125 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/doc-routing.spec.ts @@ -8,7 +8,6 @@ import { } from './utils-common'; import { writeInEditor } from './utils-editor'; import { SignIn, expectLoginPage } from './utils-signin'; -import { createRootSubPage } from './utils-sub-pages'; test.describe('Doc Routing', () => { test.beforeEach(async ({ page }) => { @@ -102,44 +101,36 @@ test.describe('Doc Routing', () => { 'This test is only relevant when silent login is disabled.', ); - const [docTitle] = await createDoc(page, '401-doc-parent', browserName, 1); + const [docTitle] = await createDoc(page, '401-doc', browserName, 1); await verifyDocName(page, docTitle); - await createRootSubPage(page, browserName, '401-doc-child'); - await writeInEditor({ page, text: 'Hello World' }); - const responsePromise = page.route( - /.*\/documents\/.*\/$|users\/me\/$/, - async (route) => { - const request = route.request(); + /** + * The session dies underneath an open document: the backend answers 401 to the + * next thing the page asks it about the document. + * + * That next request used to arrive by itself. Leaving a document sent a + * `PATCH .../content/` to save it - which is what this test used to intercept, + * navigating to another document to provoke it. Content is no longer saved over + * the api at all, so nothing is sent on the way out any more and the reload + * below is what puts a question to the backend. + */ + await page.route(/.*\/documents\/.*\/$/, async (route) => { + if (route.request().method() !== 'GET') { + await route.continue(); + return; + } - // When we quit a document, a PATCH request is sent to save the document. - // We intercept this request to simulate a 401 error from the backend. - // The GET request to users/me is also intercepted to simulate the user - // being logged out when trying to fetch user info. - // This way we can test the 401 error handling when saving the document - if ( - (request.url().includes('/documents/') && - request.method().includes('PATCH')) || - (request.url().includes('/users/me/') && - request.method().includes('GET')) - ) { - await route.fulfill({ - status: 401, - json: { - detail: 'Log in to access the document', - }, - }); - } else { - await route.continue(); - } - }, - ); + await route.fulfill({ + status: 401, + json: { + detail: 'Log in to access the document', + }, + }); + }); - await page.getByRole('link', { name: '401-doc-parent' }).click(); - - await responsePromise; + await page.reload(); await expect(page.getByText('Log in to access the document.')).toBeVisible({ timeout: 10000, diff --git a/src/frontend/apps/e2e/__tests__/app-impress/utils-export.ts b/src/frontend/apps/e2e/__tests__/app-impress/utils-export.ts index 87411bfcb..d1112f58d 100644 --- a/src/frontend/apps/e2e/__tests__/app-impress/utils-export.ts +++ b/src/frontend/apps/e2e/__tests__/app-impress/utils-export.ts @@ -6,12 +6,36 @@ import { PDFParse } from 'pdf-parse'; import pixelmatch from 'pixelmatch'; import { PNG } from 'pngjs'; -import { BrowserName, createDoc, writeReport } from './utils-common'; +import { BrowserName, randomName, writeReport } from './utils-common'; import { openSuggestionMenu } from './utils-editor'; /** - * Override the document content API response to use a test content - * This test content contains many blocks to facilitate testing + * The collaboration server's http root, from the websocket url the app is configured + * with: `ws://host/collaboration/ws/v1/docs` -> `http://host/collaboration`. The org is + * the last segment of that path and is spelled out again by each route below. + */ +const collaborationApiUrl = () => + (process.env.COLLABORATION_WS_URL ?? '') + .replace(/^ws/, 'http') + .replace(/\/ws\/v1\/docs\/?$/, ''); + +/** + * Give a document a body worth exporting: the fixture in + * `assets/base-content-test-pdf.txt` holds one block of nearly every kind, which is + * what makes the export regressions below able to catch anything. + * + * The fixture is a Yjs update, and it is pushed to the collaboration server rather + * than stubbed into a response. It used to be the latter - `GET /documents/{id}/content/` + * was intercepted and answered with this file - but that endpoint no longer exists, so + * the interception silently matched nothing and the exported document was whatever the + * two images below added to an empty page. The server is now the only thing that holds + * document content, so seeding it there is also the only way to put content in a + * document without typing it. + * + * The seed lands before the editor ever mounts. A `PATCH` merges, and BlockNote writes + * an empty paragraph into any document it opens empty - seeding a document that has + * already been opened would leave that stray paragraph in front of the fixture and + * shift every page of the render. * @param page */ export const overrideDocContent = async ({ @@ -21,29 +45,42 @@ export const overrideDocContent = async ({ page: Page; browserName: BrowserName; }) => { - // Override content prop with assets/base-content-test-pdf.txt - await page.route(/.*\/documents\/[^/]+\/content\/$/, async (route) => { - const request = route.request(); - if (request.method() === 'GET') { - const response = await route.fetch(); - void route.fulfill({ - response, - body: fs.readFileSync( - path.join(__dirname, 'assets/base-content-test-pdf.txt'), - 'utf-8', - ), - }); - } else { - await route.continue(); - } - }); + const [randomDoc] = randomName('doc-export-override-content', browserName, 1); - const [randomDoc] = await createDoc( - page, - 'doc-export-override-content', - browserName, - 1, + // created over the api rather than through the interface, which would open it + const cookies = await page.context().cookies(); + const csrfToken = cookies.find((c) => c.name === 'csrftoken')?.value ?? ''; + const created = await page.request.post( + `${process.env.BASE_API_URL}/documents/`, + { data: { title: randomDoc }, headers: { 'X-CSRFToken': csrfToken } }, ); + expect(created.ok()).toBeTruthy(); + const { id: docId } = (await created.json()) as { id: string }; + + // `PATCH /ydoc` takes the update in its `update` field; a json body carries a + // Uint8Array as base64, which is what the fixture already is + const seeded = await page.request.patch( + `${collaborationApiUrl()}/ydoc/v1/docs/${docId}`, + { + headers: { Accept: 'application/json' }, + data: { + update: fs + .readFileSync( + path.join(__dirname, 'assets/base-content-test-pdf.txt'), + 'utf-8', + ) + .trim(), + }, + }, + ); + expect(seeded.ok()).toBeTruthy(); + + await page.goto(`/docs/${docId}/`); + + // the seed has to be on screen before anything is added after it + await expect(page.getByText('Hello Heading 1')).toBeVisible({ + timeout: 15000, + }); await expect(page.getByText('copy/pasting out of doc')).toBeVisible();