test(e2e): refactor revise tests, SSR disable file upload (#1833)

- Refactor revise test to not require an explicit file but set file from buffer. (Async file setting was also incorrectly not awaited for) - Disable buttons when React stuff is server side rendered. Playwright would otherwise click buttons in HTML without the react event handlers attached which made tests flaky: in particular locally where things are faster than in CI - Uses data-testid for elements/buttons that are hard to get from Playwright otherwise - Get rid of unnecessarily tight timeout (1s) in seqset test (default is 5s) - Add npm run script e2e:ui that opens playwright GUI mode with headed playwright: this makes it particularly easy to debug tests and see why they fail (extensively used in the preparation of this)
loculus-project · May 9, 2024 · c29b912 · c29b912
1 parent 9106d31
commit c29b912
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 42 deletions.
diff --git a/website/package.json b/website/package.json
@@ -16,6 +16,7 @@
         "format-fast": "prettier --ignore-path \"../.gitignore\" --write \"./**/*.{ts,tsx,json,astro,cjs,mjs}\"",
         "test": "vitest",
         "e2e": "npx playwright test",
+        "e2e:ui": "npx playwright test --ui --headed",
         "e2e:headed": "npx playwright test --headed"
     },
     "dependencies": {

diff --git a/website/src/components/Submission/DataUploadForm.tsx b/website/src/components/Submission/DataUploadForm.tsx
@@ -143,6 +143,12 @@ const UploadComponent = ({
 }) => {
     const [myFile, rawSetMyFile] = useState<File | null>(null);
     const [isDragOver, setIsDragOver] = useState(false);
+    const [isClient, setIsClient] = useState(false);
+
+    useEffect(() => {
+        setIsClient(true);
+    }, []);
+
     const setMyFile = useCallback(
         (file: File | null) => {
             setFile(file);
@@ -203,23 +209,23 @@ const UploadComponent = ({
                     <div className='text-center'>
                         <Icon className='mx-auto h-12 w-12 text-gray-300' aria-hidden='true' />
                         <div className='mt-4  text-sm leading-6 text-gray-600'>
-                            <label
-                                htmlFor='file-upload'
-                                className='inline relative cursor-pointer rounded-md bg-white font-semibold text-primary-600 focus-within:outline-none focus-within:ring-2 focus-within:ring-primary-600 focus-within:ring-offset-2 hover:text-primary-500'
-                            >
-                                <span onClick={handleUpload}>Upload a file</span>
-                                <input
-                                    id={name}
-                                    name={name}
-                                    type='file'
-                                    className='sr-only'
-                                    aria-label={title}
-                                    onChange={(event) => {
-                                        const file = event.target.files?.[0] || null;
-                                        setMyFile(file);
-                                    }}
-                                    ref={fileInputRef}
-                                />
+                            <label className='inline relative cursor-pointer rounded-md bg-white font-semibold text-primary-600 focus-within:outline-none focus-within:ring-2 focus-within:ring-primary-600 focus-within:ring-offset-2 hover:text-primary-500'>
+                                <span onClick={handleUpload}>Upload a file </span>
+                                {isClient && (
+                                    <input
+                                        id={name}
+                                        name={name}
+                                        type='file'
+                                        className='sr-only'
+                                        aria-label={title}
+                                        data-testid={name}
+                                        onChange={(event) => {
+                                            const file = event.target.files?.[0] || null;
+                                            setMyFile(file);
+                                        }}
+                                        ref={fileInputRef}
+                                    />
+                                )}
                             </label>
                             <span className='pl-1'>or drag and drop</span>
                         </div>
@@ -232,6 +238,7 @@ const UploadComponent = ({
                     <div className='text-sm text-gray-500 py-5'>{myFile.name}</div>
                     <button
                         onClick={() => setMyFile(null)}
+                        data-testid={`discard_${name}`}
                         className='
                     text-xs break-words text-gray-700 py-1.5 px-4 border border-gray-300 rounded-md hover:bg-gray-50'
                     >
@@ -260,6 +267,12 @@ const InnerDataUploadForm = ({
     const [dataUseTermsType, setDataUseTermsType] = useState<DataUseTermsType>(openDataUseTermsType);
     const [restrictedUntil, setRestrictedUntil] = useState<DateTime>(dateTimeInMonths(6));
 
+    const [isClient, setIsClient] = useState(false);
+
+    useEffect(() => {
+        setIsClient(true);
+    }, []);
+
     const handleLoadExampleData = async () => {
         const { metadataFileContent, revisedMetadataFileContent, sequenceFileContent } = getExampleData(exampleEntries);
 
@@ -387,9 +400,9 @@ const InnerDataUploadForm = ({
                     <button
                         name='submit'
                         type='submit'
-                        className='rounded-md bg-primary-600 py-2 text-sm font-semibold text-white shadow-sm hover:bg-primary-500 focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-primary-600'
+                        className='rounded-md py-2 text-sm font-semibold shadow-sm focus-visible:outline focus-visible:outline-2 focus-visible:outline-offset-2 bg-primary-600 text-white hover:bg-primary-500'
                         onClick={handleSubmit}
-                        disabled={isLoading}
+                        disabled={isLoading || !isClient}
                     >
                         <div className={`absolute ml-1.5 inline-flex ${isLoading ? 'visible' : 'invisible'}`}>
                             <span className='loading loading-spinner loading-sm' />

diff --git a/website/tests/pages/revise/revise.page.ts b/website/tests/pages/revise/revise.page.ts
@@ -1,40 +1,34 @@
-import { unlinkSync, writeFileSync } from 'fs';
-
-import type { Locator, Page } from '@playwright/test';
-import { v4 as uuid } from 'uuid';
+import { expect, type Page } from '@playwright/test';
 
 import { routes } from '../../../src/routes/routes.ts';
 import type { Accession } from '../../../src/types/backend.ts';
 import { baseUrl, dummyOrganism, sequencesTestFile } from '../../e2e.fixture';
 import { createModifiedFileContent } from '../../util/createFileContent.ts';
 
 export class RevisePage {
-    public readonly submitButton: Locator;
-    private readonly temporaryMetadataFile: string = `./tests/testData/${uuid()}_metadata.tsv`;
-
-    constructor(public readonly page: Page) {
-        this.submitButton = page.getByRole('button', { name: 'Submit' });
-    }
+    constructor(public readonly page: Page) {}
 
     public async goto(groupId: number) {
         await this.page.goto(`${baseUrl}${routes.revisePage(dummyOrganism.key, groupId)}`);
     }
 
-    public async uploadSequenceData(file: string = sequencesTestFile) {
-        await this.page.getByLabel('Sequence file').setInputFiles(file);
+    public async submitRevisedData(accessions: Accession[]) {
+        await this.setSequenceFile();
+        await this.setRevisedMetadataFile(accessions);
+        await this.page.getByRole('button', { name: 'Submit' }).click();
     }
 
-    public async submitRevisedData(accessions: Accession[]) {
-        try {
-            await Promise.all([this.uploadSequenceData(), this.uploadRevisedMetadata(accessions)]);
-            await this.submitButton.click();
-        } finally {
-            unlinkSync(this.temporaryMetadataFile);
-        }
+    private async setSequenceFile(file: string = sequencesTestFile) {
+        await this.page.getByTestId('sequence_file').setInputFiles(file);
+        await expect(this.page.getByTestId('discard_sequence_file')).toBeEnabled();
     }
 
-    private async uploadRevisedMetadata(accessions: Accession[]) {
-        writeFileSync(this.temporaryMetadataFile, createModifiedFileContent(accessions).metadataContent);
-        await this.page.getByLabel('Metadata file').setInputFiles(this.temporaryMetadataFile);
+    private async setRevisedMetadataFile(accessions: Accession[]) {
+        await this.page.getByTestId('metadata_file').setInputFiles({
+            name: 'metadata.tsv',
+            mimeType: 'text/plain',
+            buffer: Buffer.from(createModifiedFileContent(accessions).metadataContent),
+        });
+        await expect(this.page.getByTestId('discard_metadata_file')).toBeEnabled();
     }
 }
diff --git a/website/tests/pages/user/group/group.page.ts b/website/tests/pages/user/group/group.page.ts
@@ -52,7 +52,7 @@ export class GroupPage {
         const createGroupButton = this.page.getByRole('button', { name: 'Create group' });
         await createGroupButton.click();
 
-        await this.page.waitForURL(/\/group\/\d+/, { timeout: 1000 });
+        await this.page.waitForURL(/\/group\/\d+/);
         return Number(this.page.url().split('/').pop());
     }