From 951d5b7e1bc211112e8c406c7810b61c4cce8dd7 Mon Sep 17 00:00:00 2001 From: czlonkowski <56956555+czlonkowski@users.noreply.github.com> Date: Mon, 29 Sep 2025 18:51:59 +0200 Subject: [PATCH] test: fix tests to match corrected validation behavior - Updated test expecting nodes-base prefix to be invalid - both prefixes are now valid - Changed test name to reflect that both prefixes are accepted - Fixed complex workflow test to not expect error for nodes-base prefix - Added missing mock methods getDefaultOperationForResource and getNodePropertyDefaults These tests were checking for the OLD incorrect behavior that caused false positives. Now they correctly verify that both node type prefixes are valid. --- ...anced-config-validator-integration.test.ts | 4 ++- .../workflow-validator-comprehensive.test.ts | 35 +++++++++++++------ 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/tests/unit/services/enhanced-config-validator-integration.test.ts b/tests/unit/services/enhanced-config-validator-integration.test.ts index 830e82d..2ed8f71 100644 --- a/tests/unit/services/enhanced-config-validator-integration.test.ts +++ b/tests/unit/services/enhanced-config-validator-integration.test.ts @@ -18,7 +18,9 @@ describe('EnhancedConfigValidator - Integration Tests', () => { getNode: vi.fn(), getNodeOperations: vi.fn().mockReturnValue([]), getNodeResources: vi.fn().mockReturnValue([]), - getOperationsForResource: vi.fn().mockReturnValue([]) + getOperationsForResource: vi.fn().mockReturnValue([]), + getDefaultOperationForResource: vi.fn().mockReturnValue(undefined), + getNodePropertyDefaults: vi.fn().mockReturnValue({}) }; mockResourceService = { diff --git a/tests/unit/services/workflow-validator-comprehensive.test.ts b/tests/unit/services/workflow-validator-comprehensive.test.ts index 6b954be..0c1c421 100644 --- a/tests/unit/services/workflow-validator-comprehensive.test.ts +++ b/tests/unit/services/workflow-validator-comprehensive.test.ts @@ -507,13 +507,14 @@ describe('WorkflowValidator - Comprehensive Tests', () => { expect(mockNodeRepository.getNode).not.toHaveBeenCalled(); }); - it('should error for invalid node type starting with nodes-base', async () => { + it('should accept both nodes-base and n8n-nodes-base prefixes as valid', async () => { + // This test verifies the fix for false positives - both prefixes are valid const workflow = { nodes: [ { id: '1', name: 'Webhook', - type: 'nodes-base.webhook', // Missing n8n- prefix + type: 'nodes-base.webhook', // This is now valid (normalized internally) position: [100, 100], parameters: {} } @@ -521,11 +522,24 @@ describe('WorkflowValidator - Comprehensive Tests', () => { connections: {} } as any; + // Mock the normalized node lookup + mockNodeRepository.getNode.mockImplementation((type: string) => { + if (type === 'nodes-base.webhook') { + return { + nodeType: 'nodes-base.webhook', + displayName: 'Webhook', + properties: [], + isVersioned: false + }; + } + return null; + }); + const result = await validator.validateWorkflow(workflow as any); - expect(result.valid).toBe(false); - expect(result.errors.some(e => e.message.includes('Invalid node type: "nodes-base.webhook"'))).toBe(true); - expect(result.errors.some(e => e.message.includes('Use "n8n-nodes-base.webhook" instead'))).toBe(true); + // Should NOT error for nodes-base prefix - it's valid! + expect(result.valid).toBe(true); + expect(result.errors.some(e => e.message.includes('Invalid node type'))).toBe(false); }); it.skip('should handle unknown node types with suggestions', async () => { @@ -1826,11 +1840,11 @@ describe('WorkflowValidator - Comprehensive Tests', () => { parameters: {}, typeVersion: 2 }, - // Node with wrong type format + // Node with valid alternative prefix (no longer an error) { id: '2', name: 'HTTP1', - type: 'nodes-base.httpRequest', // Wrong prefix + type: 'nodes-base.httpRequest', // Valid prefix (normalized internally) position: [300, 100], parameters: {} }, @@ -1900,12 +1914,11 @@ describe('WorkflowValidator - Comprehensive Tests', () => { const result = await validator.validateWorkflow(workflow as any); - // Should have multiple errors + // Should have multiple errors (but not for the nodes-base prefix) expect(result.valid).toBe(false); - expect(result.errors.length).toBeGreaterThan(3); + expect(result.errors.length).toBeGreaterThan(2); // Reduced by 1 since nodes-base prefix is now valid - // Specific errors - expect(result.errors.some(e => e.message.includes('Invalid node type: "nodes-base.httpRequest"'))).toBe(true); + // Specific errors (removed the invalid node type error as it's no longer invalid) expect(result.errors.some(e => e.message.includes('Missing required property \'typeVersion\''))).toBe(true); expect(result.errors.some(e => e.message.includes('Node-level properties onError are in the wrong location'))).toBe(true); expect(result.errors.some(e => e.message.includes('Connection uses node ID \'5\' instead of node name'))).toBe(true);