diff --git a/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts b/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts index 516be4aec1..b9999f02b7 100644 --- a/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts +++ b/packages/core/src/execution-engine/__tests__/workflow-execute.test.ts @@ -191,6 +191,160 @@ describe('WorkflowExecute', () => { } }); + describe('v0 hook order', () => { + const executionMode = 'manual'; + const executionOrder = 'v0'; + const nodeTypes = Helpers.NodeTypes(); + + test("don't run hooks for siblings of the destination node", async () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger', type: 'n8n-nodes-base.manualTrigger' }); + const node1 = createNodeData({ name: 'node1' }); + const node2 = createNodeData({ name: 'node2' }); + const workflowInstance = new DirectedGraph() + .addNodes(trigger, node1, node2) + .addConnections({ from: trigger, to: node1 }, { from: trigger, to: node2 }) + .toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder } }); + + const additionalData = Helpers.WorkflowExecuteAdditionalData(createDeferredPromise()); + const runHookSpy = jest.spyOn(additionalData.hooks!, 'runHook'); + + const workflowExecute = new WorkflowExecute(additionalData, executionMode); + + // ACT + await workflowExecute.run(workflowInstance, trigger, 'node1'); + + // ASSERT + const workflowHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'workflowExecuteBefore' || call[0] === 'workflowExecuteAfter', + ); + const nodeHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'nodeExecuteBefore' || call[0] === 'nodeExecuteAfter', + ); + + expect(workflowHooks.map((hook) => hook[0])).toEqual([ + 'workflowExecuteBefore', + 'workflowExecuteAfter', + ]); + + expect(nodeHooks.map((hook) => ({ name: hook[0], node: hook[1][0] }))).toEqual([ + { name: 'nodeExecuteBefore', node: 'trigger' }, + { name: 'nodeExecuteAfter', node: 'trigger' }, + { name: 'nodeExecuteBefore', node: 'node1' }, + { name: 'nodeExecuteAfter', node: 'node1' }, + ]); + }); + + test("don't run hooks if a node does not have input data", async () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger', type: 'n8n-nodes-base.manualTrigger' }); + const workflowInstance = new DirectedGraph() + .addNodes(trigger) + .toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder } }); + + const additionalData = Helpers.WorkflowExecuteAdditionalData(createDeferredPromise()); + const runHookSpy = jest.spyOn(additionalData.hooks!, 'runHook'); + + const workflowExecute = new WorkflowExecute(additionalData, executionMode); + jest.spyOn(workflowExecute, 'ensureInputData').mockReturnValue(false); + + // ACT + await workflowExecute.run(workflowInstance, trigger); + + // ASSERT + const workflowHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'workflowExecuteBefore' || call[0] === 'workflowExecuteAfter', + ); + const nodeHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'nodeExecuteBefore' || call[0] === 'nodeExecuteAfter', + ); + + expect(workflowHooks.map((hook) => hook[0])).toEqual([ + 'workflowExecuteBefore', + 'workflowExecuteAfter', + ]); + + expect(nodeHooks).toHaveLength(0); + }); + }); + + describe('v1 hook order', () => { + const executionMode = 'manual'; + const executionOrder = 'v1'; + const nodeTypes = Helpers.NodeTypes(); + + test("don't run hooks for siblings of the destination node", async () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger', type: 'n8n-nodes-base.manualTrigger' }); + const node1 = createNodeData({ name: 'node1' }); + const node2 = createNodeData({ name: 'node2' }); + const workflowInstance = new DirectedGraph() + .addNodes(trigger, node1, node2) + .addConnections({ from: trigger, to: node1 }, { from: trigger, to: node2 }) + .toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder } }); + + const additionalData = Helpers.WorkflowExecuteAdditionalData(createDeferredPromise()); + const runHookSpy = jest.spyOn(additionalData.hooks!, 'runHook'); + + const workflowExecute = new WorkflowExecute(additionalData, executionMode); + + // ACT + await workflowExecute.run(workflowInstance, trigger, 'node1'); + + // ASSERT + const workflowHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'workflowExecuteBefore' || call[0] === 'workflowExecuteAfter', + ); + const nodeHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'nodeExecuteBefore' || call[0] === 'nodeExecuteAfter', + ); + + expect(workflowHooks.map((hook) => hook[0])).toEqual([ + 'workflowExecuteBefore', + 'workflowExecuteAfter', + ]); + + expect(nodeHooks.map((hook) => ({ name: hook[0], node: hook[1][0] }))).toEqual([ + { name: 'nodeExecuteBefore', node: 'trigger' }, + { name: 'nodeExecuteAfter', node: 'trigger' }, + { name: 'nodeExecuteBefore', node: 'node1' }, + { name: 'nodeExecuteAfter', node: 'node1' }, + ]); + }); + + test("don't run hooks if a node does not have input data", async () => { + // ARRANGE + const trigger = createNodeData({ name: 'trigger', type: 'n8n-nodes-base.manualTrigger' }); + const workflowInstance = new DirectedGraph() + .addNodes(trigger) + .toWorkflow({ name: '', active: false, nodeTypes, settings: { executionOrder } }); + + const additionalData = Helpers.WorkflowExecuteAdditionalData(createDeferredPromise()); + const runHookSpy = jest.spyOn(additionalData.hooks!, 'runHook'); + + const workflowExecute = new WorkflowExecute(additionalData, executionMode); + jest.spyOn(workflowExecute, 'ensureInputData').mockReturnValue(false); + + // ACT + await workflowExecute.run(workflowInstance, trigger); + + // ASSERT + const workflowHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'workflowExecuteBefore' || call[0] === 'workflowExecuteAfter', + ); + const nodeHooks = runHookSpy.mock.calls.filter( + (call) => call[0] === 'nodeExecuteBefore' || call[0] === 'nodeExecuteAfter', + ); + + expect(workflowHooks.map((hook) => hook[0])).toEqual([ + 'workflowExecuteBefore', + 'workflowExecuteAfter', + ]); + + expect(nodeHooks).toHaveLength(0); + }); + }); + //run tests on json files from specified directory, default 'workflows' //workflows must have pinned data that would be used to test output after execution describe('run test workflows', () => { diff --git a/packages/core/src/execution-engine/workflow-execute.ts b/packages/core/src/execution-engine/workflow-execute.ts index 3a75ffdf4c..fed897536c 100644 --- a/packages/core/src/execution-engine/workflow-execute.ts +++ b/packages/core/src/execution-engine/workflow-execute.ts @@ -1433,20 +1433,12 @@ export class WorkflowExecute { } executionData.data = newTaskDataConnections; - Logger.debug(`Start processing node "${executionNode.name}"`, { - node: executionNode.name, - workflowId: workflow.id, - }); - await hooks.runHook('nodeExecuteBefore', [executionNode.name, taskStartedData]); - // Get the index of the current run runIndex = 0; if (this.runExecutionData.resultData.runData.hasOwnProperty(executionNode.name)) { runIndex = this.runExecutionData.resultData.runData[executionNode.name].length; } - currentExecutionTry = `${executionNode.name}:${runIndex}`; - if (currentExecutionTry === lastExecutionTry) { throw new ApplicationError( 'Stopped execution because it seems to be in an endless loop', @@ -1469,6 +1461,11 @@ export class WorkflowExecute { continue executionLoop; } + Logger.debug(`Start executing node "${executionNode.name}"`, { + node: executionNode.name, + workflowId: workflow.id, + }); + await hooks.runHook('nodeExecuteBefore', [executionNode.name, taskStartedData]); let maxTries = 1; if (executionData.node.retryOnFail === true) { // TODO: Remove the hardcoded default-values here and also in NodeSettings.vue diff --git a/packages/core/test/helpers/index.ts b/packages/core/test/helpers/index.ts index 8f00694b24..a8e31e87b3 100644 --- a/packages/core/test/helpers/index.ts +++ b/packages/core/test/helpers/index.ts @@ -54,7 +54,15 @@ export function WorkflowExecuteAdditionalData( ): IWorkflowExecuteAdditionalData { const hooks = new ExecutionLifecycleHooks('trigger', '1', mock()); hooks.addHandler('workflowExecuteAfter', (fullRunData) => waitPromise.resolve(fullRunData)); - return mock({ hooks, currentNodeExecutionIndex: 0 }); + return mock({ + hooks, + currentNodeExecutionIndex: 0, + // Not setting this to undefined would set it to a mock which would trigger + // conditions in the WorkflowExecute which only check if a property exists, + // e.g. `if (!this.additionalData.restartExecutionId)`. This would for + // example skip running the `workflowExecuteBefore` hook in the tests. + restartExecutionId: undefined, + }); } const preparePinData = (pinData: IDataObject) => {