Skip to content

Commit

Permalink
refactor(core): Consolidate execution lifecycle hooks even more + add…
Browse files Browse the repository at this point in the history 
…itional tests (#12898)
  • Loading branch information
@netroy
netroy authored Feb 4, 2025
1 parent 9bcbc2c commit 65ec6ae
Show file tree
Hide file tree
Showing 13 changed files with 356 additions and 302 deletions.
2 changes: 1 addition & 1 deletion packages/@n8n/api-types/src/scaling.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type RunningJobSummary = {
workflowName: string;
mode: WorkflowExecuteMode;
startedAt: Date;
retryOf: string;
retryOf?: string;
status: ExecutionStatus;
};

Expand Down
6 changes: 2 additions & 4 deletions packages/cli/src/active-executions.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,7 @@ export class ActiveExecutions {
workflowId: executionData.workflowData.id,
};

if (executionData.retryOf !== undefined) {
fullExecutionData.retryOf = executionData.retryOf.toString();
}
fullExecutionData.retryOf = executionData.retryOf ?? undefined;

const workflowId = executionData.workflowData.id;
if (workflowId !== undefined && isWorkflowIdValid(workflowId)) {
Expand Down Expand Up @@ -183,7 +181,7 @@ export class ActiveExecutions {
data = this.activeExecutions[id];
returnData.push({
id,
retryOf: data.executionData.retryOf,
retryOf: data.executionData.retryOf ?? undefined,
startedAt: data.startedAt,
mode: data.executionData.executionMode,
workflowId: data.executionData.workflowData.id,
Expand Down
125 changes: 80 additions & 45 deletions packages/cli/src/execution-lifecycle/__tests__/execution-lifecycle-hooks.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,35 +159,77 @@ describe('Execution Lifecycle Hooks', () => {
});
};

describe('getWorkflowHooksMain', () => {
beforeEach(() => {
hooks = getWorkflowHooksMain(
{
const externalHooksTests = () => {
describe('workflowExecuteBefore', () => {
it('should run workflow.preExecute hook', async () => {
await hooks.executeHookFunctions('workflowExecuteBefore', [workflow, runExecutionData]);

expect(externalHooks.run).toHaveBeenCalledWith('workflow.preExecute', [
workflow,
executionMode,
]);
});
});

describe('workflowExecuteAfter', () => {
it('should run workflow.postExecute hook', async () => {
await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]);

expect(externalHooks.run).toHaveBeenCalledWith('workflow.postExecute', [
successfulRun,
workflowData,
pushRef,
retryOf,
},
executionId,
);
executionId,
]);
});
});
};

const statisticsTests = () => {
describe('statistics events', () => {
it('workflowExecuteAfter should emit workflowExecutionCompleted statistics event', async () => {
await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]);

expect(workflowStatisticsService.emit).toHaveBeenCalledWith('workflowExecutionCompleted', {
workflowData,
fullRunData: successfulRun,
});
});

it('nodeFetchedData should handle nodeFetchedData statistics event', async () => {
await hooks.executeHookFunctions('nodeFetchedData', [workflowId, node]);

expect(workflowStatisticsService.emit).toHaveBeenCalledWith('nodeFetchedData', {
workflowId,
node,
});
});
});
};

describe('getWorkflowHooksMain', () => {
const createHooks = () =>
getWorkflowHooksMain({ executionMode, workflowData, pushRef, retryOf }, executionId);

beforeEach(() => {
hooks = createHooks();
});

workflowEventTests();
nodeEventsTests();
externalHooksTests();
statisticsTests();

it('should setup the correct set of hooks', () => {
expect(hooks).toBeInstanceOf(WorkflowHooks);
expect(hooks.mode).toBe('manual');
expect(hooks.executionId).toBe(executionId);
expect(hooks.workflowData).toEqual(workflowData);
expect(hooks.pushRef).toEqual('test-push-ref');
expect(hooks.retryOf).toEqual('test-retry-of');

const { hookFunctions } = hooks;
expect(hookFunctions.nodeExecuteBefore).toHaveLength(2);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(3);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(2);
expect(hookFunctions.workflowExecuteBefore).toHaveLength(3);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(4);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(5);
expect(hookFunctions.nodeFetchedData).toHaveLength(1);
expect(hookFunctions.sendResponse).toHaveLength(0);
});
Expand Down Expand Up @@ -219,6 +261,9 @@ describe('Execution Lifecycle Hooks', () => {

it('should save execution progress when enabled', async () => {
workflowData.settings = { saveExecutionProgress: true };
hooks = createHooks();

expect(hooks.hookFunctions.nodeExecuteAfter).toHaveLength(3);

await hooks.executeHookFunctions('nodeExecuteAfter', [
nodeName,
Expand All @@ -234,6 +279,9 @@ describe('Execution Lifecycle Hooks', () => {

it('should not save execution progress when disabled', async () => {
workflowData.settings = { saveExecutionProgress: false };
hooks = createHooks();

expect(hooks.hookFunctions.nodeExecuteAfter).toHaveLength(2);

await hooks.executeHookFunctions('nodeExecuteAfter', [
nodeName,
Expand Down Expand Up @@ -365,6 +413,7 @@ describe('Execution Lifecycle Hooks', () => {

it('should soft delete manual executions when manual saving is disabled', async () => {
hooks.workflowData.settings = { saveManualExecutions: false };
hooks = createHooks();

await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]);

Expand All @@ -373,6 +422,7 @@ describe('Execution Lifecycle Hooks', () => {

it('should not soft delete manual executions with waitTill', async () => {
hooks.workflowData.settings = { saveManualExecutions: false };
hooks = createHooks();

await hooks.executeHookFunctions('workflowExecuteAfter', [waitingRun, {}]);

Expand Down Expand Up @@ -458,32 +508,18 @@ describe('Execution Lifecycle Hooks', () => {
});
});

describe('statistics events', () => {
it('workflowExecuteAfter should emit workflowExecutionCompleted statistics event', async () => {
await hooks.executeHookFunctions('workflowExecuteAfter', [successfulRun, {}]);

expect(workflowStatisticsService.emit).toHaveBeenCalledWith('workflowExecutionCompleted', {
workflowData,
fullRunData: successfulRun,
});
});

it('nodeFetchedData should handle nodeFetchedData statistics event', async () => {
await hooks.executeHookFunctions('nodeFetchedData', [workflowId, node]);

expect(workflowStatisticsService.emit).toHaveBeenCalledWith('nodeFetchedData', {
workflowId,
node,
});
});
});

describe("when pushRef isn't set", () => {
beforeEach(() => {
hooks = getWorkflowHooksMain({ executionMode, workflowData }, executionId);
hooks = getWorkflowHooksMain({ executionMode, workflowData, retryOf }, executionId);
});

it('should not send any push events', async () => {
it('should not setup any push hooks', async () => {
const { hookFunctions } = hooks;
expect(hookFunctions.nodeExecuteBefore).toHaveLength(1);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(1);
expect(hookFunctions.workflowExecuteBefore).toHaveLength(2);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(4);

await hooks.executeHookFunctions('nodeExecuteBefore', [nodeName]);
await hooks.executeHookFunctions('nodeExecuteAfter', [
nodeName,
Expand All @@ -507,20 +543,19 @@ describe('Execution Lifecycle Hooks', () => {
});

workflowEventTests();
externalHooksTests();

it('should setup the correct set of hooks', () => {
expect(hooks).toBeInstanceOf(WorkflowHooks);
expect(hooks.mode).toBe('manual');
expect(hooks.executionId).toBe(executionId);
expect(hooks.workflowData).toEqual(workflowData);
expect(hooks.pushRef).toEqual('test-push-ref');
expect(hooks.retryOf).toEqual('test-retry-of');

const { hookFunctions } = hooks;
expect(hookFunctions.nodeExecuteBefore).toHaveLength(0);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(0);
expect(hookFunctions.workflowExecuteBefore).toHaveLength(2);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(3);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(4);
expect(hookFunctions.nodeFetchedData).toHaveLength(0);
expect(hookFunctions.sendResponse).toHaveLength(0);
});
Expand Down Expand Up @@ -584,18 +619,18 @@ describe('Execution Lifecycle Hooks', () => {
});

nodeEventsTests();
externalHooksTests();
statisticsTests();

it('should setup the correct set of hooks', () => {
expect(hooks).toBeInstanceOf(WorkflowHooks);
expect(hooks.mode).toBe('manual');
expect(hooks.executionId).toBe(executionId);
expect(hooks.workflowData).toEqual(workflowData);
expect(hooks.pushRef).toEqual('test-push-ref');
expect(hooks.retryOf).toEqual('test-retry-of');

const { hookFunctions } = hooks;
expect(hookFunctions.nodeExecuteBefore).toHaveLength(2);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(3);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(2);
expect(hookFunctions.workflowExecuteBefore).toHaveLength(2);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(4);
expect(hookFunctions.nodeFetchedData).toHaveLength(1);
Expand Down Expand Up @@ -680,20 +715,20 @@ describe('Execution Lifecycle Hooks', () => {

workflowEventTests();
nodeEventsTests();
externalHooksTests();
statisticsTests();

it('should setup the correct set of hooks', () => {
expect(hooks).toBeInstanceOf(WorkflowHooks);
expect(hooks.mode).toBe('manual');
expect(hooks.executionId).toBe(executionId);
expect(hooks.workflowData).toEqual(workflowData);
expect(hooks.pushRef).toBeUndefined();
expect(hooks.retryOf).toBeUndefined();

const { hookFunctions } = hooks;
expect(hookFunctions.nodeExecuteBefore).toHaveLength(1);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(2);
expect(hookFunctions.nodeExecuteAfter).toHaveLength(1);
expect(hookFunctions.workflowExecuteBefore).toHaveLength(2);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(3);
expect(hookFunctions.workflowExecuteAfter).toHaveLength(4);
expect(hookFunctions.nodeFetchedData).toHaveLength(1);
expect(hookFunctions.sendResponse).toHaveLength(0);
});
Expand Down
Loading

0 comments on commit 65ec6ae

Please sign in to comment.