Merge pull request #560 from fractal-analytics-platform/test-improvements

Improved stability of end to end tests

Author: zonia3000

CHANGELOG.md

@@ -1,5 +1,9 @@
 *Note: Numbers like (\#123) point to closed Pull Requests on the fractal-web repository.*
 
+# Unreleased
+
+* Improved stability of end to end tests (\#560);
+
 # 1.6.0
 
 * Displayed OAuth2 accounts in user profile page (\#557);

package-lock.json

@@ -427,10 +427,10 @@
 							<td>
 								{#if projects && row.project_id !== null && row.user_email === $page.data.userInfo.email}
 									<a href={`/v2/projects/${row.project_id}`}>
-										{projects.find((project) => project.id === row.project_id)?.name}
+										{row.project_dump.name}
 									</a>
 								{:else}
-									{projects.find((project) => project.id === row.project_id)?.name || '-'}
+									{row.project_dump.name || '-'}
 								{/if}
 							</td>
 						{/if}

src/lib/components/v2/jobs/JobsList.svelte

@@ -4,7 +4,10 @@ const PORT = process.env.PORT || 8080;
 
 const server = createServer();
 
-const tasksStatusMap = new Map();
+/** @type {Map<number, string>} */
+const jobsStatusMapV1 = new Map();
+/** @type {Map<number, string>} */
+const jobsStatusMapV2 = new Map();
 
 server.on('request', (request, response) => {
 	response.setHeader('Content-type', 'text/plain');
@@ -20,46 +23,65 @@ server.on('request', (request, response) => {
 		if (!task) {
 			return badRequest(response, 'missing task identifier');
 		}
-		const status = getTaskStatus(task);
+		const { version, jobId } = extractJobInfo(task);
+		const status = getJobStatus(version, jobId);
 		if (request.method === 'GET' && status !== 'submitted') {
-			tasksStatusMap.delete(task);
+			console.log(`Removing job ${jobId}`);
+			getJobStatusMap(version).delete(jobId);
 		}
 		response.end(status);
 	} else {
 		// PUT method is called by Playwright tests to update the desired task status
-		if (url.pathname.startsWith('/done')) {
-			setRunningTasksTo('done');
-			response.end('done');
-		} else if (url.pathname.startsWith('/failed')) {
-			setRunningTasksTo('failed');
-			response.end('failed');
+		const match = url.pathname.match(/\/(v1|v2)\/(\d+)/);
+		if (!match) {
+			return badRequest(response, `missing job identifier in ${url.pathname}`);
 		}
+		const jobId = Number(match[2]);
+		const status = url.searchParams.get('status');
+		if (!status) {
+			return badRequest(response, 'missing status');
+		}
+		const version = url.pathname.startsWith('/v2') ? 'v2' : 'v1';
+		console.log(`Setting job ${jobId} status to ${status}`);
+		getJobStatusMap(version).set(Number(jobId), status);
+		response.end(status);
 	}
 });
 
-function getTaskStatus(task) {
-	if (!tasksStatusMap.has(task)) {
-		console.log(`Creating entry for task ${task}`);
-		// Set all the other running tasks to failed
-		// If there are no flacky tests there shouldn't be other running tasks
-		setRunningTasksTo('failed');
-		// Add new task as submitted
-		tasksStatusMap.set(task, 'submitted');
+/**
+ * @param {string} taskFolder
+ */
+function extractJobInfo(taskFolder) {
+	const match = taskFolder.match(/job_(\d+)/);
+	if (!match) {
+		throw new Error(`Unable to extract job id from ${taskFolder}`);
 	}
-	return tasksStatusMap.get(task);
+	const version = taskFolder.includes('proj_v2') ? 'v2' : 'v1';
+	return { jobId: Number(match[1]), version };
 }
 
 /**
- * We handle only one running task, to simplify the fake server.
- * In case of flacky Playwright tests there could be more than one submitted task.
- * In that case we set all the submitted task to the same desired final value.
- * @param {string} status
+ * @param {string} version
+ * @param {number} jobId
+ * @returns {string|undefined}
  */
-function setRunningTasksTo(status) {
-	for (const [key, value] of tasksStatusMap.entries()) {
-		if (value === 'submitted') {
-			tasksStatusMap.set(key, status);
-		}
+function getJobStatus(version, jobId) {
+	const jobsStatusMap = getJobStatusMap(version);
+	if (!jobsStatusMap.has(jobId)) {
+		console.log(`Creating entry for job ${jobId}`);
+		jobsStatusMap.set(jobId, 'submitted');
+	}
+	return jobsStatusMap.get(jobId);
+}
+
+/**
+ * @param {string} version
+ */
+function getJobStatusMap(version) {
+	if (version === 'v1') {
+		return jobsStatusMapV1;
+	} else {
+		return jobsStatusMapV2;
 	}
 }
 

tests/fake-job-server.js

@@ -1,14 +1,15 @@
 import { expect, test } from '@playwright/test';
-import { waitPageLoading } from '../utils.js';
+import { waitModalClosed, waitPageLoading } from '../utils.js';
 import { PageWithWorkflow } from './workflow_fixture.js';
 import * as fs from 'fs';
 
 test('Execute a job and show it on the job tables [v1]', async ({ page, request }) => {
 	/** @type {PageWithWorkflow} */
 	let workflow1;
 	await test.step('Create first job and wait its failure', async () => {
-		workflow1 = await createJob(page, request, 'input', 'output');
-		await workflow1.triggerTaskFailure();
+		const job = await createJob(page, request, 'input', 'output');
+		workflow1 = job.workflow;
+		await workflow1.triggerTaskFailure(job.jobId);
 		const jobBadge = page.locator('.badge.text-bg-danger');
 		await jobBadge.waitFor();
 		expect(await jobBadge.innerText()).toEqual('failed');
@@ -17,8 +18,12 @@ test('Execute a job and show it on the job tables [v1]', async ({ page, request
 
 	/** @type {PageWithWorkflow} */
 	let workflow2;
+	/** @type {number} */
+	let jobId2;
 	await test.step('Create second job', async () => {
-		workflow2 = await createJob(page, request, 'input2', 'output2');
+		const job = await createJob(page, request, 'input2', 'output2');
+		workflow2 = job.workflow;
+		jobId2 = job.jobId;
 	});
 
 	await test.step('Open the admin jobs', async () => {
@@ -84,7 +89,7 @@ test('Execute a job and show it on the job tables [v1]', async ({ page, request
 	});
 
 	await test.step('Wait job completion', async () => {
-		await workflow2.triggerTaskSuccess();
+		await workflow2.triggerTaskSuccess(jobId2);
 		const jobBadge = page.locator('.badge.text-bg-success');
 		await jobBadge.waitFor();
 		expect(await jobBadge.innerText()).toEqual('done');
@@ -161,19 +166,22 @@ async function getWorkflowRow(page, workflowName) {
  * @param {import('@playwright/test').APIRequestContext} request
  * @param {string} inputDataset
  * @param {string} outputDataset
+ * @returns {Promise<{ workflow: PageWithWorkflow, jobId: number }>}
  */
 async function createJob(page, request, inputDataset, outputDataset) {
 	const workflow = new PageWithWorkflow(page, request);
+	/** @type {number|undefined} */
+	let jobId;
 	await test.step('Create workflow', async () => {
 		await workflow.createProject();
 		await workflow.createDataset(inputDataset, 'image');
 		await workflow.createDataset(outputDataset, 'zarr');
 		await workflow.createWorkflow();
 		await page.waitForURL(/** @type {string} */ (workflow.url));
 		await addTaskToWorkflow(workflow);
-		await runWorkflow(page, inputDataset, outputDataset);
+		jobId = await runWorkflow(page, inputDataset, outputDataset);
 	});
-	return workflow;
+	return { workflow, jobId: /** @type {number} */ (jobId) };
 }
 
 /**
@@ -189,8 +197,11 @@ async function addTaskToWorkflow(workflow) {
  * @param {import('@playwright/test').Page} page
  * @param {string} inputDataset
  * @param {string} outputDataset
+ * @returns {Promise<number>} the id of the job
  */
 async function runWorkflow(page, inputDataset, outputDataset) {
+	/** @type {number|undefined} */
+	let jobId;
 	await test.step('Run workflow', async () => {
 		const runWorkflowBtn = page.getByRole('button', { name: 'Run workflow' });
 		await runWorkflowBtn.click();
@@ -204,5 +215,16 @@ async function runWorkflow(page, inputDataset, outputDataset) {
 		const confirmBtn = page.locator('.modal.show').getByRole('button', { name: 'Confirm' });
 		await confirmBtn.click();
 		await page.waitForURL(new RegExp(`/v1/projects/(\\d+)/workflows/(\\d+)/jobs`));
+		await page
+			.getByRole('row', { name: 'submitted' })
+			.getByRole('button', { name: 'Info' })
+			.click();
+		const modal = page.locator('.modal.show');
+		await modal.waitFor();
+		const value = await modal.getByRole('listitem').nth(1).textContent();
+		jobId = Number(value?.trim());
+		await modal.getByRole('button', { name: 'Close' }).click();
+		await waitModalClosed(page);
 	});
+	return /** @type {number} */ (jobId);
 }

tests/v1/admin_jobs.spec.js

@@ -25,7 +25,9 @@ test('Execute jobs', async ({ page, workflow }) => {
 	});
 
 	await test.step('Check workflow jobs page', async () => {
-		await page.waitForURL(`/v1/projects/${workflow.projectId}/workflows/${workflow.workflowId}/jobs`);
+		await page.waitForURL(
+			`/v1/projects/${workflow.projectId}/workflows/${workflow.workflowId}/jobs`
+		);
 		await page.locator('table tbody').waitFor();
 		expect(await page.locator('table tbody tr').count()).toEqual(1);
 		const cells = await page.locator('table tbody tr td').allInnerTexts();
@@ -82,7 +84,7 @@ test('Execute jobs', async ({ page, workflow }) => {
 		const modalTitle = page.locator('.modal.show .modal-title');
 		await modalTitle.waitFor();
 		await expect(modalTitle).toHaveText('Workflow Job logs');
-		await workflow.triggerTaskFailure();
+		await workflow.triggerTaskFailure(jobId);
 		await page.waitForFunction(() => {
 			const modalBody = document.querySelector('.modal.show .modal-body');
 			return modalBody instanceof HTMLElement && modalBody.innerText.includes('TASK ERROR');

tests/v1/jobs.spec.js

@@ -71,19 +71,26 @@ export class PageWithWorkflow extends PageWithProject {
 		await this.addUserTask('Fake Task');
 	}
 
-	async triggerTaskSuccess() {
-		await this.setTaskStatus('done');
+	/**
+	 * @param {number} jobId
+	 */
+	async triggerTaskSuccess(jobId) {
+		await this.setTaskStatus(jobId, 'done');
 	}
 
-	async triggerTaskFailure() {
-		await this.setTaskStatus('failed');
+	/**
+	 * @param {number} jobId
+	 */
+	async triggerTaskFailure(jobId) {
+		await this.setTaskStatus(jobId, 'failed');
 	}
 
 	/**
+	 * @param {number} jobId
 	 * @param {import('$lib/types.js').JobStatus} status
 	 */
-	async setTaskStatus(status) {
-		const response = await this.request.put(`http://localhost:8080/${status}`);
+	async setTaskStatus(jobId, status) {
+		const response = await this.request.put(`http://localhost:8080/v1/${jobId}?status=${status}`);
 		expect(response.ok()).toEqual(true);
 	}
 

tests/v1/workflow_fixture.js

@@ -95,7 +95,7 @@ test('Admin groups management', async ({ page }) => {
 		}
 		await modal.getByRole('button', { name: 'Add' }).click();
 		await waitModalClosed(page);
-		finalCount = selectableGroups.length + 2;
+		finalCount = selectableGroups1 + 2;
 		await expect(groupBadges).toHaveCount(finalCount);
 		await expect(page.getByRole('button', { name: 'Add group' })).not.toBeVisible();
 	});