👌 Updating tests, using type over interface

n8n-io · Dec 20, 2024 · 60d6ed9 · 60d6ed9
1 parent 675f4fa
commit 60d6ed9
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 100 deletions.
diff --git a/packages/editor-ui/src/composables/useDocumentVisibility.ts b/packages/editor-ui/src/composables/useDocumentVisibility.ts
@@ -3,11 +3,11 @@ import { ref, onMounted, onUnmounted } from 'vue';
 
 type VisibilityHandler = () => void;
 
-interface DocumentVisibilityResult {
+type DocumentVisibilityResult = {
 	isVisible: Ref<boolean>;
 	onDocumentVisible: (handler: VisibilityHandler) => void;
 	onDocumentHidden: (handler: VisibilityHandler) => void;
-}
+};
 
 export function useDocumentVisibility(): DocumentVisibilityResult {
 	const isVisible = ref<boolean>(!document.hidden);

diff --git a/packages/editor-ui/src/utils/nodeSettingsUtils.test.ts b/packages/editor-ui/src/utils/nodeSettingsUtils.test.ts
@@ -1,10 +1,9 @@
 import { describe, it, expect, afterAll } from 'vitest';
 import { mock } from 'vitest-mock-extended';
-import type { IConnections, NodeParameterValueType, ResourceMapperField } from 'n8n-workflow';
+import type { IConnections, NodeParameterValueType } from 'n8n-workflow';
 import { updateDynamicConnections } from './nodeSettingsUtils';
 import { SWITCH_NODE_TYPE } from '@/constants';
 import type { INodeUi, IUpdateInformation } from '@/Interface';
-import { isResourceMapperFieldListStale } from './nodeTypesUtils';
 
 describe('updateDynamicConnections', () => {
 	afterAll(() => {
@@ -161,99 +160,3 @@ describe('updateDynamicConnections', () => {
 		expect(result).toBeNull();
 	});
 });
-
-describe('isResourceMapperFieldListStale', () => {
-	const baseField: ResourceMapperField = {
-		id: 'test',
-		displayName: 'test',
-		required: false,
-		defaultMatch: false,
-		display: true,
-		canBeUsedToMatch: true,
-		type: 'string',
-	};
-
-	test('returns false for identical lists', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(false);
-	});
-
-	test('returns true for different lengths', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField }, { ...baseField, id: 'test2' }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when field is removed', () => {
-		const oldFields = [
-			{ ...baseField, id: 'test1' },
-			{ ...baseField, id: 'test2' },
-		];
-		const newFields = [
-			{ ...baseField, id: 'test1' },
-			{ ...baseField, id: 'test3' },
-		];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when displayName changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField, displayName: 'changed' }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when required changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField, required: true }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when defaultMatch changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField, defaultMatch: true }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when display changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField, display: false }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when canBeUsedToMatch changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields = [{ ...baseField, canBeUsedToMatch: false }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns true when type changes', () => {
-		const oldFields = [{ ...baseField }];
-		const newFields: ResourceMapperField[] = [{ ...baseField, type: 'number' }];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
-	});
-
-	test('returns false for multiple identical fields', () => {
-		const oldFields = [
-			{ ...baseField, id: 'test1' },
-			{ ...baseField, id: 'test2' },
-			{ ...baseField, id: 'test3' },
-		];
-		const newFields = [
-			{ ...baseField, id: 'test1' },
-			{ ...baseField, id: 'test2' },
-			{ ...baseField, id: 'test3' },
-		];
-		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(false);
-	});
-
-	test('handles empty arrays correctly', () => {
-		expect(isResourceMapperFieldListStale([], [])).toBe(false);
-	});
-
-	test('returns true when comparing empty array with non-empty array', () => {
-		const nonEmptyFields = [{ ...baseField }];
-		expect(isResourceMapperFieldListStale([], nonEmptyFields)).toBe(true);
-		expect(isResourceMapperFieldListStale(nonEmptyFields, [])).toBe(true);
-	});
-});
diff --git a/packages/editor-ui/src/utils/nodeTypeUtils.test.ts b/packages/editor-ui/src/utils/nodeTypeUtils.test.ts
@@ -0,0 +1,75 @@
+import type { ResourceMapperField } from 'n8n-workflow';
+import { isResourceMapperFieldListStale } from './nodeTypesUtils';
+
+describe('isResourceMapperFieldListStale', () => {
+	const baseField: ResourceMapperField = {
+		id: 'test',
+		displayName: 'test',
+		required: false,
+		defaultMatch: false,
+		display: true,
+		canBeUsedToMatch: true,
+		type: 'string',
+	};
+
+	// Test property changes
+	test.each([
+		[
+			'displayName',
+			{ ...baseField },
+			{ ...baseField, displayName: 'changed' } as ResourceMapperField,
+		],
+		['required', { ...baseField }, { ...baseField, required: true } as ResourceMapperField],
+		['defaultMatch', { ...baseField }, { ...baseField, defaultMatch: true } as ResourceMapperField],
+		['display', { ...baseField }, { ...baseField, display: false }],
+		[
+			'canBeUsedToMatch',
+			{ ...baseField },
+			{ ...baseField, canBeUsedToMatch: false } as ResourceMapperField,
+		],
+		['type', { ...baseField }, { ...baseField, type: 'number' } as ResourceMapperField],
+	])('returns true when %s changes', (_property, oldField, newField) => {
+		expect(isResourceMapperFieldListStale([oldField], [newField])).toBe(true);
+	});
+
+	// Test different array lengths
+	test.each([
+		['empty vs non-empty', [], [baseField]],
+		['non-empty vs empty', [baseField], []],
+		['one vs two fields', [baseField], [baseField, { ...baseField, id: 'test2' }]],
+	])('returns true for different lengths: %s', (_scenario, oldFields, newFields) => {
+		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
+	});
+
+	// Test identical cases
+	test.each([
+		['empty arrays', [], []],
+		['single field', [baseField], [{ ...baseField }]],
+		[
+			'multiple fields',
+			[
+				{ ...baseField, id: 'test1' },
+				{ ...baseField, id: 'test2' },
+			],
+			[
+				{ ...baseField, id: 'test1' },
+				{ ...baseField, id: 'test2' },
+			],
+		],
+	])('returns false for identical lists: %s', (_scenario, oldFields, newFields) => {
+		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(false);
+	});
+
+	// This test case is complex enough to keep separate
+	test('returns true when field is removed/replaced', () => {
+		const oldFields = [
+			{ ...baseField, id: 'test1' },
+			{ ...baseField, id: 'test2' },
+		];
+		const newFields = [
+			{ ...baseField, id: 'test1' },
+			{ ...baseField, id: 'test3' }, // different id
+		];
+		expect(isResourceMapperFieldListStale(oldFields, newFields)).toBe(true);
+	});
+});