mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-02-06 05:23:08 +00:00
test: Add comprehensive edge case coverage for telemetry quick wins
Added 20 edge case tests based on code review recommendations: **Duplicate ID Validation (4 tests)**: - Multiple duplicate IDs (3+ nodes with same ID) - Duplicate IDs with same node type - Duplicate IDs with empty/null node names - Duplicate IDs with missing node properties **AI Agent Validator (16 tests)**: maxIterations edge cases (7 tests): - Boundary values: 0 (reject), 1 (accept), 51 (warn), MAX_SAFE_INTEGER (warn) - Invalid types: NaN (reject), negative decimal (reject) - Threshold testing: 50 vs 51 promptType validation (4 tests): - Whitespace-only text (reject) - Very long text 3200+ chars (accept) - undefined/null text (reject) System message validation (5 tests): - Empty/whitespace messages (suggest adding) - Very long messages >1000 chars (accept) - Special characters, emojis, unicode (accept) - Multi-line formatting (accept) - Boundary: 19 chars (warn), 20 chars (accept) **Test Quality Improvements**: - Fixed flaky system message test (changed from expect.stringContaining to .some()) - All tests are deterministic - Comprehensive inline comments - Follows existing test patterns All 20 new tests passing. Zero regressions. Concieved by Romuald Członkowski - www.aiadvisors.pl/en
This commit is contained in:
czlonkowski
@@ -2355,6 +2355,62 @@ return [{"json": {"result": result}}]
|
||||
const textErrors = context.errors.filter(e => e.property === 'text');
|
||||
expect(textErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should reject whitespace-only text with promptType "define"', () => {
|
||||
// Edge case: Text is only whitespace
|
||||
context.config.promptType = 'define';
|
||||
context.config.text = ' \n\t ';
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'missing_required',
|
||||
property: 'text',
|
||||
message: 'Custom prompt text is required when promptType is "define"',
|
||||
fix: 'Provide a custom prompt in the text field, or change promptType to "auto"'
|
||||
});
|
||||
});
|
||||
|
||||
it('should accept very long text with promptType "define"', () => {
|
||||
// Edge case: Very long prompt text (common for complex AI agents)
|
||||
context.config.promptType = 'define';
|
||||
context.config.text = 'You are a helpful assistant. '.repeat(100); // 3200 characters
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const textErrors = context.errors.filter(e => e.property === 'text');
|
||||
expect(textErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle undefined text with promptType "define"', () => {
|
||||
// Edge case: Text is undefined
|
||||
context.config.promptType = 'define';
|
||||
context.config.text = undefined;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'missing_required',
|
||||
property: 'text',
|
||||
message: 'Custom prompt text is required when promptType is "define"',
|
||||
fix: 'Provide a custom prompt in the text field, or change promptType to "auto"'
|
||||
});
|
||||
});
|
||||
|
||||
it('should handle null text with promptType "define"', () => {
|
||||
// Edge case: Text is null
|
||||
context.config.promptType = 'define';
|
||||
context.config.text = null;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'missing_required',
|
||||
property: 'text',
|
||||
message: 'Custom prompt text is required when promptType is "define"',
|
||||
fix: 'Provide a custom prompt in the text field, or change promptType to "auto"'
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('system message validation', () => {
|
||||
@@ -2363,9 +2419,11 @@ return [{"json": {"result": result}}]
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.suggestions).toContain(
|
||||
expect.stringContaining('system message')
|
||||
// Should contain a suggestion about system message
|
||||
const hasSysMessageSuggestion = context.suggestions.some(s =>
|
||||
s.toLowerCase().includes('system message')
|
||||
);
|
||||
expect(hasSysMessageSuggestion).toBe(true);
|
||||
});
|
||||
|
||||
it('should warn when system message is too short', () => {
|
||||
@@ -2389,6 +2447,93 @@ return [{"json": {"result": result}}]
|
||||
const systemWarnings = context.warnings.filter(w => w.property === 'systemMessage');
|
||||
expect(systemWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should suggest adding system message when empty string', () => {
|
||||
// Edge case: Empty string system message
|
||||
context.config.systemMessage = '';
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
// Should contain a suggestion about system message
|
||||
const hasSysMessageSuggestion = context.suggestions.some(s =>
|
||||
s.toLowerCase().includes('system message')
|
||||
);
|
||||
expect(hasSysMessageSuggestion).toBe(true);
|
||||
});
|
||||
|
||||
it('should suggest adding system message when whitespace only', () => {
|
||||
// Edge case: Whitespace-only system message
|
||||
context.config.systemMessage = ' \n\t ';
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
// Should contain a suggestion about system message
|
||||
const hasSysMessageSuggestion = context.suggestions.some(s =>
|
||||
s.toLowerCase().includes('system message')
|
||||
);
|
||||
expect(hasSysMessageSuggestion).toBe(true);
|
||||
});
|
||||
|
||||
it('should accept very long system messages', () => {
|
||||
// Edge case: Very long system message (>1000 chars) for complex agents
|
||||
context.config.systemMessage = 'You are a highly specialized assistant. '.repeat(30); // ~1260 chars
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const systemWarnings = context.warnings.filter(w => w.property === 'systemMessage');
|
||||
expect(systemWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle system messages with special characters', () => {
|
||||
// Edge case: System message with special characters, emojis, unicode
|
||||
context.config.systemMessage = 'You are an assistant 🤖 that handles data with special chars: @#$%^&*(){}[]|\\/<>~`';
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const systemWarnings = context.warnings.filter(w => w.property === 'systemMessage');
|
||||
expect(systemWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should handle system messages with newlines and formatting', () => {
|
||||
// Edge case: Multi-line system message with formatting
|
||||
context.config.systemMessage = `You are a helpful assistant.
|
||||
|
||||
Your responsibilities include:
|
||||
1. Analyzing customer feedback
|
||||
2. Generating reports
|
||||
3. Providing insights
|
||||
|
||||
Always be professional and concise.`;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const systemWarnings = context.warnings.filter(w => w.property === 'systemMessage');
|
||||
expect(systemWarnings).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should warn about exactly 19 character system message', () => {
|
||||
// Edge case: Just under the 20 character threshold
|
||||
context.config.systemMessage = 'Be a good assistant'; // 19 chars
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.warnings).toContainEqual({
|
||||
type: 'inefficient',
|
||||
property: 'systemMessage',
|
||||
message: 'System message is very short (< 20 characters)',
|
||||
suggestion: 'Consider a more detailed system message to guide the agent\'s behavior'
|
||||
});
|
||||
});
|
||||
|
||||
it('should not warn about exactly 20 character system message', () => {
|
||||
// Edge case: Exactly at the 20 character threshold
|
||||
context.config.systemMessage = 'Be a great assistant'; // 20 chars
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const systemWarnings = context.warnings.filter(w => w.property === 'systemMessage');
|
||||
expect(systemWarnings).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('maxIterations validation', () => {
|
||||
@@ -2426,6 +2571,87 @@ return [{"json": {"result": result}}]
|
||||
const maxIterErrors = context.errors.filter(e => e.property === 'maxIterations');
|
||||
expect(maxIterErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should reject maxIterations of 0', () => {
|
||||
// Edge case: Zero iterations is invalid
|
||||
context.config.maxIterations = 0;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'invalid_value',
|
||||
property: 'maxIterations',
|
||||
message: 'maxIterations must be a positive number',
|
||||
fix: 'Set maxIterations to a value >= 1 (e.g., 10)'
|
||||
});
|
||||
});
|
||||
|
||||
it('should accept maxIterations of 1', () => {
|
||||
// Edge case: Minimum valid value
|
||||
context.config.maxIterations = 1;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
const maxIterErrors = context.errors.filter(e => e.property === 'maxIterations');
|
||||
expect(maxIterErrors).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should warn about maxIterations of 51', () => {
|
||||
// Edge case: Just above the threshold (50)
|
||||
context.config.maxIterations = 51;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.warnings).toContainEqual(
|
||||
expect.objectContaining({
|
||||
type: 'inefficient',
|
||||
property: 'maxIterations',
|
||||
message: expect.stringContaining('51')
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle extreme maxIterations values', () => {
|
||||
// Edge case: Very large number
|
||||
context.config.maxIterations = Number.MAX_SAFE_INTEGER;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.warnings).toContainEqual(
|
||||
expect.objectContaining({
|
||||
type: 'inefficient',
|
||||
property: 'maxIterations'
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject NaN maxIterations', () => {
|
||||
// Edge case: Not a number
|
||||
context.config.maxIterations = 'invalid';
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'invalid_value',
|
||||
property: 'maxIterations',
|
||||
message: 'maxIterations must be a positive number',
|
||||
fix: 'Set maxIterations to a value >= 1 (e.g., 10)'
|
||||
});
|
||||
});
|
||||
|
||||
it('should reject negative decimal maxIterations', () => {
|
||||
// Edge case: Negative decimal
|
||||
context.config.maxIterations = -0.5;
|
||||
|
||||
NodeSpecificValidators.validateAIAgent(context);
|
||||
|
||||
expect(context.errors).toContainEqual({
|
||||
type: 'invalid_value',
|
||||
property: 'maxIterations',
|
||||
message: 'maxIterations must be a positive number',
|
||||
fix: 'Set maxIterations to a value >= 1 (e.g., 10)'
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('error handling', () => {
|
||||
|
||||
@@ -353,5 +353,222 @@ describe('WorkflowValidator', () => {
|
||||
// This validates that our implementation uses the pattern
|
||||
expect(expectedPattern.test('crypto.randomUUID()')).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect multiple nodes with the same duplicate ID', () => {
|
||||
// Edge case: Three or more nodes with the same ID
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Multiple Duplicates',
|
||||
nodes: [
|
||||
{
|
||||
id: 'shared-id',
|
||||
name: 'First Node',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 3,
|
||||
position: [250, 300],
|
||||
parameters: {}
|
||||
},
|
||||
{
|
||||
id: 'shared-id', // Duplicate 1
|
||||
name: 'Second Node',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 2,
|
||||
position: [450, 300],
|
||||
parameters: {}
|
||||
},
|
||||
{
|
||||
id: 'shared-id', // Duplicate 2
|
||||
name: 'Third Node',
|
||||
type: 'n8n-nodes-base.code',
|
||||
typeVersion: 1,
|
||||
position: [650, 300],
|
||||
parameters: {}
|
||||
}
|
||||
],
|
||||
connections: {}
|
||||
};
|
||||
|
||||
// Simulate validation logic
|
||||
const nodeIds = new Set<string>();
|
||||
const nodeIdToIndex = new Map<string, number>();
|
||||
const errors: Array<{ message: string }> = [];
|
||||
|
||||
for (let i = 0; i < workflow.nodes.length; i++) {
|
||||
const node = workflow.nodes[i];
|
||||
if (nodeIds.has(node.id)) {
|
||||
const firstNodeIndex = nodeIdToIndex.get(node.id);
|
||||
const firstNode = firstNodeIndex !== undefined ? workflow.nodes[firstNodeIndex] : undefined;
|
||||
|
||||
errors.push({
|
||||
message: `Duplicate node ID: "${node.id}". Node at index ${i} (name: "${node.name}", type: "${node.type}") conflicts with node at index ${firstNodeIndex} (name: "${firstNode?.name || 'unknown'}", type: "${firstNode?.type || 'unknown'}")`
|
||||
});
|
||||
} else {
|
||||
nodeIds.add(node.id);
|
||||
nodeIdToIndex.set(node.id, i);
|
||||
}
|
||||
}
|
||||
|
||||
// Should report 2 errors (nodes at index 1 and 2 both conflict with node at index 0)
|
||||
expect(errors).toHaveLength(2);
|
||||
expect(errors[0].message).toContain('index 1');
|
||||
expect(errors[0].message).toContain('Second Node');
|
||||
expect(errors[1].message).toContain('index 2');
|
||||
expect(errors[1].message).toContain('Third Node');
|
||||
});
|
||||
|
||||
it('should handle duplicate IDs with same node type', () => {
|
||||
// Edge case: Both nodes are the same type
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Same Type Duplicates',
|
||||
nodes: [
|
||||
{
|
||||
id: 'duplicate-slack',
|
||||
name: 'Slack Send 1',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
typeVersion: 2,
|
||||
position: [250, 300],
|
||||
parameters: {}
|
||||
},
|
||||
{
|
||||
id: 'duplicate-slack',
|
||||
name: 'Slack Send 2',
|
||||
type: 'n8n-nodes-base.slack',
|
||||
typeVersion: 2,
|
||||
position: [450, 300],
|
||||
parameters: {}
|
||||
}
|
||||
],
|
||||
connections: {}
|
||||
};
|
||||
|
||||
// Simulate validation logic
|
||||
const nodeIds = new Set<string>();
|
||||
const nodeIdToIndex = new Map<string, number>();
|
||||
const errors: Array<{ message: string }> = [];
|
||||
|
||||
for (let i = 0; i < workflow.nodes.length; i++) {
|
||||
const node = workflow.nodes[i];
|
||||
if (nodeIds.has(node.id)) {
|
||||
const firstNodeIndex = nodeIdToIndex.get(node.id);
|
||||
const firstNode = firstNodeIndex !== undefined ? workflow.nodes[firstNodeIndex] : undefined;
|
||||
|
||||
errors.push({
|
||||
message: `Duplicate node ID: "${node.id}". Node at index ${i} (name: "${node.name}", type: "${node.type}") conflicts with node at index ${firstNodeIndex} (name: "${firstNode?.name || 'unknown'}", type: "${firstNode?.type || 'unknown'}")`
|
||||
});
|
||||
} else {
|
||||
nodeIds.add(node.id);
|
||||
nodeIdToIndex.set(node.id, i);
|
||||
}
|
||||
}
|
||||
|
||||
expect(errors).toHaveLength(1);
|
||||
expect(errors[0].message).toContain('Duplicate node ID: "duplicate-slack"');
|
||||
expect(errors[0].message).toContain('Slack Send 2');
|
||||
expect(errors[0].message).toContain('Slack Send 1');
|
||||
// Both should show the same type
|
||||
expect(errors[0].message).toMatch(/n8n-nodes-base\.slack.*n8n-nodes-base\.slack/s);
|
||||
});
|
||||
|
||||
it('should handle duplicate IDs with empty node names gracefully', () => {
|
||||
// Edge case: Empty string node names
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Empty Names',
|
||||
nodes: [
|
||||
{
|
||||
id: 'empty-name-id',
|
||||
name: '',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 3,
|
||||
position: [250, 300],
|
||||
parameters: {}
|
||||
},
|
||||
{
|
||||
id: 'empty-name-id',
|
||||
name: '',
|
||||
type: 'n8n-nodes-base.set',
|
||||
typeVersion: 2,
|
||||
position: [450, 300],
|
||||
parameters: {}
|
||||
}
|
||||
],
|
||||
connections: {}
|
||||
};
|
||||
|
||||
// Simulate validation logic with safe fallback
|
||||
const nodeIds = new Set<string>();
|
||||
const nodeIdToIndex = new Map<string, number>();
|
||||
const errors: Array<{ message: string }> = [];
|
||||
|
||||
for (let i = 0; i < workflow.nodes.length; i++) {
|
||||
const node = workflow.nodes[i];
|
||||
if (nodeIds.has(node.id)) {
|
||||
const firstNodeIndex = nodeIdToIndex.get(node.id);
|
||||
const firstNode = firstNodeIndex !== undefined ? workflow.nodes[firstNodeIndex] : undefined;
|
||||
|
||||
errors.push({
|
||||
message: `Duplicate node ID: "${node.id}". Node at index ${i} (name: "${node.name}", type: "${node.type}") conflicts with node at index ${firstNodeIndex} (name: "${firstNode?.name || 'unknown'}", type: "${firstNode?.type || 'unknown'}")`
|
||||
});
|
||||
} else {
|
||||
nodeIds.add(node.id);
|
||||
nodeIdToIndex.set(node.id, i);
|
||||
}
|
||||
}
|
||||
|
||||
// Should not crash and should use empty string in message
|
||||
expect(errors).toHaveLength(1);
|
||||
expect(errors[0].message).toContain('Duplicate node ID');
|
||||
expect(errors[0].message).toContain('name: ""');
|
||||
});
|
||||
|
||||
it('should handle duplicate IDs with missing node properties', () => {
|
||||
// Edge case: Node with undefined type or name
|
||||
const workflow = {
|
||||
name: 'Test Workflow with Missing Properties',
|
||||
nodes: [
|
||||
{
|
||||
id: 'missing-props',
|
||||
name: 'Valid Node',
|
||||
type: 'n8n-nodes-base.httpRequest',
|
||||
typeVersion: 3,
|
||||
position: [250, 300],
|
||||
parameters: {}
|
||||
},
|
||||
{
|
||||
id: 'missing-props',
|
||||
name: undefined as any,
|
||||
type: undefined as any,
|
||||
typeVersion: 2,
|
||||
position: [450, 300],
|
||||
parameters: {}
|
||||
}
|
||||
],
|
||||
connections: {}
|
||||
};
|
||||
|
||||
// Simulate validation logic with safe fallbacks
|
||||
const nodeIds = new Set<string>();
|
||||
const nodeIdToIndex = new Map<string, number>();
|
||||
const errors: Array<{ message: string }> = [];
|
||||
|
||||
for (let i = 0; i < workflow.nodes.length; i++) {
|
||||
const node = workflow.nodes[i];
|
||||
if (nodeIds.has(node.id)) {
|
||||
const firstNodeIndex = nodeIdToIndex.get(node.id);
|
||||
const firstNode = firstNodeIndex !== undefined ? workflow.nodes[firstNodeIndex] : undefined;
|
||||
|
||||
errors.push({
|
||||
message: `Duplicate node ID: "${node.id}". Node at index ${i} (name: "${node.name}", type: "${node.type}") conflicts with node at index ${firstNodeIndex} (name: "${firstNode?.name || 'unknown'}", type: "${firstNode?.type || 'unknown'}")`
|
||||
});
|
||||
} else {
|
||||
nodeIds.add(node.id);
|
||||
nodeIdToIndex.set(node.id, i);
|
||||
}
|
||||
}
|
||||
|
||||
// Should use fallback values without crashing
|
||||
expect(errors).toHaveLength(1);
|
||||
expect(errors[0].message).toContain('Duplicate node ID: "missing-props"');
|
||||
expect(errors[0].message).toContain('name: "undefined"');
|
||||
expect(errors[0].message).toContain('type: "undefined"');
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user