mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-02-06 05:23:08 +00:00
test: add comprehensive WorkflowValidator tests (97.59% coverage)
- Create comprehensive test suite with 69 tests for WorkflowValidator - Increase coverage from 2.32% to 97.59% - Fix bugs in WorkflowValidator: - Add null checks for workflow.nodes before accessing length - Fix checkNodeErrorHandling to process each node individually - Fix disabled node validation logic - Ensure error-prone nodes generate proper warnings - Test all major methods and edge cases: - validateWorkflow with different options - validateAllNodes with various node types - validateConnections including cycles and orphans - validateExpressions with complex expressions - checkWorkflowPatterns for best practices - checkNodeErrorHandling for error configurations - generateSuggestions for helpful tips All 69 tests now passing with excellent coverage. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
1
coverage.json
Normal file
1
coverage.json
Normal file
File diff suppressed because one or more lines are too long
@@ -101,8 +101,8 @@ export class WorkflowValidator {
|
|||||||
errors: [],
|
errors: [],
|
||||||
warnings: [],
|
warnings: [],
|
||||||
statistics: {
|
statistics: {
|
||||||
totalNodes: workflow.nodes.length,
|
totalNodes: workflow.nodes?.length || 0,
|
||||||
enabledNodes: workflow.nodes.filter(n => !n.disabled).length,
|
enabledNodes: workflow.nodes?.filter(n => !n.disabled).length || 0,
|
||||||
triggerNodes: 0,
|
triggerNodes: 0,
|
||||||
validConnections: 0,
|
validConnections: 0,
|
||||||
invalidConnections: 0,
|
invalidConnections: 0,
|
||||||
@@ -783,8 +783,10 @@ export class WorkflowValidator {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check node-level error handling properties
|
// Check node-level error handling properties for ALL nodes
|
||||||
this.checkNodeErrorHandling(workflow, result);
|
for (const node of workflow.nodes) {
|
||||||
|
this.checkNodeErrorHandling(node, workflow, result);
|
||||||
|
}
|
||||||
|
|
||||||
// Check for very long linear workflows
|
// Check for very long linear workflows
|
||||||
const linearChainLength = this.getLongestLinearChain(workflow);
|
const linearChainLength = this.getLongestLinearChain(workflow);
|
||||||
@@ -795,6 +797,9 @@ export class WorkflowValidator {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Generate error handling suggestions based on all nodes
|
||||||
|
this.generateErrorHandlingSuggestions(workflow, result);
|
||||||
|
|
||||||
// Check for missing credentials
|
// Check for missing credentials
|
||||||
for (const node of workflow.nodes) {
|
for (const node of workflow.nodes) {
|
||||||
if (node.credentials && Object.keys(node.credentials).length > 0) {
|
if (node.credentials && Object.keys(node.credentials).length > 0) {
|
||||||
@@ -1017,17 +1022,21 @@ export class WorkflowValidator {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Check node-level error handling configuration
|
* Check node-level error handling configuration for a single node
|
||||||
*/
|
*/
|
||||||
private checkNodeErrorHandling(
|
private checkNodeErrorHandling(
|
||||||
|
node: WorkflowNode,
|
||||||
workflow: WorkflowJson,
|
workflow: WorkflowJson,
|
||||||
result: WorkflowValidationResult
|
result: WorkflowValidationResult
|
||||||
): void {
|
): void {
|
||||||
// Define node types that typically interact with external services
|
// Only skip if disabled is explicitly true (not just truthy)
|
||||||
|
if (node.disabled === true) return;
|
||||||
|
|
||||||
|
// Define node types that typically interact with external services (lowercase for comparison)
|
||||||
const errorProneNodeTypes = [
|
const errorProneNodeTypes = [
|
||||||
'httpRequest',
|
'httprequest',
|
||||||
'webhook',
|
'webhook',
|
||||||
'emailSend',
|
'emailsend',
|
||||||
'slack',
|
'slack',
|
||||||
'discord',
|
'discord',
|
||||||
'telegram',
|
'telegram',
|
||||||
@@ -1041,8 +1050,8 @@ export class WorkflowValidator {
|
|||||||
'salesforce',
|
'salesforce',
|
||||||
'hubspot',
|
'hubspot',
|
||||||
'airtable',
|
'airtable',
|
||||||
'googleSheets',
|
'googlesheets',
|
||||||
'googleDrive',
|
'googledrive',
|
||||||
'dropbox',
|
'dropbox',
|
||||||
's3',
|
's3',
|
||||||
'ftp',
|
'ftp',
|
||||||
@@ -1055,30 +1064,27 @@ export class WorkflowValidator {
|
|||||||
'anthropic'
|
'anthropic'
|
||||||
];
|
];
|
||||||
|
|
||||||
for (const node of workflow.nodes) {
|
const normalizedType = node.type.toLowerCase();
|
||||||
if (node.disabled) continue;
|
const isErrorProne = errorProneNodeTypes.some(type => normalizedType.includes(type));
|
||||||
|
|
||||||
const normalizedType = node.type.toLowerCase();
|
// CRITICAL: Check for node-level properties in wrong location (inside parameters)
|
||||||
const isErrorProne = errorProneNodeTypes.some(type => normalizedType.includes(type));
|
const nodeLevelProps = [
|
||||||
|
// Error handling properties
|
||||||
// CRITICAL: Check for node-level properties in wrong location (inside parameters)
|
'onError', 'continueOnFail', 'retryOnFail', 'maxTries', 'waitBetweenTries', 'alwaysOutputData',
|
||||||
const nodeLevelProps = [
|
// Other node-level properties
|
||||||
// Error handling properties
|
'executeOnce', 'disabled', 'notes', 'notesInFlow', 'credentials'
|
||||||
'onError', 'continueOnFail', 'retryOnFail', 'maxTries', 'waitBetweenTries', 'alwaysOutputData',
|
];
|
||||||
// Other node-level properties
|
const misplacedProps: string[] = [];
|
||||||
'executeOnce', 'disabled', 'notes', 'notesInFlow', 'credentials'
|
|
||||||
];
|
if (node.parameters) {
|
||||||
const misplacedProps: string[] = [];
|
for (const prop of nodeLevelProps) {
|
||||||
|
if (node.parameters[prop] !== undefined) {
|
||||||
if (node.parameters) {
|
misplacedProps.push(prop);
|
||||||
for (const prop of nodeLevelProps) {
|
|
||||||
if (node.parameters[prop] !== undefined) {
|
|
||||||
misplacedProps.push(prop);
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
}
|
||||||
if (misplacedProps.length > 0) {
|
|
||||||
|
if (misplacedProps.length > 0) {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
@@ -1098,12 +1104,12 @@ export class WorkflowValidator {
|
|||||||
`}`
|
`}`
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate error handling properties
|
// Validate error handling properties
|
||||||
|
|
||||||
// Check for onError property (the modern approach)
|
// Check for onError property (the modern approach)
|
||||||
if (node.onError !== undefined) {
|
if (node.onError !== undefined) {
|
||||||
const validOnErrorValues = ['continueRegularOutput', 'continueErrorOutput', 'stopWorkflow'];
|
const validOnErrorValues = ['continueRegularOutput', 'continueErrorOutput', 'stopWorkflow'];
|
||||||
if (!validOnErrorValues.includes(node.onError)) {
|
if (!validOnErrorValues.includes(node.onError)) {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
@@ -1113,10 +1119,10 @@ export class WorkflowValidator {
|
|||||||
message: `Invalid onError value: "${node.onError}". Must be one of: ${validOnErrorValues.join(', ')}`
|
message: `Invalid onError value: "${node.onError}". Must be one of: ${validOnErrorValues.join(', ')}`
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for deprecated continueOnFail
|
// Check for deprecated continueOnFail
|
||||||
if (node.continueOnFail !== undefined) {
|
if (node.continueOnFail !== undefined) {
|
||||||
if (typeof node.continueOnFail !== 'boolean') {
|
if (typeof node.continueOnFail !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
@@ -1133,19 +1139,19 @@ export class WorkflowValidator {
|
|||||||
message: 'Using deprecated "continueOnFail: true". Use "onError: \'continueRegularOutput\'" instead for better control and UI compatibility.'
|
message: 'Using deprecated "continueOnFail: true". Use "onError: \'continueRegularOutput\'" instead for better control and UI compatibility.'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for conflicting error handling properties
|
// Check for conflicting error handling properties
|
||||||
if (node.continueOnFail !== undefined && node.onError !== undefined) {
|
if (node.continueOnFail !== undefined && node.onError !== undefined) {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'Cannot use both "continueOnFail" and "onError" properties. Use only "onError" for modern workflows.'
|
message: 'Cannot use both "continueOnFail" and "onError" properties. Use only "onError" for modern workflows.'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
if (node.retryOnFail !== undefined) {
|
if (node.retryOnFail !== undefined) {
|
||||||
if (typeof node.retryOnFail !== 'boolean') {
|
if (typeof node.retryOnFail !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
@@ -1201,21 +1207,21 @@ export class WorkflowValidator {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (node.alwaysOutputData !== undefined && typeof node.alwaysOutputData !== 'boolean') {
|
if (node.alwaysOutputData !== undefined && typeof node.alwaysOutputData !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'alwaysOutputData must be a boolean value'
|
message: 'alwaysOutputData must be a boolean value'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Warnings for error-prone nodes without error handling
|
// Warnings for error-prone nodes without error handling
|
||||||
const hasErrorHandling = node.onError || node.continueOnFail || node.retryOnFail;
|
const hasErrorHandling = node.onError || node.continueOnFail || node.retryOnFail;
|
||||||
|
|
||||||
if (isErrorProne && !hasErrorHandling) {
|
if (isErrorProne && !hasErrorHandling) {
|
||||||
const nodeTypeSimple = normalizedType.split('.').pop() || normalizedType;
|
const nodeTypeSimple = normalizedType.split('.').pop() || normalizedType;
|
||||||
|
|
||||||
// Special handling for specific node types
|
// Special handling for specific node types
|
||||||
@@ -1245,83 +1251,91 @@ export class WorkflowValidator {
|
|||||||
type: 'warning',
|
type: 'warning',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: `${nodeTypeSimple} node interacts with external services but has no error handling configured. Consider using "onError" property.`
|
message: `${nodeTypeSimple} node without error handling. Consider using "onError" property for better error management.`
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check for problematic combinations
|
// Check for problematic combinations
|
||||||
if (node.continueOnFail && node.retryOnFail) {
|
if (node.continueOnFail && node.retryOnFail) {
|
||||||
result.warnings.push({
|
result.warnings.push({
|
||||||
type: 'warning',
|
type: 'warning',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'Both continueOnFail and retryOnFail are enabled. The node will retry first, then continue on failure.'
|
message: 'Both continueOnFail and retryOnFail are enabled. The node will retry first, then continue on failure.'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Validate additional node-level properties
|
// Validate additional node-level properties
|
||||||
|
|
||||||
// Check executeOnce
|
// Check executeOnce
|
||||||
if (node.executeOnce !== undefined && typeof node.executeOnce !== 'boolean') {
|
if (node.executeOnce !== undefined && typeof node.executeOnce !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'executeOnce must be a boolean value'
|
message: 'executeOnce must be a boolean value'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check disabled
|
// Check disabled
|
||||||
if (node.disabled !== undefined && typeof node.disabled !== 'boolean') {
|
if (node.disabled !== undefined && typeof node.disabled !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'disabled must be a boolean value'
|
message: 'disabled must be a boolean value'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check notesInFlow
|
// Check notesInFlow
|
||||||
if (node.notesInFlow !== undefined && typeof node.notesInFlow !== 'boolean') {
|
if (node.notesInFlow !== undefined && typeof node.notesInFlow !== 'boolean') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'notesInFlow must be a boolean value'
|
message: 'notesInFlow must be a boolean value'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check notes
|
// Check notes
|
||||||
if (node.notes !== undefined && typeof node.notes !== 'string') {
|
if (node.notes !== undefined && typeof node.notes !== 'string') {
|
||||||
result.errors.push({
|
result.errors.push({
|
||||||
type: 'error',
|
type: 'error',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'notes must be a string value'
|
message: 'notes must be a string value'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Provide guidance for executeOnce
|
// Provide guidance for executeOnce
|
||||||
if (node.executeOnce === true) {
|
if (node.executeOnce === true) {
|
||||||
result.warnings.push({
|
result.warnings.push({
|
||||||
type: 'warning',
|
type: 'warning',
|
||||||
nodeId: node.id,
|
nodeId: node.id,
|
||||||
nodeName: node.name,
|
nodeName: node.name,
|
||||||
message: 'executeOnce is enabled. This node will execute only once regardless of input items.'
|
message: 'executeOnce is enabled. This node will execute only once regardless of input items.'
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
// Suggest alwaysOutputData for debugging
|
// Suggest alwaysOutputData for debugging
|
||||||
if ((node.continueOnFail || node.retryOnFail) && !node.alwaysOutputData) {
|
if ((node.continueOnFail || node.retryOnFail) && !node.alwaysOutputData) {
|
||||||
if (normalizedType.includes('httprequest') || normalizedType.includes('webhook')) {
|
if (normalizedType.includes('httprequest') || normalizedType.includes('webhook')) {
|
||||||
result.suggestions.push(
|
result.suggestions.push(
|
||||||
`Consider enabling alwaysOutputData on "${node.name}" to capture error responses for debugging`
|
`Consider enabling alwaysOutputData on "${node.name}" to capture error responses for debugging`
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Generate error handling suggestions based on all nodes
|
||||||
|
*/
|
||||||
|
private generateErrorHandlingSuggestions(
|
||||||
|
workflow: WorkflowJson,
|
||||||
|
result: WorkflowValidationResult
|
||||||
|
): void {
|
||||||
// Add general suggestions based on findings
|
// Add general suggestions based on findings
|
||||||
const nodesWithoutErrorHandling = workflow.nodes.filter(n =>
|
const nodesWithoutErrorHandling = workflow.nodes.filter(n =>
|
||||||
!n.disabled && !n.onError && !n.continueOnFail && !n.retryOnFail
|
!n.disabled && !n.onError && !n.continueOnFail && !n.retryOnFail
|
||||||
|
|||||||
1944
tests/unit/services/workflow-validator-comprehensive.test.ts
Normal file
1944
tests/unit/services/workflow-validator-comprehensive.test.ts
Normal file
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user