diff --git a/.github/scripts/check-tests.mjs b/.github/scripts/check-tests.mjs deleted file mode 100644 index 1e2e38e05b..0000000000 --- a/.github/scripts/check-tests.mjs +++ /dev/null @@ -1,103 +0,0 @@ -import { readFile } from 'fs/promises'; -import path from 'path'; -import util from 'util'; -import { exec } from 'child_process'; -import { glob } from 'glob'; -import ts from 'typescript'; - -const execAsync = util.promisify(exec); - -const filterAsync = async (asyncPredicate, arr) => { - const filterResults = await Promise.all( - arr.map(async (item) => ({ - item, - shouldKeep: await asyncPredicate(item), - })), - ); - - return filterResults.filter(({ shouldKeep }) => shouldKeep).map(({ item }) => item); -}; - -const isAbstractClass = (node) => { - if (ts.isClassDeclaration(node)) { - return ( - node.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.AbstractKeyword) || false - ); - } - return false; -}; - -const isAbstractMethod = (node) => { - return ( - ts.isMethodDeclaration(node) && - Boolean(node.modifiers?.find((modifier) => modifier.kind === ts.SyntaxKind.AbstractKeyword)) - ); -}; - -// Function to check if a file has a function declaration, function expression, object method or class -const hasFunctionOrClass = async (filePath) => { - const fileContent = await readFile(filePath, 'utf-8'); - const sourceFile = ts.createSourceFile(filePath, fileContent, ts.ScriptTarget.Latest, true); - - let hasFunctionOrClass = false; - const visit = (node) => { - if ( - ts.isFunctionDeclaration(node) || - ts.isFunctionExpression(node) || - ts.isArrowFunction(node) || - (ts.isMethodDeclaration(node) && !isAbstractMethod(node)) || - (ts.isClassDeclaration(node) && !isAbstractClass(node)) - ) { - hasFunctionOrClass = true; - } - node.forEachChild(visit); - }; - - visit(sourceFile); - - return hasFunctionOrClass; -}; - -const main = async () => { - // Run a git command to get a list of all changed files in the branch (branch has to be up to date with master) - const changedFiles = await execAsync( - 'git diff --name-only --diff-filter=d origin/master..HEAD', - ).then(({ stdout }) => stdout.trim().split('\n').filter(Boolean)); - - // Get all .spec.ts and .test.ts files from the packages - const specAndTestTsFiles = await glob('packages/*/**/{test,__tests__}/**/*.{spec,test}.ts'); - const specAndTestTsFilesNames = specAndTestTsFiles.map((file) => - path.parse(file).name.replace(/\.(test|spec)/, ''), - ); - - // Filter out the .ts and .vue files from the changed files - const changedVueFiles = changedFiles.filter((file) => file.endsWith('.vue')); - // .ts files with any kind of function declaration or class and not in any of the test folders - const changedTsFilesWithFunction = await filterAsync( - async (filePath) => - filePath.endsWith('.ts') && - !(await glob('packages/*/**/{test,__tests__}/*.ts')).includes(filePath) && - (await hasFunctionOrClass(filePath)), - changedFiles, - ); - - // For each .ts or .vue file, check if there's a corresponding .test.ts or .spec.ts file in the repository - const missingTests = changedVueFiles - .concat(changedTsFilesWithFunction) - .reduce((filesList, nextFile) => { - const fileName = path.parse(nextFile).name; - - if (!specAndTestTsFilesNames.includes(fileName)) { - filesList.push(nextFile); - } - - return filesList; - }, []); - - if (missingTests.length) { - console.error(`Missing tests for:\n${missingTests.join('\n')}`); - process.exit(1); - } -}; - -main(); diff --git a/.github/scripts/package.json b/.github/scripts/package.json index 80bf0baf11..7640790f2d 100644 --- a/.github/scripts/package.json +++ b/.github/scripts/package.json @@ -7,7 +7,6 @@ "p-limit": "3.1.0", "picocolors": "1.0.1", "semver": "7.5.4", - "tempfile": "5.0.0", - "typescript": "*" + "tempfile": "5.0.0" } } diff --git a/.github/workflows/check-tests.yml b/.github/workflows/check-tests.yml deleted file mode 100644 index 97f380974a..0000000000 --- a/.github/workflows/check-tests.yml +++ /dev/null @@ -1,30 +0,0 @@ -name: Check Test Files - -on: - pull_request: - branches: - - '**' - - '!release/*' - pull_request_target: - branches: - - master - -jobs: - check-tests: - runs-on: ubuntu-latest - continue-on-error: true - steps: - - name: Checkout code - uses: actions/checkout@v4.1.1 - with: - fetch-depth: 0 - - - name: Use Node.js - uses: actions/setup-node@v4.0.2 - with: - node-version: 20.x - - - run: npm install --prefix=.github/scripts --no-package-lock - - - name: Check for test files - run: node .github/scripts/check-tests.mjs diff --git a/cypress/e2e/29-templates.cy.ts b/cypress/e2e/29-templates.cy.ts index 5b52889c94..727b0cbe3f 100644 --- a/cypress/e2e/29-templates.cy.ts +++ b/cypress/e2e/29-templates.cy.ts @@ -129,7 +129,6 @@ describe('Workflow templates', () => { workflowPage.actions.shouldHaveWorkflowName('Demo: ' + OnboardingWorkflow.name); workflowPage.getters.canvasNodes().should('have.length', 4); workflowPage.getters.stickies().should('have.length', 1); - workflowPage.getters.canvasNodes().first().should('have.descendants', '.node-pin-data-icon'); }); it('can import template', () => { @@ -142,6 +141,7 @@ describe('Workflow templates', () => { }); it('should save template id with the workflow', () => { + cy.intercept('POST', '/rest/workflows').as('saveWorkflow'); templatesPage.actions.importTemplate(); cy.visit(templatesPage.url); @@ -159,10 +159,8 @@ describe('Workflow templates', () => { workflowPage.actions.hitSelectAll(); workflowPage.actions.hitCopy(); - cy.grantBrowserPermissions('clipboardReadWrite', 'clipboardSanitizedWrite'); - // Check workflow JSON by copying it to clipboard - cy.readClipboard().then((workflowJSON) => { - expect(workflowJSON).to.contain('"templateId": "1"'); + cy.wait('@saveWorkflow').then((interception) => { + expect(interception.request.body.meta.templateId).to.equal('1'); }); }); diff --git a/cypress/e2e/39-projects.cy.ts b/cypress/e2e/39-projects.cy.ts index cc1ab5ca36..327bff4f93 100644 --- a/cypress/e2e/39-projects.cy.ts +++ b/cypress/e2e/39-projects.cy.ts @@ -498,7 +498,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move workflow")') + .contains('button', 'Move workflow') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -506,7 +506,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(':contains("Project 1")') .click(); - projects.getResourceMoveModal().find('button:contains("Move workflow")').click(); + projects.getResourceMoveModal().contains('button', 'Move workflow').click(); clearNotifications(); workflowsPage.getters @@ -524,7 +524,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move workflow")') + .contains('button', 'Move workflow') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -532,7 +532,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(':contains("Project 2")') .click(); - projects.getResourceMoveModal().find('button:contains("Move workflow")').click(); + projects.getResourceMoveModal().contains('button', 'Move workflow').click(); // Move the workflow from Project 2 to a member user projects.getMenuItems().last().click(); @@ -544,7 +544,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move workflow")') + .contains('button', 'Move workflow') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -553,7 +553,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .filter(`:contains("${INSTANCE_MEMBERS[0].email}")`) .click(); - projects.getResourceMoveModal().find('button:contains("Move workflow")').click(); + projects.getResourceMoveModal().contains('button', 'Move workflow').click(); workflowsPage.getters.workflowCards().should('have.length', 1); // Move the workflow from member user back to Home @@ -569,7 +569,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move workflow")') + .contains('button', 'Move workflow') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -578,7 +578,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .filter(`:contains("${INSTANCE_OWNER.email}")`) .click(); - projects.getResourceMoveModal().find('button:contains("Move workflow")').click(); + projects.getResourceMoveModal().contains('button', 'Move workflow').click(); clearNotifications(); workflowsPage.getters .workflowCards() @@ -596,7 +596,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move credential")') + .contains('button', 'Move credential') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -604,7 +604,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(':contains("Project 2")') .click(); - projects.getResourceMoveModal().find('button:contains("Move credential")').click(); + projects.getResourceMoveModal().contains('button', 'Move credential').click(); clearNotifications(); credentialsPage.getters.credentialCards().should('not.have.length'); @@ -619,7 +619,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move credential")') + .contains('button', 'Move credential') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -627,7 +627,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(`:contains("${INSTANCE_ADMIN.email}")`) .click(); - projects.getResourceMoveModal().find('button:contains("Move credential")').click(); + projects.getResourceMoveModal().contains('button', 'Move credential').click(); credentialsPage.getters.credentialCards().should('have.length', 1); // Move the credential from admin user back to instance owner @@ -641,7 +641,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move credential")') + .contains('button', 'Move credential') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -649,7 +649,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(`:contains("${INSTANCE_OWNER.email}")`) .click(); - projects.getResourceMoveModal().find('button:contains("Move credential")').click(); + projects.getResourceMoveModal().contains('button', 'Move credential').click(); clearNotifications(); @@ -666,7 +666,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move credential")') + .contains('button', 'Move credential') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -674,7 +674,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 5) .filter(':contains("Project 1")') .click(); - projects.getResourceMoveModal().find('button:contains("Move credential")').click(); + projects.getResourceMoveModal().contains('button', 'Move credential').click(); projects.getMenuItems().first().click(); projects.getProjectTabCredentials().click(); @@ -721,7 +721,7 @@ describe('Projects', { disableAutoLogin: true }, () => { projects .getResourceMoveModal() .should('be.visible') - .find('button:contains("Move workflow")') + .contains('button', 'Move workflow') .should('be.disabled'); projects.getProjectMoveSelect().click(); getVisibleSelect() @@ -729,7 +729,7 @@ describe('Projects', { disableAutoLogin: true }, () => { .should('have.length', 4) .filter(':contains("Project 1")') .click(); - projects.getResourceMoveModal().find('button:contains("Move workflow")').click(); + projects.getResourceMoveModal().contains('button', 'Move workflow').click(); workflowsPage.getters .workflowCards() diff --git a/cypress/e2e/5-ndv.cy.ts b/cypress/e2e/5-ndv.cy.ts index a10dbc94e6..e8215db38f 100644 --- a/cypress/e2e/5-ndv.cy.ts +++ b/cypress/e2e/5-ndv.cy.ts @@ -862,4 +862,18 @@ describe('NDV', () => { .contains('To search field contents rather than just names, use Table or JSON view') .should('exist'); }); + + it('ADO-2931 - should handle multiple branches of the same input with the first branch empty correctly', () => { + cy.createFixtureWorkflow('Test_ndv_two_branches_of_same_parent_false_populated.json'); + workflowPage.actions.zoomToFit(); + workflowPage.actions.openNode('DebugHelper'); + ndv.getters.inputPanel().should('be.visible'); + ndv.getters.outputPanel().should('be.visible'); + ndv.actions.execute(); + // This ensures we rendered the inputPanel + ndv.getters + .inputPanel() + .find('[data-test-id=run-data-schema-item]') + .should('contain.text', 'a1'); + }); }); diff --git a/cypress/fixtures/Test_ndv_two_branches_of_same_parent_false_populated.json b/cypress/fixtures/Test_ndv_two_branches_of_same_parent_false_populated.json new file mode 100644 index 0000000000..056a35a786 --- /dev/null +++ b/cypress/fixtures/Test_ndv_two_branches_of_same_parent_false_populated.json @@ -0,0 +1,94 @@ +{ + "nodes": [ + { + "parameters": { + "conditions": { + "options": { + "caseSensitive": true, + "leftValue": "", + "typeValidation": "strict", + "version": 2 + }, + "conditions": [ + { + "id": "6f0cf983-824b-4339-a5de-6b374a23b4b0", + "leftValue": "={{ $json.a }}", + "rightValue": 3, + "operator": { + "type": "number", + "operation": "equals" + } + } + ], + "combinator": "and" + }, + "options": {} + }, + "type": "n8n-nodes-base.if", + "typeVersion": 2.2, + "position": [220, 0], + "id": "1755282a-ec4a-4d02-a833-0316ca413cc4", + "name": "If" + }, + { + "parameters": {}, + "type": "n8n-nodes-base.manualTrigger", + "typeVersion": 1, + "position": [0, 0], + "id": "de1e7acf-12d8-4e56-ba42-709ffb397db2", + "name": "When clicking ‘Test workflow’" + }, + { + "parameters": { + "category": "randomData" + }, + "type": "n8n-nodes-base.debugHelper", + "typeVersion": 1, + "position": [580, 0], + "id": "86440d33-f833-453c-bcaa-fff7e0083501", + "name": "DebugHelper", + "alwaysOutputData": true + } + ], + "connections": { + "If": { + "main": [ + [ + { + "node": "DebugHelper", + "type": "main", + "index": 0 + } + ], + [ + { + "node": "DebugHelper", + "type": "main", + "index": 0 + } + ] + ] + }, + "When clicking ‘Test workflow’": { + "main": [ + [ + { + "node": "If", + "type": "main", + "index": 0 + } + ] + ] + } + }, + "pinData": { + "When clicking ‘Test workflow’": [ + { + "a": 1 + }, + { + "a": 2 + } + ] + } +} diff --git a/cypress/pages/workflow.ts b/cypress/pages/workflow.ts index 1b63688da1..e99b01aa46 100644 --- a/cypress/pages/workflow.ts +++ b/cypress/pages/workflow.ts @@ -32,7 +32,11 @@ export class WorkflowPage extends BasePage { canvasNodes: () => cy.ifCanvasVersion( () => cy.getByTestId('canvas-node'), - () => cy.getByTestId('canvas-node').not('[data-node-type="n8n-nodes-internal.addNodes"]'), + () => + cy + .getByTestId('canvas-node') + .not('[data-node-type="n8n-nodes-internal.addNodes"]') + .not('[data-node-type="n8n-nodes-base.stickyNote"]'), ), canvasNodeByName: (nodeName: string) => this.getters.canvasNodes().filter(`:contains(${nodeName})`), diff --git a/packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsOpenAI/EmbeddingsOpenAi.node.ts b/packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsOpenAI/EmbeddingsOpenAi.node.ts index fb5f67e30c..cd44cb114b 100644 --- a/packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsOpenAI/EmbeddingsOpenAi.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/embeddings/EmbeddingsOpenAI/EmbeddingsOpenAi.node.ts @@ -79,7 +79,7 @@ export class EmbeddingsOpenAi implements INodeType { }, ], group: ['transform'], - version: [1, 1.1], + version: [1, 1.1, 1.2], description: 'Use Embeddings OpenAI', defaults: { name: 'Embeddings OpenAI', @@ -106,7 +106,7 @@ export class EmbeddingsOpenAi implements INodeType { requestDefaults: { ignoreHttpStatusErrors: true, baseURL: - '={{ $parameter.options?.baseURL?.split("/").slice(0,-1).join("/") || "https://api.openai.com" }}', + '={{ $parameter.options?.baseURL?.split("/").slice(0,-1).join("/") || $credentials.url?.split("/").slice(0,-1).join("/") || "https://api.openai.com" }}', }, properties: [ getConnectionHintNoticeField([NodeConnectionType.AiVectorStore]), @@ -171,6 +171,11 @@ export class EmbeddingsOpenAi implements INodeType { default: 'https://api.openai.com/v1', description: 'Override the default base URL for the API', type: 'string', + displayOptions: { + hide: { + '@version': [{ _cnd: { gte: 1.2 } }], + }, + }, }, { displayName: 'Batch Size', @@ -219,6 +224,8 @@ export class EmbeddingsOpenAi implements INodeType { const configuration: ClientOptions = {}; if (options.baseURL) { configuration.baseURL = options.baseURL; + } else if (credentials.url) { + configuration.baseURL = credentials.url as string; } const embeddings = new OpenAIEmbeddings( diff --git a/packages/@n8n/nodes-langchain/nodes/llms/LMChatOpenAi/LmChatOpenAi.node.ts b/packages/@n8n/nodes-langchain/nodes/llms/LMChatOpenAi/LmChatOpenAi.node.ts index f3dceb5dd1..cf24d944de 100644 --- a/packages/@n8n/nodes-langchain/nodes/llms/LMChatOpenAi/LmChatOpenAi.node.ts +++ b/packages/@n8n/nodes-langchain/nodes/llms/LMChatOpenAi/LmChatOpenAi.node.ts @@ -22,7 +22,7 @@ export class LmChatOpenAi implements INodeType { name: 'lmChatOpenAi', icon: { light: 'file:openAiLight.svg', dark: 'file:openAiLight.dark.svg' }, group: ['transform'], - version: 1, + version: [1, 1.1], description: 'For advanced usage with an AI chain', defaults: { name: 'OpenAI Chat Model', @@ -55,7 +55,7 @@ export class LmChatOpenAi implements INodeType { requestDefaults: { ignoreHttpStatusErrors: true, baseURL: - '={{ $parameter.options?.baseURL?.split("/").slice(0,-1).join("/") || "https://api.openai.com" }}', + '={{ $parameter.options?.baseURL?.split("/").slice(0,-1).join("/") || $credentials?.url?.split("/").slice(0,-1).join("/") || "https://api.openai.com" }}', }, properties: [ getConnectionHintNoticeField([NodeConnectionType.AiChain, NodeConnectionType.AiAgent]), @@ -82,7 +82,7 @@ export class LmChatOpenAi implements INodeType { routing: { request: { method: 'GET', - url: '={{ $parameter.options?.baseURL?.split("/").slice(-1).pop() || "v1" }}/models', + url: '={{ $parameter.options?.baseURL?.split("/").slice(-1).pop() || $credentials?.url?.split("/").slice(-1).pop() || "v1" }}/models', }, output: { postReceive: [ @@ -98,6 +98,7 @@ export class LmChatOpenAi implements INodeType { // If the baseURL is not set or is set to api.openai.com, include only chat models pass: `={{ ($parameter.options?.baseURL && !$parameter.options?.baseURL?.includes('api.openai.com')) || + ($credentials?.url && !$credentials.url.includes('api.openai.com')) || $responseItem.id.startsWith('ft:') || $responseItem.id.startsWith('o1') || ($responseItem.id.startsWith('gpt-') && !$responseItem.id.includes('instruct')) @@ -156,6 +157,11 @@ export class LmChatOpenAi implements INodeType { default: 'https://api.openai.com/v1', description: 'Override the default base URL for the API', type: 'string', + displayOptions: { + hide: { + '@version': [{ _cnd: { gte: 1.1 } }], + }, + }, }, { displayName: 'Frequency Penalty', @@ -261,6 +267,8 @@ export class LmChatOpenAi implements INodeType { const configuration: ClientOptions = {}; if (options.baseURL) { configuration.baseURL = options.baseURL; + } else if (credentials.url) { + configuration.baseURL = credentials.url as string; } const model = new ChatOpenAI({ diff --git a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts index 977530cc83..bc22ac948f 100644 --- a/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts +++ b/packages/@n8n/nodes-langchain/nodes/vendors/OpenAi/actions/assistant/message.operation.ts @@ -106,6 +106,11 @@ const properties: INodeProperties[] = [ default: 'https://api.openai.com/v1', description: 'Override the default base URL for the API', type: 'string', + displayOptions: { + hide: { + '@version': [{ _cnd: { gte: 1.8 } }], + }, + }, }, { displayName: 'Max Retries', @@ -182,11 +187,13 @@ export async function execute(this: IExecuteFunctions, i: number): Promise { it('should serialize correctly', () => { const error = new Error('a.unknown is not a function'); - error.stack = defaultStack; + Object.defineProperty(error, 'stack', { + value: defaultStack, + enumerable: true, + }); + // error.stack = defaultStack; const executionError = new ExecutionError(error, 1); diff --git a/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts b/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts index 63a2dd5e0b..ef593d9589 100644 --- a/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts +++ b/packages/@n8n/task-runner/src/js-task-runner/errors/execution-error.ts @@ -10,8 +10,6 @@ export class ExecutionError extends SerializableError { context: { itemIndex: number } | undefined = undefined; - stack = ''; - lineNumber: number | undefined = undefined; constructor(error: ErrorLike, itemIndex?: number) { @@ -22,7 +20,12 @@ export class ExecutionError extends SerializableError { this.context = { itemIndex: this.itemIndex }; } - this.stack = error.stack ?? ''; + // Override the stack trace with the given error's stack trace. Since + // node v22 it's not writable, so we can't assign it directly + Object.defineProperty(this, 'stack', { + value: error.stack, + enumerable: true, + }); this.populateFromStack(); } @@ -31,7 +34,7 @@ export class ExecutionError extends SerializableError { * Populate error `message` and `description` from error `stack`. */ private populateFromStack() { - const stackRows = this.stack.split('\n'); + const stackRows = (this.stack ?? '').split('\n'); if (stackRows.length === 0) { this.message = 'Unknown error'; diff --git a/packages/@n8n/task-runner/src/polyfills.ts b/packages/@n8n/task-runner/src/polyfills.ts new file mode 100644 index 0000000000..7d8a83dd0e --- /dev/null +++ b/packages/@n8n/task-runner/src/polyfills.ts @@ -0,0 +1,5 @@ +// WebCrypto Polyfill for older versions of Node.js 18 +if (!globalThis.crypto?.getRandomValues) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-var-requires, @typescript-eslint/no-unsafe-member-access + globalThis.crypto = require('node:crypto').webcrypto; +} diff --git a/packages/@n8n/task-runner/src/start.ts b/packages/@n8n/task-runner/src/start.ts index 69e9462516..391b6ba156 100644 --- a/packages/@n8n/task-runner/src/start.ts +++ b/packages/@n8n/task-runner/src/start.ts @@ -1,3 +1,4 @@ +import './polyfills'; import type { ErrorReporter } from 'n8n-core'; import { ensureError, setGlobalState } from 'n8n-workflow'; import Container from 'typedi'; diff --git a/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts b/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts index e03fb8dc47..f923cb4b73 100644 --- a/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts +++ b/packages/cli/src/evaluation/test-runner/__tests__/test-runner.service.ee.test.ts @@ -15,7 +15,11 @@ import type { ExecutionRepository } from '@/databases/repositories/execution.rep import type { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; import type { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import type { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; +import { NodeTypes } from '@/node-types'; import type { WorkflowRunner } from '@/workflow-runner'; +import { mockInstance } from '@test/mocking'; +import { mockNodeTypesData } from '@test-integration/utils/node-types-data'; import { TestRunnerService } from '../test-runner.service.ee'; @@ -27,10 +31,28 @@ const wfEvaluationJson = JSON.parse( readFileSync(path.join(__dirname, './mock-data/workflow.evaluation.json'), { encoding: 'utf-8' }), ); +const wfMultipleTriggersJson = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/workflow.multiple-triggers.json'), { + encoding: 'utf-8', + }), +); + const executionDataJson = JSON.parse( readFileSync(path.join(__dirname, './mock-data/execution-data.json'), { encoding: 'utf-8' }), ); +const executionDataMultipleTriggersJson = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/execution-data.multiple-triggers.json'), { + encoding: 'utf-8', + }), +); + +const executionDataMultipleTriggersJson2 = JSON.parse( + readFileSync(path.join(__dirname, './mock-data/execution-data.multiple-triggers-2.json'), { + encoding: 'utf-8', + }), +); + const executionMocks = [ mock({ id: 'some-execution-id', @@ -93,6 +115,11 @@ describe('TestRunnerService', () => { const testRunRepository = mock(); const testMetricRepository = mock(); + const mockNodeTypes = mockInstance(NodeTypes); + mockInstance(LoadNodesAndCredentials, { + loadedNodes: mockNodeTypesData(['manualTrigger', 'set', 'if', 'code']), + }); + beforeEach(() => { const executionsQbMock = mockDeep>({ fallbackMockImplementation: jest.fn().mockReturnThis(), @@ -131,6 +158,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); expect(testRunnerService).toBeInstanceOf(TestRunnerService); @@ -144,6 +172,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -180,6 +209,7 @@ describe('TestRunnerService', () => { activeExecutions, testRunRepository, testMetricRepository, + mockNodeTypes, ); workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ @@ -267,4 +297,125 @@ describe('TestRunnerService', () => { metric2: 0, }); }); + + test('should specify correct start nodes when running workflow under test', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + workflowRepository.findById.calledWith('workflow-under-test-id').mockResolvedValueOnce({ + id: 'workflow-under-test-id', + ...wfUnderTestJson, + }); + + workflowRepository.findById.calledWith('evaluation-workflow-id').mockResolvedValueOnce({ + id: 'evaluation-workflow-id', + ...wfEvaluationJson, + }); + + workflowRunner.run.mockResolvedValueOnce('some-execution-id'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-2'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-3'); + workflowRunner.run.mockResolvedValueOnce('some-execution-id-4'); + + // Mock executions of workflow under test + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id') + .mockResolvedValue(mockExecutionData()); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-3') + .mockResolvedValue(mockExecutionData()); + + // Mock executions of evaluation workflow + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-2') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 1, metric2: 0 })); + + activeExecutions.getPostExecutePromise + .calledWith('some-execution-id-4') + .mockResolvedValue(mockEvaluationExecutionData({ metric1: 0.5 })); + + await testRunnerService.runTest( + mock(), + mock({ + workflowId: 'workflow-under-test-id', + evaluationWorkflowId: 'evaluation-workflow-id', + mockedNodes: [{ name: 'When clicking ‘Test workflow’' }], + }), + ); + + expect(workflowRunner.run).toHaveBeenCalledTimes(4); + + // Check workflow under test was executed + expect(workflowRunner.run).toHaveBeenCalledWith( + expect.objectContaining({ + executionMode: 'evaluation', + pinData: { + 'When clicking ‘Test workflow’': + executionDataJson.resultData.runData['When clicking ‘Test workflow’'][0].data.main[0], + }, + workflowData: expect.objectContaining({ + id: 'workflow-under-test-id', + }), + triggerToStartFrom: expect.objectContaining({ + name: 'When clicking ‘Test workflow’', + }), + }), + ); + }); + + test('should properly choose trigger and start nodes', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + const startNodesData = (testRunnerService as any).getStartNodesData( + wfMultipleTriggersJson, + executionDataMultipleTriggersJson, + ); + + expect(startNodesData).toEqual({ + startNodes: expect.arrayContaining([expect.objectContaining({ name: 'NoOp' })]), + triggerToStartFrom: expect.objectContaining({ + name: 'When clicking ‘Test workflow’', + }), + }); + }); + + test('should properly choose trigger and start nodes 2', async () => { + const testRunnerService = new TestRunnerService( + workflowRepository, + workflowRunner, + executionRepository, + activeExecutions, + testRunRepository, + testMetricRepository, + mockNodeTypes, + ); + + const startNodesData = (testRunnerService as any).getStartNodesData( + wfMultipleTriggersJson, + executionDataMultipleTriggersJson2, + ); + + expect(startNodesData).toEqual({ + startNodes: expect.arrayContaining([expect.objectContaining({ name: 'NoOp' })]), + triggerToStartFrom: expect.objectContaining({ + name: 'When chat message received', + }), + }); + }); }); diff --git a/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts b/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts index 11903581cd..36781d6839 100644 --- a/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts +++ b/packages/cli/src/evaluation/test-runner/test-runner.service.ee.ts @@ -6,6 +6,7 @@ import type { IRunExecutionData, IWorkflowExecutionDataProcess, } from 'n8n-workflow'; +import { NodeConnectionType, Workflow } from 'n8n-workflow'; import assert from 'node:assert'; import { Service } from 'typedi'; @@ -18,6 +19,7 @@ import { ExecutionRepository } from '@/databases/repositories/execution.reposito import { TestMetricRepository } from '@/databases/repositories/test-metric.repository.ee'; import { TestRunRepository } from '@/databases/repositories/test-run.repository.ee'; import { WorkflowRepository } from '@/databases/repositories/workflow.repository'; +import { NodeTypes } from '@/node-types'; import { getRunData } from '@/workflow-execute-additional-data'; import { WorkflowRunner } from '@/workflow-runner'; @@ -41,8 +43,50 @@ export class TestRunnerService { private readonly activeExecutions: ActiveExecutions, private readonly testRunRepository: TestRunRepository, private readonly testMetricRepository: TestMetricRepository, + private readonly nodeTypes: NodeTypes, ) {} + /** + * Prepares the start nodes and trigger node data props for the `workflowRunner.run` method input. + */ + private getStartNodesData( + workflow: WorkflowEntity, + pastExecutionData: IRunExecutionData, + ): Pick { + // Create a new workflow instance to use the helper functions (getChildNodes) + const workflowInstance = new Workflow({ + nodes: workflow.nodes, + connections: workflow.connections, + active: false, + nodeTypes: this.nodeTypes, + }); + + // Determine the trigger node of the past execution + const pastExecutionTriggerNode = getPastExecutionTriggerNode(pastExecutionData); + assert(pastExecutionTriggerNode, 'Could not find the trigger node of the past execution'); + + const triggerNodeData = pastExecutionData.resultData.runData[pastExecutionTriggerNode][0]; + assert(triggerNodeData, 'Trigger node data not found'); + + const triggerToStartFrom = { + name: pastExecutionTriggerNode, + data: triggerNodeData, + }; + + // Start nodes are the nodes that are connected to the trigger node + const startNodes = workflowInstance + .getChildNodes(pastExecutionTriggerNode, NodeConnectionType.Main, 1) + .map((nodeName) => ({ + name: nodeName, + sourceData: { previousNode: pastExecutionTriggerNode }, + })); + + return { + startNodes, + triggerToStartFrom, + }; + } + /** * Runs a test case with the given pin data. * Waits for the workflow under test to finish execution. @@ -56,20 +100,13 @@ export class TestRunnerService { // Create pin data from the past execution data const pinData = createPinData(workflow, mockedNodes, pastExecutionData); - // Determine the start node of the past execution - const pastExecutionStartNode = getPastExecutionTriggerNode(pastExecutionData); - // Prepare the data to run the workflow const data: IWorkflowExecutionDataProcess = { - destinationNode: pastExecutionData.startData?.destinationNode, - startNodes: pastExecutionStartNode - ? [{ name: pastExecutionStartNode, sourceData: null }] - : undefined, + ...this.getStartNodesData(workflow, pastExecutionData), executionMode: 'evaluation', runData: {}, pinData, workflowData: workflow, - partialExecutionVersion: '-1', userId, }; diff --git a/packages/cli/src/evaluation/test-runner/utils.ee.ts b/packages/cli/src/evaluation/test-runner/utils.ee.ts index e608ad6b4a..9cb516b430 100644 --- a/packages/cli/src/evaluation/test-runner/utils.ee.ts +++ b/packages/cli/src/evaluation/test-runner/utils.ee.ts @@ -31,8 +31,8 @@ export function createPinData( } /** - * Returns the start node of the past execution. - * The start node is the node that has no source and has run data. + * Returns the trigger node of the past execution. + * The trigger node is the node that has no source and has run data. */ export function getPastExecutionTriggerNode(executionData: IRunExecutionData) { return Object.keys(executionData.resultData.runData).find((nodeName) => { diff --git a/packages/cli/templates/form-trigger.handlebars b/packages/cli/templates/form-trigger.handlebars index 8d2b13e5a7..ee868b072f 100644 --- a/packages/cli/templates/form-trigger.handlebars +++ b/packages/cli/templates/form-trigger.handlebars @@ -450,7 +450,11 @@