refactor(core): Add more unit tests for Workflow.ts (no-changelog) (#9868)

Co-authored-by: Danny Martini <danny@n8n.io>

packages/workflow/src/Workflow.ts

@@ -63,6 +63,18 @@ function dedupe<T>(arr: T[]): T[] {
 	return [...new Set(arr)];
 }
 
+export interface WorkflowParameters {
+	id?: string;
+	name?: string;
+	nodes: INode[];
+	connections: IConnections;
+	active: boolean;
+	nodeTypes: INodeTypes;
+	staticData?: IDataObject;
+	settings?: IWorkflowSettings;
+	pinData?: IPinData;
+}
+
 export class Workflow {
 	id: string;
 
@@ -90,18 +102,7 @@ export class Workflow {
 
 	pinData?: IPinData;
 
-	// constructor(id: string | undefined, nodes: INode[], connections: IConnections, active: boolean, nodeTypes: INodeTypes, staticData?: IDataObject, settings?: IWorkflowSettings) {
+	constructor(parameters: WorkflowParameters) {
-	constructor(parameters: {
-		id?: string;
-		name?: string;
-		nodes: INode[];
-		connections: IConnections;
-		active: boolean;
-		nodeTypes: INodeTypes;
-		staticData?: IDataObject;
-		settings?: IWorkflowSettings;
-		pinData?: IPinData;
-	}) {
 		this.id = parameters.id as string; // @tech_debt Ensure this is not optional
 		this.name = parameters.name;
 		this.nodeTypes = parameters.nodeTypes;
@@ -251,16 +252,14 @@ export class Workflow {
 	 * is fine. If there are issues it returns the issues
 	 * which have been found for the different nodes.
 	 * TODO: Does currently not check for credential issues!
-	 *
 	 */
-	checkReadyForExecution(inputData: {
+	checkReadyForExecution(
+		inputData: {
 			startNode?: string;
 			destinationNode?: string;
 			pinDataNodeNames?: string[];
-	}): IWorkflowIssues | null {
+		} = {},
-		let node: INode;
+	): IWorkflowIssues | null {
-		let nodeType: INodeType | undefined;
-		let nodeIssues: INodeIssues | null = null;
 		const workflowIssues: IWorkflowIssues = {};
 
 		let checkNodes: string[] = [];
@@ -277,14 +276,14 @@ export class Workflow {
 		}
 
 		for (const nodeName of checkNodes) {
-			nodeIssues = null;
+			let nodeIssues: INodeIssues | null = null;
-			node = this.nodes[nodeName];
+			const node = this.nodes[nodeName];
 
 			if (node.disabled === true) {
 				continue;
 			}
 
-			nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
+			const nodeType = this.nodeTypes.getByNameAndVersion(node.type, node.typeVersion);
 
 			if (nodeType === undefined) {
 				// Node type is not known

packages/workflow/test/Workflow.test.ts

@@ -1,3 +1,5 @@
+import { mock } from 'jest-mock-extended';
+import { NodeConnectionType } from '@/Interfaces';
 import type {
 	IBinaryKeyData,
 	IConnections,
@@ -5,10 +7,14 @@ import type {
 	INode,
 	INodeExecutionData,
 	INodeParameters,
+	INodeType,
+	INodeTypeDescription,
+	INodeTypes,
 	IRunExecutionData,
 	NodeParameterValueType,
 } from '@/Interfaces';
-import { Workflow } from '@/Workflow';
+import { Workflow, type WorkflowParameters } from '@/Workflow';
+import * as NodeHelpers from '@/NodeHelpers';
 
 process.env.TEST_VARIABLE_1 = 'valueEnvVariable1';
 
@@ -20,7 +26,128 @@ interface StubNode {
 }
 
 describe('Workflow', () => {
-	describe('renameNodeInParameterValue for expressions', () => {
+	describe('checkIfWorkflowCanBeActivated', () => {
+		const disabledNode = mock<INode>({ type: 'triggerNode', disabled: true });
+		const unknownNode = mock<INode>({ type: 'unknownNode' });
+		const noTriggersNode = mock<INode>({ type: 'noTriggersNode' });
+		const pollNode = mock<INode>({ type: 'pollNode' });
+		const triggerNode = mock<INode>({ type: 'triggerNode' });
+		const webhookNode = mock<INode>({ type: 'webhookNode' });
+
+		const nodeTypes = mock<INodeTypes>();
+		nodeTypes.getByNameAndVersion.mockImplementation((type) => {
+			// TODO: getByNameAndVersion signature needs to be updated to allow returning undefined
+			if (type === 'unknownNode') return undefined as unknown as INodeType;
+			const partial: Partial<INodeType> = {
+				poll: undefined,
+				trigger: undefined,
+				webhook: undefined,
+				description: mock<INodeTypeDescription>({
+					properties: [],
+				}),
+			};
+			if (type === 'pollNode') partial.poll = jest.fn();
+			if (type === 'triggerNode') partial.trigger = jest.fn();
+			if (type === 'webhookNode') partial.webhook = jest.fn();
+			return mock(partial);
+		});
+
+		test.each([
+			['should skip disabled nodes', disabledNode, [], false],
+			['should skip nodes marked as ignored', triggerNode, ['triggerNode'], false],
+			['should skip unknown nodes', unknownNode, [], false],
+			['should skip nodes with no trigger method', noTriggersNode, [], false],
+			['should activate if poll method exists', pollNode, [], true],
+			['should activate if trigger method exists', triggerNode, [], true],
+			['should activate if webhook method exists', webhookNode, [], true],
+		])('%s', async (_, node, ignoredNodes, expected) => {
+			const params = mock<WorkflowParameters>({ nodeTypes });
+			params.nodes = [node];
+			const workflow = new Workflow(params);
+			expect(workflow.checkIfWorkflowCanBeActivated(ignoredNodes)).toBe(expected);
+		});
+	});
+
+	describe('checkReadyForExecution', () => {
+		const disabledNode = mock<INode>({ name: 'Disabled Node', disabled: true });
+		const startNode = mock<INode>({ name: 'Start Node' });
+		const unknownNode = mock<INode>({ name: 'Unknown Node', type: 'unknownNode' });
+
+		const nodeParamIssuesSpy = jest.spyOn(NodeHelpers, 'getNodeParametersIssues');
+
+		const nodeTypes = mock<INodeTypes>();
+		nodeTypes.getByNameAndVersion.mockImplementation((type) => {
+			// TODO: getByNameAndVersion signature needs to be updated to allow returning undefined
+			if (type === 'unknownNode') return undefined as unknown as INodeType;
+			return mock<INodeType>({
+				description: {
+					properties: [],
+				},
+			});
+		});
+
+		beforeEach(() => jest.clearAllMocks());
+
+		it('should return null if there are no nodes', () => {
+			const workflow = new Workflow({
+				nodes: [],
+				connections: {},
+				active: false,
+				nodeTypes,
+			});
+
+			const issues = workflow.checkReadyForExecution();
+			expect(issues).toBe(null);
+			expect(nodeTypes.getByNameAndVersion).not.toHaveBeenCalled();
+			expect(nodeParamIssuesSpy).not.toHaveBeenCalled();
+		});
+
+		it('should return null if there are no enabled nodes', () => {
+			const workflow = new Workflow({
+				nodes: [disabledNode],
+				connections: {},
+				active: false,
+				nodeTypes,
+			});
+
+			const issues = workflow.checkReadyForExecution({ startNode: disabledNode.name });
+			expect(issues).toBe(null);
+			expect(nodeTypes.getByNameAndVersion).toHaveBeenCalledTimes(1);
+			expect(nodeParamIssuesSpy).not.toHaveBeenCalled();
+		});
+
+		it('should return typeUnknown for unknown nodes', () => {
+			const workflow = new Workflow({
+				nodes: [unknownNode],
+				connections: {},
+				active: false,
+				nodeTypes,
+			});
+
+			const issues = workflow.checkReadyForExecution({ startNode: unknownNode.name });
+			expect(issues).toEqual({ [unknownNode.name]: { typeUnknown: true } });
+			expect(nodeTypes.getByNameAndVersion).toHaveBeenCalledTimes(2);
+			expect(nodeParamIssuesSpy).not.toHaveBeenCalled();
+		});
+
+		it('should return issues for regular nodes', () => {
+			const workflow = new Workflow({
+				nodes: [startNode],
+				connections: {},
+				active: false,
+				nodeTypes,
+			});
+			nodeParamIssuesSpy.mockReturnValue({ execution: false });
+
+			const issues = workflow.checkReadyForExecution({ startNode: startNode.name });
+			expect(issues).toEqual({ [startNode.name]: { execution: false } });
+			expect(nodeTypes.getByNameAndVersion).toHaveBeenCalledTimes(2);
+			expect(nodeParamIssuesSpy).toHaveBeenCalled();
+		});
+	});
+
+	describe('renameNodeInParameterValue', () => {
+		describe('for expressions', () => {
 			const tests = [
 				{
 					description: 'do nothing if there is no expression',
@@ -81,7 +208,8 @@ describe('Workflow', () => {
 					},
 					output: {
 						value1: '={{$("NewName")["data"]["value1"] + \'Node1\'}}',
-					value2: '={{$("NewName")["data"]["value2"] + \' - \' + $("NewName")["data"]["value2"]}}',
+						value2:
+							'={{$("NewName")["data"]["value2"] + \' - \' + $("NewName")["data"]["value2"]}}',
 					},
 				},
 				{
@@ -267,7 +395,7 @@ describe('Workflow', () => {
 			}
 		});
 
-	describe('renameNodeInParameterValue for node with renamable content', () => {
+		describe('for node with renamable content', () => {
 			const tests = [
 				{
 					description: "should work with $('name')",
@@ -318,6 +446,7 @@ describe('Workflow', () => {
 				});
 			}
 		});
+	});
 
 	describe('renameNode', () => {
 		const tests = [
@@ -377,7 +506,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node2',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -408,7 +537,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node2',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -444,7 +573,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node2',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -475,7 +604,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node2New',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -532,7 +661,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node3',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -543,12 +672,12 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node3',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 									{
 										node: 'Node5',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -559,12 +688,12 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node4',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 									{
 										node: 'Node5',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -616,7 +745,7 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node3New',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -627,12 +756,12 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node3New',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 									{
 										node: 'Node5',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -643,12 +772,12 @@ describe('Workflow', () => {
 								[
 									{
 										node: 'Node4',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 									{
 										node: 'Node5',
-										type: 'main',
+										type: NodeConnectionType.Main,
 										index: 0,
 									},
 								],
@@ -1229,7 +1358,7 @@ describe('Workflow', () => {
 							[
 								{
 									node: 'Node2',
-									type: 'main',
+									type: NodeConnectionType.Main,
 									index: 0,
 								},
 							],
@@ -1240,7 +1369,7 @@ describe('Workflow', () => {
 							[
 								{
 									node: 'Node3',
-									type: 'main',
+									type: NodeConnectionType.Main,
 									index: 0,
 								},
 							],
@@ -1251,7 +1380,7 @@ describe('Workflow', () => {
 							[
 								{
 									node: 'Node2',
-									type: 'main',
+									type: NodeConnectionType.Main,
 									index: 0,
 								},
 							],
@@ -1521,7 +1650,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1532,7 +1661,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set1',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1591,21 +1720,21 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set1',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
 						[
 							{
 								node: 'Set',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
 						[
 							{
 								node: 'Set',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1616,7 +1745,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set2',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1627,7 +1756,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set2',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1691,7 +1820,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1699,7 +1828,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Switch',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1710,7 +1839,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set1',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1721,12 +1850,12 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set1',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 							{
 								node: 'Switch',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],
@@ -1737,7 +1866,7 @@ describe('Workflow', () => {
 						[
 							{
 								node: 'Set1',
-								type: 'main',
+								type: NodeConnectionType.Main,
 								index: 0,
 							},
 						],