test(core): Move unit tests closer to testable components (no-changelog) (#10287)

2024-08-05 12:12:25 +03:00
parent 8131d66f8c
commit afa43e75f6
80 changed files with 95 additions and 105 deletions
--- a/packages/cli/src/executionLifecycleHooks/tests/restoreBinaryDataId.test.ts
+++ b/packages/cli/src/executionLifecycleHooks/tests/restoreBinaryDataId.test.ts
@@ -0,0 +1,192 @@
+import { restoreBinaryDataId } from '@/executionLifecycleHooks/restoreBinaryDataId';
+import { BinaryDataService } from 'n8n-core';
+import { mockInstance } from '@test/mocking';
+
+import type { IRun } from 'n8n-workflow';
+import config from '@/config';
+
+function toIRun(item?: object) {
+	return {
+		data: {
+			resultData: {
+				runData: {
+					myNode: [
+						{
+							data: {
+								main: [[item]],
+							},
+						},
+					],
+				},
+			},
+		},
+	} as unknown as IRun;
+}
+
+function getDataId(run: IRun, kind: 'binary' | 'json') {
+	// @ts-expect-error The type doesn't have the correct structure
+	return run.data.resultData.runData.myNode[0].data.main[0][0][kind].data.id;
+}
+
+const binaryDataService = mockInstance(BinaryDataService);
+
+for (const mode of ['filesystem-v2', 's3'] as const) {
+	describe(`on ${mode} mode`, () => {
+		beforeAll(() => {
+			config.set('binaryDataManager.mode', mode);
+		});
+
+		afterEach(() => {
+			jest.clearAllMocks();
+		});
+
+		it('should restore if binary data ID is missing execution ID', async () => {
+			const workflowId = '6HYhhKmJch2cYxGj';
+			const executionId = 'temp';
+			const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6';
+
+			const incorrectFileId = `workflows/${workflowId}/executions/temp/binary_data/${binaryDataFileUuid}`;
+
+			const run = toIRun({
+				binary: {
+					data: { id: `s3:${incorrectFileId}` },
+				},
+			});
+
+			await restoreBinaryDataId(run, executionId, 'webhook');
+
+			const correctFileId = incorrectFileId.replace('temp', executionId);
+			const correctBinaryDataId = `s3:${correctFileId}`;
+
+			expect(binaryDataService.rename).toHaveBeenCalledWith(incorrectFileId, correctFileId);
+			expect(getDataId(run, 'binary')).toBe(correctBinaryDataId);
+		});
+
+		it('should do nothing if binary data ID is not missing execution ID', async () => {
+			const workflowId = '6HYhhKmJch2cYxGj';
+			const executionId = '999';
+			const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6';
+
+			const fileId = `workflows/${workflowId}/executions/${executionId}/binary_data/${binaryDataFileUuid}`;
+
+			const binaryDataId = `s3:${fileId}`;
+
+			const run = toIRun({
+				binary: {
+					data: {
+						id: binaryDataId,
+					},
+				},
+			});
+
+			await restoreBinaryDataId(run, executionId, 'webhook');
+
+			expect(binaryDataService.rename).not.toHaveBeenCalled();
+			expect(getDataId(run, 'binary')).toBe(binaryDataId);
+		});
+
+		it('should do nothing if no binary data ID', async () => {
+			const executionId = '999';
+			const dataId = '123';
+
+			const run = toIRun({
+				json: {
+					data: { id: dataId },
+				},
+			});
+
+			await restoreBinaryDataId(run, executionId, 'webhook');
+
+			expect(binaryDataService.rename).not.toHaveBeenCalled();
+			expect(getDataId(run, 'json')).toBe(dataId);
+		});
+
+		it('should do nothing on itemless case', async () => {
+			const executionId = '999';
+
+			const promise = restoreBinaryDataId(toIRun(), executionId, 'webhook');
+
+			await expect(promise).resolves.not.toThrow();
+
+			expect(binaryDataService.rename).not.toHaveBeenCalled();
+		});
+
+		it('should do nothing if data is undefined', async () => {
+			const executionId = '999';
+
+			const run = toIRun({
+				json: {
+					data: undefined,
+				},
+			});
+
+			const promise = restoreBinaryDataId(run, executionId, 'webhook');
+
+			await expect(promise).resolves.not.toThrow();
+
+			expect(binaryDataService.rename).not.toHaveBeenCalled();
+		});
+
+		it('should do nothing if workflow execution mode is not `webhook`', async () => {
+			const executionId = '999';
+
+			const run = toIRun({
+				json: {
+					data: undefined,
+				},
+			});
+
+			const promise = restoreBinaryDataId(run, executionId, 'internal');
+
+			await expect(promise).resolves.not.toThrow();
+
+			expect(binaryDataService.rename).not.toHaveBeenCalled();
+		});
+
+		it('should ignore error thrown on renaming', async () => {
+			const workflowId = '6HYhhKmJch2cYxGj';
+			const executionId = 'temp';
+			const binaryDataFileUuid = 'a5c3f1ed-9d59-4155-bc68-9a370b3c51f6';
+
+			const incorrectFileId = `workflows/${workflowId}/executions/temp/binary_data/${binaryDataFileUuid}`;
+
+			const run = toIRun({
+				binary: {
+					data: { id: `s3:${incorrectFileId}` },
+				},
+			});
+
+			binaryDataService.rename.mockRejectedValueOnce(new Error('ENOENT'));
+
+			const promise = restoreBinaryDataId(run, executionId, 'webhook');
+
+			await expect(promise).resolves.not.toThrow();
+
+			expect(binaryDataService.rename).toHaveBeenCalled();
+		});
+	});
+}
+
+describe('on default mode', () => {
+	afterEach(() => {
+		config.load(config.default);
+	});
+
+	it('should do nothing', async () => {
+		config.set('binaryDataManager.mode', 'default');
+
+		const executionId = '999';
+
+		const run = toIRun({
+			json: {
+				data: undefined,
+			},
+		});
+
+		const promise = restoreBinaryDataId(run, executionId, 'internal');
+
+		await expect(promise).resolves.not.toThrow();
+
+		expect(binaryDataService.rename).not.toHaveBeenCalled();
+	});
+});
--- a/packages/cli/src/executionLifecycleHooks/tests/saveExecutionProgress.test.ts
+++ b/packages/cli/src/executionLifecycleHooks/tests/saveExecutionProgress.test.ts
@@ -0,0 +1,106 @@
+import { ExecutionRepository } from '@/databases/repositories/execution.repository';
+import { mockInstance } from '@test/mocking';
+import { Logger } from '@/Logger';
+import { saveExecutionProgress } from '@/executionLifecycleHooks/saveExecutionProgress';
+import * as fnModule from '@/executionLifecycleHooks/toSaveSettings';
+import {
+	ErrorReporterProxy,
+	type IRunExecutionData,
+	type ITaskData,
+	type IWorkflowBase,
+} from 'n8n-workflow';
+import type { IExecutionResponse } from '@/Interfaces';
+
+mockInstance(Logger);
+
+const executionRepository = mockInstance(ExecutionRepository);
+
+afterEach(() => {
+	jest.clearAllMocks();
+});
+
+const commonArgs: [IWorkflowBase, string, string, ITaskData, IRunExecutionData, string] = [
+	{} as IWorkflowBase,
+	'some-execution-id',
+	'My Node',
+	{} as ITaskData,
+	{} as IRunExecutionData,
+	'some-session-id',
+];
+
+const commonSettings = { error: true, success: true, manual: true };
+
+test('should ignore if save settings say so', async () => {
+	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
+		...commonSettings,
+		progress: false,
+	});
+
+	await saveExecutionProgress(...commonArgs);
+
+	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
+});
+
+test('should ignore on leftover async call', async () => {
+	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
+		...commonSettings,
+		progress: true,
+	});
+
+	executionRepository.findSingleExecution.mockResolvedValue({
+		finished: true,
+	} as IExecutionResponse);
+
+	await saveExecutionProgress(...commonArgs);
+
+	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
+});
+
+test('should update execution', async () => {
+	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
+		...commonSettings,
+		progress: true,
+	});
+
+	const reporterSpy = jest.spyOn(ErrorReporterProxy, 'error');
+
+	executionRepository.findSingleExecution.mockResolvedValue({} as IExecutionResponse);
+
+	await saveExecutionProgress(...commonArgs);
+
+	expect(executionRepository.updateExistingExecution).toHaveBeenCalledWith('some-execution-id', {
+		data: {
+			executionData: undefined,
+			resultData: {
+				lastNodeExecuted: 'My Node',
+				runData: {
+					'My Node': [{}],
+				},
+			},
+			startData: {},
+		},
+		status: 'running',
+	});
+
+	expect(reporterSpy).not.toHaveBeenCalled();
+});
+
+test('should report error on failure', async () => {
+	jest.spyOn(fnModule, 'toSaveSettings').mockReturnValue({
+		...commonSettings,
+		progress: true,
+	});
+
+	const reporterSpy = jest.spyOn(ErrorReporterProxy, 'error');
+
+	const error = new Error('Something went wrong');
+
+	executionRepository.findSingleExecution.mockImplementation(() => {
+		throw error;
+	});
+
+	await saveExecutionProgress(...commonArgs);
+
+	expect(executionRepository.updateExistingExecution).not.toHaveBeenCalled();
+	expect(reporterSpy).toHaveBeenCalledWith(error);
+});
--- a/packages/cli/src/executionLifecycleHooks/tests/toSaveSettings.test.ts
+++ b/packages/cli/src/executionLifecycleHooks/tests/toSaveSettings.test.ts
@@ -0,0 +1,154 @@
+import config from '@/config';
+import { toSaveSettings } from '@/executionLifecycleHooks/toSaveSettings';
+
+afterEach(() => {
+	config.load(config.default);
+});
+
+describe('failed production executions', () => {
+	it('should favor workflow settings over defaults', () => {
+		config.set('executions.saveDataOnError', 'none');
+
+		const saveSettings = toSaveSettings({ saveDataErrorExecution: 'all' });
+
+		expect(saveSettings.error).toBe(true);
+
+		config.set('executions.saveDataOnError', 'all');
+
+		const _saveSettings = toSaveSettings({ saveDataErrorExecution: 'none' });
+
+		expect(_saveSettings.error).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataOnError', 'all');
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.error).toBe(true);
+
+		config.set('executions.saveDataOnError', 'none');
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.error).toBe(false);
+	});
+});
+
+describe('successful production executions', () => {
+	it('should favor workflow settings over defaults', () => {
+		config.set('executions.saveDataOnSuccess', 'none');
+
+		const saveSettings = toSaveSettings({ saveDataSuccessExecution: 'all' });
+
+		expect(saveSettings.success).toBe(true);
+
+		config.set('executions.saveDataOnSuccess', 'all');
+
+		const _saveSettings = toSaveSettings({ saveDataSuccessExecution: 'none' });
+
+		expect(_saveSettings.success).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataOnSuccess', 'all');
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.success).toBe(true);
+
+		config.set('executions.saveDataOnSuccess', 'none');
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.success).toBe(false);
+	});
+});
+
+describe('manual executions', () => {
+	it('should favor workflow setting over default', () => {
+		config.set('executions.saveDataManualExecutions', false);
+
+		const saveSettings = toSaveSettings({ saveManualExecutions: true });
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', true);
+
+		const _saveSettings = toSaveSettings({ saveManualExecutions: false });
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+
+	it('should favor fall back to default if workflow setting is explicit default', () => {
+		config.set('executions.saveDataManualExecutions', true);
+
+		const saveSettings = toSaveSettings({ saveManualExecutions: 'DEFAULT' });
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', false);
+
+		const _saveSettings = toSaveSettings({ saveManualExecutions: 'DEFAULT' });
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveDataManualExecutions', true);
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.manual).toBe(true);
+
+		config.set('executions.saveDataManualExecutions', false);
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.manual).toBe(false);
+	});
+});
+
+describe('execution progress', () => {
+	it('should favor workflow setting over default', () => {
+		config.set('executions.saveExecutionProgress', false);
+
+		const saveSettings = toSaveSettings({ saveExecutionProgress: true });
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', true);
+
+		const _saveSettings = toSaveSettings({ saveExecutionProgress: false });
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+
+	it('should favor fall back to default if workflow setting is explicit default', () => {
+		config.set('executions.saveExecutionProgress', true);
+
+		const saveSettings = toSaveSettings({ saveExecutionProgress: 'DEFAULT' });
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', false);
+
+		const _saveSettings = toSaveSettings({ saveExecutionProgress: 'DEFAULT' });
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+
+	it('should fall back to default if no workflow setting', () => {
+		config.set('executions.saveExecutionProgress', true);
+
+		const saveSettings = toSaveSettings();
+
+		expect(saveSettings.progress).toBe(true);
+
+		config.set('executions.saveExecutionProgress', false);
+
+		const _saveSettings = toSaveSettings();
+
+		expect(_saveSettings.progress).toBe(false);
+	});
+});