ci: Fix WorkflowHistoryManager tests (no-changelog) (#7356)

[DB Tests](https://github.com/n8n-io/n8n/actions/runs/6418058186)
n8n-io · Oct 5, 2023 · 1691750 · 1691750
1 parent 7ebf8f3
commit 1691750
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 162 deletions.
diff --git a/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts b/packages/cli/src/workflows/workflowHistory/workflowHistoryManager.ee.ts
@@ -1,9 +1,9 @@
-import { WorkflowHistoryRepository } from '@/databases/repositories';
 import { Service } from 'typedi';
+import { LessThan } from 'typeorm';
+import { DateTime } from 'luxon';
+import { WorkflowHistoryRepository } from '@/databases/repositories';
 import { WORKFLOW_HISTORY_PRUNE_INTERVAL } from './constants';
 import { getWorkflowHistoryPruneTime, isWorkflowHistoryEnabled } from './workflowHistoryHelper.ee';
-import { DateTime } from 'luxon';
-import { LessThan } from 'typeorm';
 
 @Service()
 export class WorkflowHistoryManager {

diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts
@@ -118,7 +118,7 @@ export async function terminate() {
 export async function truncate(collections: CollectionName[]) {
 	const [tag, rest] = separate(collections, (c) => c === 'Tag');
 
-	if (tag) {
+	if (tag.length) {
 		await Container.get(TagRepository).delete({});
 		await Container.get(WorkflowTagMappingRepository).delete({});
 	}

diff --git a/packages/cli/test/integration/workflowHistoryManager.test.ts b/packages/cli/test/integration/workflowHistoryManager.test.ts
@@ -1,208 +1,121 @@
+import Container from 'typedi';
+import { In } from 'typeorm';
+import { DateTime } from 'luxon';
+
+import config from '@/config';
 import { WorkflowHistoryRepository } from '@/databases/repositories';
-import * as testDb from './shared/testDb';
 import { License } from '@/License';
-import { mockInstance } from './shared/utils';
 import { WorkflowHistoryManager } from '@/workflows/workflowHistory/workflowHistoryManager.ee';
-import Container from 'typedi';
-import config from '@/config';
-import { DateTime } from 'luxon';
-import { In } from 'typeorm';
 
-let licenseMock: License;
-let licensePruneTime = -1;
-let licenseEnabled = true;
-let manager: WorkflowHistoryManager;
-
-beforeAll(async () => {
-	await testDb.init();
-
-	licenseMock = mockInstance(License, {
-		isWorkflowHistoryLicensed() {
-			return licenseEnabled;
-		},
-		getWorkflowHistoryPruneLimit() {
-			return licensePruneTime;
-		},
+import * as testDb from './shared/testDb';
+import { mockInstance } from './shared/utils';
+
+describe('Workflow History Manager', () => {
+	const license = mockInstance(License);
+	let repo: WorkflowHistoryRepository;
+	let manager: WorkflowHistoryManager;
+
+	beforeAll(async () => {
+		await testDb.init();
+		repo = Container.get(WorkflowHistoryRepository);
+		manager = Container.get(WorkflowHistoryManager);
 	});
-});
 
-beforeEach(async () => {
-	await testDb.truncate([WorkflowHistoryRepository]);
-	jest.useRealTimers();
-	jest.clearAllMocks();
-	config.set('workflowHistory.enabled', true);
-	config.set('workflowHistory.pruneTime', -1);
-	licensePruneTime = -1;
-	licenseEnabled = true;
-});
+	beforeEach(async () => {
+		await testDb.truncate(['Workflow']);
+		jest.clearAllMocks();
 
-afterEach(() => {
-	manager?.shutdown();
-});
+		config.set('workflowHistory.enabled', true);
+		config.set('workflowHistory.pruneTime', -1);
 
-describe('Workflow History Manager', () => {
-	test('should prune on interval', () => {
-		jest.useFakeTimers();
+		license.isWorkflowHistoryLicensed.mockReturnValue(true);
+		license.getWorkflowHistoryPruneLimit.mockReturnValue(-1);
+	});
 
-		manager = new WorkflowHistoryManager(Container.get(WorkflowHistoryRepository));
-		manager.init();
+	test('should prune on interval', () => {
 		const pruneSpy = jest.spyOn(manager, 'prune');
 		const currentCount = pruneSpy.mock.calls.length;
 
-		jest.runOnlyPendingTimers();
+		jest.useFakeTimers();
+		manager.init();
 
+		jest.runOnlyPendingTimers();
 		expect(pruneSpy).toBeCalledTimes(currentCount + 1);
 
 		jest.runOnlyPendingTimers();
 		expect(pruneSpy).toBeCalledTimes(currentCount + 2);
+
+		manager.shutdown();
+		jest.clearAllTimers();
+		jest.useRealTimers();
+		pruneSpy.mockRestore();
 	});
 
 	test('should not prune when not licensed', async () => {
-		// Set a prune time just to make sure it gets to the delete
-		config.set('workflowHistory.pruneTime', 24);
-
-		licenseEnabled = false;
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
-
-		const workflow = await testDb.createWorkflow();
-		await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
-
-		expect(await repo.count()).toBe(10);
-
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).not.toBeCalled();
-		expect(await repo.count()).toBe(10);
+		license.isWorkflowHistoryLicensed.mockReturnValue(false);
+		await createWorkflowHistory();
+		await pruneAndAssertCount();
 	});
 
 	test('should not prune when licensed but disabled', async () => {
-		// Set a prune time just to make sure it gets to the delete
-		config.set('workflowHistory.pruneTime', 24);
-
 		config.set('workflowHistory.enabled', false);
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
-
-		const workflow = await testDb.createWorkflow();
-		await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
-
-		expect(await repo.count()).toBe(10);
-
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).not.toBeCalled();
-		expect(await repo.count()).toBe(10);
+		await createWorkflowHistory();
+		await pruneAndAssertCount();
 	});
 
 	test('should not prune when both prune times are -1 (infinite)', async () => {
-		config.set('workflowHistory.pruneTime', -1);
-		licensePruneTime = -1;
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
-
-		const workflow = await testDb.createWorkflow();
-		await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
-
-		expect(await repo.count()).toBe(10);
-
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).not.toBeCalled();
-		expect(await repo.count()).toBe(10);
+		await createWorkflowHistory();
+		await pruneAndAssertCount();
 	});
 
 	test('should prune when config prune time is not -1 (infinite)', async () => {
 		config.set('workflowHistory.pruneTime', 24);
-		licensePruneTime = -1;
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
-
-		const workflow = await testDb.createWorkflow();
-		await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
-
-		expect(await repo.count()).toBe(10);
-
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).toBeCalled();
-		expect(await repo.count()).toBe(0);
+		await createWorkflowHistory();
+		await pruneAndAssertCount(0);
 	});
 
 	test('should prune when license prune time is not -1 (infinite)', async () => {
-		config.set('workflowHistory.pruneTime', -1);
-		licensePruneTime = 24;
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
-
-		const workflow = await testDb.createWorkflow();
-		await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
+		license.getWorkflowHistoryPruneLimit.mockReturnValue(24);
 
-		expect(await repo.count()).toBe(10);
-
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).toBeCalled();
-		expect(await repo.count()).toBe(0);
+		await createWorkflowHistory();
+		await pruneAndAssertCount(0);
 	});
 
 	test('should only prune versions older than prune time', async () => {
 		config.set('workflowHistory.pruneTime', 24);
-		licensePruneTime = -1;
-
-		const repo = Container.get(WorkflowHistoryRepository);
-		manager = new WorkflowHistoryManager(repo);
-		manager.init();
 
-		const workflow = await testDb.createWorkflow();
-		const recentVersions = await testDb.createManyWorkflowHistoryItems(workflow.id, 10);
-		const oldVersions = await testDb.createManyWorkflowHistoryItems(
-			workflow.id,
-			10,
-			DateTime.now().minus({ days: 2 }).toJSDate(),
-		);
+		const recentVersions = await createWorkflowHistory(0);
+		const oldVersions = await createWorkflowHistory();
 
-		expect(await repo.count()).toBe(20);
+		await pruneAndAssertCount(10, 20);
 
-		const deleteSpy = jest.spyOn(repo, 'delete');
-		await manager.prune();
-		expect(deleteSpy).toBeCalled();
-		expect(await repo.count()).toBe(10);
 		expect(
 			await repo.count({ where: { versionId: In(recentVersions.map((i) => i.versionId)) } }),
 		).toBe(10);
 		expect(
 			await repo.count({ where: { versionId: In(oldVersions.map((i) => i.versionId)) } }),
 		).toBe(0);
 	});
+
+	const createWorkflowHistory = async (ageInDays = 2) => {
+		const workflow = await testDb.createWorkflow();
+		const time = DateTime.now().minus({ days: ageInDays }).toJSDate();
+		return testDb.createManyWorkflowHistoryItems(workflow.id, 10, time);
+	};
+
+	const pruneAndAssertCount = async (finalCount = 10, initialCount = 10) => {
+		expect(await repo.count()).toBe(initialCount);
+
+		const deleteSpy = jest.spyOn(repo, 'delete');
+		await manager.prune();
+
+		if (initialCount === finalCount) {
+			expect(deleteSpy).not.toBeCalled();
+		} else {
+			expect(deleteSpy).toBeCalled();
+		}
+		deleteSpy.mockRestore();
+
+		expect(await repo.count()).toBe(finalCount);
+	};
 });