test: fix failing test and add comprehensive version extraction test coverage

Address code review feedback from PR #285:

1. Fix Failing Test (CRITICAL)
   - Updated test from baseDescription.defaultVersion to description.defaultVersion
   - Added test to verify baseDescription is correctly ignored (legacy bug)

2. Add Missing Test Coverage (HIGH PRIORITY)
   - Test currentVersion priority over description.defaultVersion
   - Test currentVersion = 0 edge case (version 0 should be valid)
   - All 34 tests now passing

3. Enhanced Documentation
   - Added comprehensive JSDoc for extractVersion() explaining priority chain
   - Enhanced validation comments explaining why typeVersion must run before langchain skip
   - Clarified that parameter validation (not typeVersion) is skipped for langchain nodes

Test Results:
-  34/34 tests passing
-  Version extraction priority chain validated
-  Edge cases covered (version 0, missing properties)
-  Legacy bug prevention tested

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
czlonkowski
2025-10-07 20:23:45 +02:00
parent b986beef2c
commit 8e2e1dce62
3 changed files with 72 additions and 8 deletions

View File

@@ -134,6 +134,24 @@ export class NodeParser {
description.name?.toLowerCase().includes('webhook'); description.name?.toLowerCase().includes('webhook');
} }
/**
* Extracts the version from a node class.
*
* Priority Chain:
* 1. Instance currentVersion (VersionedNodeType's computed property)
* 2. Instance description.defaultVersion (explicit default)
* 3. Instance nodeVersions (fallback to max available version)
* 4. Description version array (legacy nodes)
* 5. Description version scalar (simple versioning)
* 6. Class-level properties (if instantiation fails)
* 7. Default to "1"
*
* Critical Fix (v2.17.4): Removed check for non-existent instance.baseDescription.defaultVersion
* which caused AI Agent to incorrectly return version "3" instead of "2.2"
*
* @param nodeClass - The node class or instance to extract version from
* @returns The version as a string
*/
private extractVersion(nodeClass: any): string { private extractVersion(nodeClass: any): string {
// Check instance properties first // Check instance properties first
try { try {

View File

@@ -445,7 +445,9 @@ export class WorkflowValidator {
} }
// Validate typeVersion for ALL versioned nodes (including langchain nodes) // Validate typeVersion for ALL versioned nodes (including langchain nodes)
// This validation runs BEFORE the langchain skip to ensure typeVersion is checked // CRITICAL: This MUST run BEFORE the langchain skip below!
// Otherwise, langchain nodes with invalid typeVersion (e.g., 99999) would pass validation
// but fail at runtime in n8n. This was the bug fixed in v2.17.4.
if (nodeInfo.isVersioned) { if (nodeInfo.isVersioned) {
// Check if typeVersion is missing // Check if typeVersion is missing
if (!node.typeVersion) { if (!node.typeVersion) {
@@ -485,9 +487,9 @@ export class WorkflowValidator {
} }
} }
// Skip parameter validation for langchain nodes // Skip PARAMETER validation for langchain nodes (but NOT typeVersion validation above!)
// They have dedicated AI-specific validators in validateAISpecificNodes() // Langchain nodes have dedicated AI-specific validators in validateAISpecificNodes()
// This prevents parameter validation conflicts and ensures proper AI validation // which handle their unique parameter structures (AI connections, tool ports, etc.)
if (normalizedType.startsWith('nodes-langchain.')) { if (normalizedType.startsWith('nodes-langchain.')) {
continue; continue;
} }

View File

@@ -286,20 +286,64 @@ describe('NodeParser', () => {
}); });
describe('version extraction', () => { describe('version extraction', () => {
it('should extract version from baseDescription.defaultVersion', () => { it('should prioritize currentVersion over description.defaultVersion', () => {
const NodeClass = class { const NodeClass = class {
baseDescription = { currentVersion = 2.2; // Should be returned
description = {
name: 'AI Agent',
displayName: 'AI Agent',
defaultVersion: 3 // Should be ignored when currentVersion exists
};
};
const result = parser.parse(NodeClass, 'n8n-nodes-base');
expect(result.version).toBe('2.2');
});
it('should extract version from description.defaultVersion', () => {
const NodeClass = class {
description = {
name: 'test', name: 'test',
displayName: 'Test', displayName: 'Test',
defaultVersion: 3 defaultVersion: 3
}; };
}; };
const result = parser.parse(NodeClass, 'n8n-nodes-base'); const result = parser.parse(NodeClass, 'n8n-nodes-base');
expect(result.version).toBe('3'); expect(result.version).toBe('3');
}); });
it('should handle currentVersion = 0 correctly', () => {
const NodeClass = class {
currentVersion = 0; // Edge case: version 0 should be valid
description = {
name: 'test',
displayName: 'Test',
defaultVersion: 5 // Should be ignored
};
};
const result = parser.parse(NodeClass, 'n8n-nodes-base');
expect(result.version).toBe('0');
});
it('should NOT extract version from non-existent baseDescription (legacy bug)', () => {
const NodeClass = class {
baseDescription = { // This property doesn't exist on VersionedNodeType!
name: 'test',
displayName: 'Test',
defaultVersion: 3
};
};
const result = parser.parse(NodeClass, 'n8n-nodes-base');
expect(result.version).toBe('1'); // Should fallback to default
});
it('should extract version from nodeVersions keys', () => { it('should extract version from nodeVersions keys', () => {
const NodeClass = class { const NodeClass = class {
description = { name: 'test', displayName: 'Test' }; description = { name: 'test', displayName: 'Test' };