fix(ci, webpage): exit E2E step with proper exit code and fix broken …

…website tests (#2188) --------- Co-authored-by: Cornelius Roemer <[email protected]> Co-authored-by: Theo Sanderson <[email protected]>
loculus-project · Jun 27, 2024 · ab4ddcb · ab4ddcb
1 parent f076cc1
commit ab4ddcb
Show file tree

Hide file tree

Showing 7 changed files with 34 additions and 38 deletions.
diff --git a/.github/workflows/e2e-k3d.yml b/.github/workflows/e2e-k3d.yml
@@ -132,10 +132,13 @@ jobs:
         run: sleep 10
       - name: Run E2E test
         run: |
+          set -o pipefail
           cd website && npm run e2e 2>&1 | tee output.txt
+          EXIT_CODE=$?
           echo '```' >> $GITHUB_STEP_SUMMARY
           sed -n '/Running [0-9]\+ tests/,$p' output.txt >> $GITHUB_STEP_SUMMARY
           echo '```' >> $GITHUB_STEP_SUMMARY
+          exit $EXIT_CODE
       - uses: actions/upload-artifact@v4
         if: ${{ failure() }}
         with:

diff --git a/website/src/components/SearchPage/Table.tsx b/website/src/components/SearchPage/Table.tsx
@@ -78,7 +78,7 @@ export const Table: FC<TableProps> = ({
         );
 
     return (
-        <div className='w-full overflow-x-auto text-sm'>
+        <div className='w-full overflow-x-auto text-sm' aria-label='Search Results Table'>
             <Tooltip id='table-tip' />
             {data.length !== 0 ? (
                 <table className='w-full text-left border-collapse'>
@@ -109,7 +109,10 @@ export const Table: FC<TableProps> = ({
                                     row[primaryKey] === previewedSeqId ? 'bg-gray-200' : ''
                                 } `}
                             >
-                                <td className='px-2  whitespace-nowrap    text-primary-900 md:pl-6'>
+                                <td
+                                    className='px-2  whitespace-nowrap    text-primary-900 md:pl-6'
+                                    aria-label='SearchResult'
+                                >
                                     <a
                                         href={routes.sequencesDetailsPage(row[primaryKey] as string)}
                                         className='text-primary-900 hover:text-primary-800 hover:no-underline'

diff --git a/website/src/pages/seq/[accessionVersion]/index.astro b/website/src/pages/seq/[accessionVersion]/index.astro
@@ -65,7 +65,7 @@ const runtimeConfig = getRuntimeConfig();
                                 : undefined
                         }
                         showFastaDownload={
-                            result.type === SequenceDetailsTableResultType.TABLE_DATA && result.isRevocation
+                            result.type === SequenceDetailsTableResultType.TABLE_DATA && !result.isRevocation
                         }
                     />
                     {result.type === SequenceDetailsTableResultType.TABLE_DATA &&

diff --git a/website/src/utils/parseFasta.ts b/website/src/utils/parseFasta.ts
@@ -6,9 +6,11 @@ export type FastaEntry = {
 };
 
 export function parseFasta(fasta: string) {
+    if (fasta === '') {
+        return [];
+    }
     const fastaEntries: FastaEntry[] = [];
     let currentEntry: FastaEntry | null = null;
-
     const lines = fasta.split('\n');
     for (const line of lines) {
         if (line.startsWith('>')) {

diff --git a/website/tests/pages/navigation.spec.ts b/website/tests/pages/navigation.spec.ts
@@ -2,7 +2,7 @@ import { baseUrl, dummyOrganism, test } from '../e2e.fixture';
 
 const organismIndependentNavigationItems = [
     { link: 'My account', title: 'My account' },
-    { link: 'API docs', title: 'Api Docs' },
+    { link: 'API docs', title: 'API Documentation' },
     { link: 'Governance', title: 'Governance' },
     { link: 'Status', title: 'Status' },
 ];

diff --git a/website/tests/pages/search/index.spec.ts b/website/tests/pages/search/index.spec.ts
@@ -6,12 +6,6 @@ import { baseUrl, dummyOrganism, expect, test } from '../../e2e.fixture';
 import { getTestSequences } from '../../util/testSequenceProvider.ts';
 
 test.describe('The search page', () => {
-    test('should show the search form with button and a table', async ({ searchPage }) => {
-        await searchPage.goto();
-        await expect(searchPage.searchButton).toBeVisible();
-        await expect(searchPage.table).toBeVisible();
-    });
-
     test('should find no data in the future', async ({ searchPage }) => {
         const tomorrow = DateTime.now().plus({ days: 1 }).toISODate();
 
@@ -28,12 +22,11 @@ test.describe('The search page', () => {
         await searchPage.getAccessionField().click();
         await searchPage.getAccessionField().fill(testAccessionVersion);
 
-        await searchPage.clickSearchButton();
-
         await searchPage.page.waitForURL(
             `${baseUrl}${routes.searchPage(dummyOrganism.key)}?accession=${testAccessionVersion}`,
         );
-        const accessionLink = searchPage.page.getByText(testAccessionVersion, { exact: true });
+        const accessionLink = searchPage.page.getByRole('link', { name: testAccessionVersion });
+        await searchPage.page.getByText('Search returned 1 sequence');
         await expect(accessionLink).toBeVisible();
 
         const rowLocator = searchPage.page.locator('tr');
@@ -46,14 +39,13 @@ test.describe('The search page', () => {
 
     test('should search a few sequence entries by accession', async ({ searchPage }) => {
         await searchPage.goto();
-        const previousAccessions = (await searchPage.getTableContent()).map((arr) => arr[0]);
+        const previousAccessions = await searchPage.getAccessions(3);
 
         const query = `doesnotexist\n${previousAccessions[0]},${previousAccessions[1]}\t${previousAccessions[2]}`;
         await searchPage.getAccessionField().click();
         await searchPage.getAccessionField().fill(query);
-        await searchPage.clickSearchButton();
 
-        const newAccessions = (await searchPage.getTableContent()).map((arr) => arr[0]);
+        const newAccessions = await searchPage.getAccessions(3);
 
         expect(newAccessions.length).toBe(3);
         expect(newAccessions.includes(previousAccessions[0])).toBeTruthy();
@@ -63,17 +55,16 @@ test.describe('The search page', () => {
 
     test('should search many sequence entries by accession', async ({ searchPage }) => {
         await searchPage.goto();
-        const previousAccessions = (await searchPage.getTableContent()).map((arr) => arr[0]);
+        const previousAccessions = await searchPage.getAccessions(3);
 
         let query = `doesnotexist\n${previousAccessions[0]},${previousAccessions[1]}\t${previousAccessions[2]}`;
         for (let i = 0; i < 1000; i++) {
             query += `\ndoesnotexist${i}`;
         }
         await searchPage.getAccessionField().click();
         await searchPage.getAccessionField().fill(query);
-        await searchPage.clickSearchButton();
 
-        const newAccessions = (await searchPage.getTableContent()).map((arr) => arr[0]);
+        const newAccessions = await searchPage.getAccessions(3);
 
         expect(newAccessions.length).toBe(3);
         expect(newAccessions.includes(previousAccessions[0])).toBeTruthy();
@@ -85,9 +76,10 @@ test.describe('The search page', () => {
         await searchPage.goto();
         await searchPage.searchFor([{ name: 'country', filterValue: 'Switzerland' }]);
 
-        const resultCount = await searchPage.page.getByText('Switzerland').count();
-
-        expect(resultCount).toBeGreaterThan(0);
+        await searchPage.page.locator('tr').first().waitFor();
+        const rowLocator = await searchPage.page.locator('tr').getByText('Switzerland');
+        const rowCount = await rowLocator.count();
+        await expect(rowCount).toBeGreaterThan(0);
     });
 
     test('should reset the search', async ({ searchPage }) => {

diff --git a/website/tests/pages/search/search.page.ts b/website/tests/pages/search/search.page.ts
@@ -6,12 +6,10 @@ import { baseUrl, dummyOrganism } from '../../e2e.fixture';
 export const ACCESSION = 'Accession';
 
 export class SearchPage {
-    public readonly searchButton: Locator;
     public readonly table: Locator;
     public readonly resetButton: Locator;
 
     constructor(public readonly page: Page) {
-        this.searchButton = page.getByRole('button', { name: 'Search sequences' });
         this.resetButton = page.getByRole('button', { name: 'reset' });
         this.table = page.getByRole('table');
     }
@@ -20,10 +18,6 @@ export class SearchPage {
         await this.page.goto(`${baseUrl}${routes.searchPage(dummyOrganism.key)}`);
     }
 
-    public async clickSearchButton() {
-        await this.searchButton.click();
-    }
-
     public async clickResetButton() {
         await this.resetButton.click();
     }
@@ -34,19 +28,21 @@ export class SearchPage {
 
     public async searchFor(params: { name: string; filterValue: string }[]) {
         await this.page.goto(
-            `${baseUrl}${routes.searchPage(dummyOrganism.key)}${params
+            `${baseUrl}${routes.searchPage(dummyOrganism.key)}?${params
                 .map((param) => `&${param.name}=${param.filterValue}`)
                 .join('')}`,
         );
     }
 
-    public async getTableContent() {
-        const tableData = await this.page.locator('table >> css=tr').evaluateAll((rows) => {
-            return rows.map((row) => {
-                const cells = Array.from(row.querySelectorAll('td'));
-                return cells.map((cell) => cell.textContent!.trim());
-            });
-        });
-        return tableData.slice(1);
+    public async getAccessions(elementsCount: number) {
+        const rowLocator = this.page.locator('tr');
+        const accessions = [];
+
+        for (let index = 1; index < 1 + elementsCount; index++) {
+            const element = await rowLocator.nth(index);
+            const innerText = await element.innerText();
+            accessions.push(innerText.split(' ')[0]);
+        }
+        return accessions;
     }
 }