tests: Fix flakyness of service install task expectations

The hypothesis is that b/c the tasks are updated by a different write TX, there is a race condition between waiting for the service installs to be created and then expected for the task to have already been marked as successful. Change-type: patch
balena-io · Nov 28, 2024 · 9cbe2f0 · 9cbe2f0
1 parent 0c738a4
commit 9cbe2f0
Show file tree

Hide file tree

Showing 3 changed files with 55 additions and 21 deletions.
diff --git a/test/25_service-installs.ts b/test/25_service-installs.ts
@@ -8,7 +8,10 @@ import * as fixtures from './test-lib/fixtures.js';
 import { assertExists, expectToEventually } from './test-lib/common.js';
 import * as config from '../src/lib/config.js';
 import { supertest } from './test-lib/supertest.js';
-import { expectNewTasks, resetLatestTaskIds } from './test-lib/api-helpers.js';
+import {
+	expectNewSettledTasks,
+	resetLatestTaskIds,
+} from './test-lib/api-helpers.js';
 
 export default () => {
 	versions.test((version, pineTest) => {
@@ -77,7 +80,7 @@ export default () => {
 							ctx.app1Service1.id,
 						);
 					});
-					await expectNewTasks(
+					await expectNewSettledTasks(
 						'create_service_installs',
 						isServiceInstallEnabled
 							? [
@@ -120,7 +123,7 @@ export default () => {
 							ctx.app1Service2.id,
 						);
 					});
-					await expectNewTasks(
+					await expectNewSettledTasks(
 						'create_service_installs',
 						isServiceInstallEnabled
 							? [
@@ -162,7 +165,7 @@ export default () => {
 							ctx.app1Service3.id,
 						);
 					});
-					await expectNewTasks(
+					await expectNewSettledTasks(
 						'create_service_installs',
 						isServiceInstallEnabled
 							? [
@@ -188,6 +191,20 @@ export default () => {
 						})
 						.expect(200);
 
+					await expectNewSettledTasks(
+						'create_service_installs',
+						isServiceInstallEnabled
+							? [
+									{
+										is_executed_with__parameter_set: {
+											devices: [ctx.device.id],
+										},
+										status: 'succeeded',
+									},
+								]
+							: [],
+					);
+
 					await pineUser
 						.patch({
 							resource: 'device',
@@ -212,18 +229,10 @@ export default () => {
 							ctx.app2Service1.id,
 						);
 					});
-					await expectNewTasks(
+					await expectNewSettledTasks(
 						'create_service_installs',
 						isServiceInstallEnabled
 							? [
-									// the first one is from unpinning
-									{
-										is_executed_with__parameter_set: {
-											devices: [ctx.device.id],
-										},
-										status: 'succeeded',
-									},
-									// the second one is from moving application
 									{
 										is_executed_with__parameter_set: {
 											devices: [ctx.device.id],

diff --git a/test/test-lib/api-helpers.ts b/test/test-lib/api-helpers.ts
@@ -269,11 +269,11 @@ const latestTaskIdByHandler: Dictionary<string | undefined> = {};
 
 export const expectNewTasks = async (
 	handler: string,
-	expectedUsage: TaskExpectation[],
+	expectedTasks: TaskExpectation[],
 ) => {
 	const tasks = await expectTasks(
 		handler,
-		expectedUsage,
+		expectedTasks,
 		latestTaskIdByHandler[handler],
 	);
 	const lastTaskId = tasks.at(-1)?.id;
@@ -282,7 +282,15 @@ export const expectNewTasks = async (
 	}
 };
 
-export const resetLatestTaskIds = async (handler: string) => {
+export const expectNewSettledTasks = async (
+	handler: string,
+	expectedTasks: TaskExpectation[],
+) => {
+	await waitUntilTasksFinish(handler);
+	await expectNewTasks(handler, expectedTasks);
+};
+
+const waitUntilTasksFinish = async function (handler: string) {
 	await expectToEventually(async () => {
 		const runningTasks = await sbvrUtils.api.tasks.get({
 			resource: 'task',
@@ -297,8 +305,15 @@ export const resetLatestTaskIds = async (handler: string) => {
 				$orderby: { id: 'desc' },
 			},
 		});
-		expect(runningTasks).to.have.lengthOf(0);
+		expect(
+			runningTasks,
+			`Found still queued '${handler}' tasks`,
+		).to.have.lengthOf(0);
 	});
+};
+
+export const resetLatestTaskIds = async (handler: string) => {
+	await waitUntilTasksFinish(handler);
 	const [latestTask] = await sbvrUtils.api.tasks.get({
 		resource: 'task',
 		passthrough: { req: permissions.rootRead },

diff --git a/test/test-lib/common.ts b/test/test-lib/common.ts
@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import { stripIndent } from 'common-tags';
 import { setTimeout } from 'timers/promises';
 import { TypedError } from 'typed-error';
+import { ThisShouldNeverHappenError } from '../../src/infra/error-handling/index.js';
 
 type PredicateFunction = () => Resolvable<boolean>;
 
@@ -107,15 +108,24 @@ export async function expectToEventually<T>(
 	attempts = 20,
 	interval = 100,
 ): Promise<T> {
+	let error: Error | undefined;
 	for (let i = 0; i < attempts; i++) {
 		try {
 			return await fn();
-
-			// eslint-disable-next-line @typescript-eslint/no-unused-vars -- So that the interface is already well defined.
-		} catch (_e) {
+		} catch (e) {
+			if (!(e instanceof Error)) {
+				throw ThisShouldNeverHappenError(
+					'Thrown error is not an instanceof Error',
+				);
+			}
+			error = e;
 			console.log(`⌚  Waiting ${interval}ms (${i}/${attempts})...`);
 			await setTimeout(interval);
 		}
 	}
-	throw new Error('Expectation failed');
+	if (error == null) {
+		// error should always be assigned
+		throw ThisShouldNeverHappenError('Unexpected test error');
+	}
+	throw new Error(`Expectation failed: ${error.message}`, { cause: error });
 }