From 7374cbdab026a00552ca0deadf78ae989afbfe13 Mon Sep 17 00:00:00 2001 From: mfsiega <93014743+mfsiega@users.noreply.github.com> Date: Thu, 17 Sep 2026 18:15:35 +0000 Subject: [PATCH] test(engine): Add a Playwright parity project for engine 2.0 (no-changelog) (#38708) Co-authored-by: Claude Fable 5.1 Co-authored-by: Danny Martini --- .github/workflows/ci-pull-requests.yml | 48 +++++- .../testing/containers/n8n-start-stack.ts | 14 +- .../testing/containers/services/engine.ts | 77 ++++++++++ packages/testing/containers/services/n8n.ts | 5 + .../testing/containers/services/postgres.ts | 81 +++++++++- packages/testing/containers/services/types.ts | 4 + packages/testing/containers/stack.ts | 9 ++ .../testing/containers/tests/engine.test.ts | 85 +++++++++++ .../containers/tests/postgres-helper.test.ts | 37 +++++ packages/testing/playwright/README.md | 31 ++++ .../composables/TestEntryComposer.ts | 23 ++- packages/testing/playwright/fixtures/base.ts | 37 ++++- .../playwright/fixtures/engine-parity.test.ts | 70 +++++++++ .../playwright/fixtures/engine-parity.ts | 77 ++++++++++ packages/testing/playwright/package.json | 1 + packages/testing/playwright/pages/n8nPage.ts | 3 + .../testing/playwright/playwright-projects.ts | 18 +++ .../testing/playwright/services/api-helper.ts | 16 +- .../services/workflow-api-helper.test.ts | 139 ++++++++++++++++++ .../services/workflow-api-helper.ts | 76 +++++++++- .../tests/e2e/api/manual-run-outcome.spec.ts | 33 +++++ packages/testing/playwright/vitest.config.ts | 1 + 22 files changed, 858 insertions(+), 27 deletions(-) create mode 100644 packages/testing/containers/services/engine.ts create mode 100644 packages/testing/containers/tests/engine.test.ts create mode 100644 packages/testing/containers/tests/postgres-helper.test.ts create mode 100644 packages/testing/playwright/fixtures/engine-parity.test.ts create mode 100644 packages/testing/playwright/fixtures/engine-parity.ts create mode 100644 packages/testing/playwright/services/workflow-api-helper.test.ts create mode 100644 packages/testing/playwright/tests/e2e/api/manual-run-outcome.spec.ts diff --git a/.github/workflows/ci-pull-requests.yml b/.github/workflows/ci-pull-requests.yml index af70f4f6ded..c50bb991f79 100644 --- a/.github/workflows/ci-pull-requests.yml +++ b/.github/workflows/ci-pull-requests.yml @@ -30,6 +30,7 @@ jobs: db: ${{ fromJSON(steps.ci-filter.outputs.results).db == true }} db_migrations: ${{ fromJSON(steps.ci-filter.outputs.results)['db-migrations'] == true }} e2e_performance: ${{ fromJSON(steps.ci-filter.outputs.results)['e2e-performance'] == true }} + engine_e2e: ${{ fromJSON(steps.ci-filter.outputs.results)['engine-e2e'] == true }} instance_ai_workflow_eval: ${{ fromJSON(steps.ci-filter.outputs.results)['instance-ai-workflow-eval'] == true }} commit_sha: ${{ steps.commit-sha.outputs.sha }} merge_base: ${{ steps.ci-filter.outputs.merge-base }} @@ -145,6 +146,25 @@ jobs: packages/testing/playwright/utils/performance-helper.ts packages/testing/containers/** .github/workflows/test-e2e-performance-reusable.yml + # Whole directories, not the `engine-*` files alone. Engine 2.0 + # routing lives in ordinary control plane code: `workflow-runner.ts` + # and the webhook and trigger paths decide whether a run reaches the + # data plane, and the executions code merges v1 and v2 rows. A path + # this list misses skips `e2e-engine`, and `required-checks` counts + # a skipped job as a pass, so the gap would be silent. + engine-e2e: + packages/@n8n/engine/** + packages/@n8n/node-engine-compatibility/** + packages/@n8n/config/src/configs/engine.config.ts + packages/cli/src/modules/engine-v2/** + packages/cli/src/services/engine-* + packages/cli/src/executions/** + packages/cli/src/webhooks/** + packages/cli/src/workflows/triggers/** + packages/cli/src/workflow-runner.ts + packages/testing/containers/** + packages/testing/playwright/** + .github/workflows/ci-pull-requests.yml instance-ai-workflow-eval: packages/@n8n/instance-ai/src/** packages/@n8n/instance-ai/evaluations/** @@ -314,8 +334,10 @@ jobs: (needs.install-and-build.outputs.runtime == 'true' || needs.install-and-build.outputs.e2e == 'true' || needs.install-and-build.outputs.e2e_performance == 'true' + || needs.install-and-build.outputs.engine_e2e == 'true' || contains(github.event.pull_request.labels.*.name, 'force-e2e') - || contains(github.event.pull_request.labels.*.name, 'run-perf-tests')) + || contains(github.event.pull_request.labels.*.name, 'run-perf-tests') + || contains(github.event.pull_request.labels.*.name, 'force-engine-e2e')) uses: ./.github/workflows/prepare-docker-reusable.yml with: branch: ${{ needs.install-and-build.outputs.commit_sha }} @@ -343,6 +365,29 @@ jobs: artifact-prefix: e2e secrets: inherit + # Engine 2.0 parity: runs the `@engine:*` specs against a main that routes + # every workflow to the new engine. Runs, and blocks, when engine code changes + # or when the PR carries the `force-engine-e2e` label; skipped otherwise. One + # shard is enough while the tagged set is small. Internal PRs only: this + # project has no fork variant yet. + e2e-engine: + name: E2E on engine 2.0 + needs: [install-and-build, prepare-docker] + if: >- + needs.prepare-docker.result == 'success' && + (needs.install-and-build.outputs.engine_e2e == 'true' + || contains(github.event.pull_request.labels.*.name, 'force-engine-e2e')) && + github.event.pull_request.head.repo.fork != true && + github.event_name != 'merge_group' + uses: ./.github/workflows/test-e2e-reusable.yml + with: + branch: ${{ needs.install-and-build.outputs.commit_sha }} + test-mode: docker-artifact + test-command: pnpm --filter=n8n-playwright test:container:engine-v2:e2e + workers: '1' + artifact-prefix: e2e-engine + secrets: inherit + # Boots the editor-ui against the Vite dev server and fails on any console # or page error during load. Catches regressions in dev-mode module # resolution (missing Vite alias, broken workspace package interop) that @@ -441,6 +486,7 @@ jobs: lint, check-packaging, e2e, + e2e-engine, dev-server-smoke, frontend-declarations, db-tests, diff --git a/packages/testing/containers/n8n-start-stack.ts b/packages/testing/containers/n8n-start-stack.ts index de66096b21c..9838eadd784 100644 --- a/packages/testing/containers/n8n-start-stack.ts +++ b/packages/testing/containers/n8n-start-stack.ts @@ -50,6 +50,7 @@ ${colors.yellow}Options:${colors.reset} --services-only Start services only (no n8n containers), write .env for local dev --services Comma-separated services (e.g. postgres,redis,mailpit,proxy,kafka) --postgres Use PostgreSQL instead of SQLite + --engine Run engine 2.0 in the main process (implies --postgres, not for --services-only) --queue Enable queue mode (requires PostgreSQL) --source-control Enable source control (Git) container for testing --oidc Enable OIDC testing with Keycloak (requires PostgreSQL) @@ -141,6 +142,7 @@ async function main() { help: { type: 'boolean', short: 'h' }, 'services-only': { type: 'boolean' }, postgres: { type: 'boolean' }, + engine: { type: 'boolean' }, queue: { type: 'boolean' }, services: { type: 'string' }, 'source-control': { type: 'boolean' }, @@ -194,7 +196,8 @@ async function main() { // Build configuration const config: N8NConfig = { - postgres: values.postgres ?? false, + postgres: Boolean(values.postgres || values.engine), + ...(values.engine ? { engine: 'in-process' as const } : {}), services, projectName: values.name ?? @@ -234,9 +237,13 @@ async function main() { if (values.queue || values.mains || values.workers) { log.warn('Performance plans use SQLite only. Queue mode ignored.'); } + if (values.engine) { + log.warn('Performance plans use SQLite only. Engine 2.0 ignored.'); + } config.resourceQuota = plan; config.postgres = false; // Force SQLite for performance plans + delete config.engine; // Engine 2.0 needs Postgres config.mains = 1; // Force single instance for performance plans config.workers = 0; @@ -267,6 +274,10 @@ async function main() { log.error('No services specified. Use flags like --postgres, --redis, --mailpit, etc.'); process.exit(1); } + if (values.engine) { + // TODO(CAT-4579): write the engine env into `.env` and make the flag work here. + log.warn('Services-only mode starts no n8n. Engine 2.0 ignored.'); + } log.header('Starting service containers'); log.info(`Project: ${config.projectName}`); @@ -453,6 +464,7 @@ function displayConfig(config: N8NConfig) { log.info(`Mode: ${modeStr}`); const enabledFeatures: string[] = []; + if (config.engine) enabledFeatures.push(`Engine 2.0 (${config.engine})`); if (services.includes('gitea')) enabledFeatures.push('Source Control (Gitea)'); if (services.includes('keycloak')) enabledFeatures.push('OIDC (Keycloak)'); if (services.includes('victoriaLogs')) enabledFeatures.push('Observability'); diff --git a/packages/testing/containers/services/engine.ts b/packages/testing/containers/services/engine.ts new file mode 100644 index 00000000000..d842b22f360 --- /dev/null +++ b/packages/testing/containers/services/engine.ts @@ -0,0 +1,77 @@ +/** + * How the stack runs engine 2.0. + * + * `in-process` enables the `engine-v2` backend module, so the data plane runs + * inside the main container. A separate data plane container is not supported + * yet: the standalone engine image has no v1 step executor. + */ +export type EngineMode = 'in-process'; + +export const ENGINE_MODULE = 'engine-v2'; + +/** The data plane keeps its own database on the stack Postgres. */ +export const ENGINE_DATABASE = 'n8n_engine'; + +/** Every value the URL interpolates, so a missing one is named, not printed. */ +const CONNECTION_KEYS = [ + 'DB_POSTGRESDB_USER', + 'DB_POSTGRESDB_PASSWORD', + 'DB_POSTGRESDB_HOST', + 'DB_POSTGRESDB_PORT', +] as const; + +interface EngineEnvOptions { + engine: EngineMode | undefined; + isQueueMode: boolean; +} + +/** + * Refuses a stack that engine 2.0 cannot run on. `createN8NStack` calls this + * before it starts a container, so the run fails in a second instead of after + * a Postgres boot. No-op when `engine` is unset. + */ +export function assertEngineSupported({ + engine, + isQueueMode, + usePostgres, +}: EngineEnvOptions & { usePostgres: boolean }): void { + if (!engine) return; + + if (isQueueMode) { + throw new Error('Engine 2.0 does not support queue mode: use a single main and no workers'); + } + + if (!usePostgres) { + throw new Error('Engine 2.0 needs Postgres: set `postgres: true` on the stack config'); + } +} + +/** + * Adds the env that turns on engine 2.0 to an n8n environment in place. + * + * Reads the `DB_POSTGRESDB_*` values the Postgres service already contributed, + * so the caller never handles credentials. No-op when `engine` is unset. + */ +export function applyEngineEnv( + env: Record, + { engine, isQueueMode }: EngineEnvOptions, +): void { + if (!engine) return; + + assertEngineSupported({ engine, isQueueMode, usePostgres: env.DB_TYPE === 'postgresdb' }); + + // `env` types every value as present, so a missing one would otherwise reach + // the URL as the literal `undefined` and fail at engine boot. + const missing = CONNECTION_KEYS.filter((key) => !env[key]); + if (missing.length > 0) { + throw new Error(`Engine 2.0 needs the Postgres connection env: missing ${missing.join(', ')}`); + } + + const modules = (env.N8N_ENABLED_MODULES ?? '').split(',').filter(Boolean); + if (!modules.includes(ENGINE_MODULE)) modules.push(ENGINE_MODULE); + env.N8N_ENABLED_MODULES = modules.join(','); + + const user = encodeURIComponent(env.DB_POSTGRESDB_USER); + const password = encodeURIComponent(env.DB_POSTGRESDB_PASSWORD); + env.N8N_ENGINE_DATABASE_URL = `postgres://${user}:${password}@${env.DB_POSTGRESDB_HOST}:${env.DB_POSTGRESDB_PORT}/${ENGINE_DATABASE}`; +} diff --git a/packages/testing/containers/services/n8n.ts b/packages/testing/containers/services/n8n.ts index 81d52c667c7..129df720a32 100644 --- a/packages/testing/containers/services/n8n.ts +++ b/packages/testing/containers/services/n8n.ts @@ -12,6 +12,7 @@ import { import { N8nImagePullPolicy } from '../n8n-image-pull-policy'; import type { StartupDeadline } from '../startup-deadline'; import { TEST_CONTAINER_IMAGES } from '../test-containers'; +import { applyEngineEnv, type EngineMode } from './engine'; import type { FileToMount } from './types'; const N8N_IMAGE = TEST_CONTAINER_IMAGES.n8n; @@ -75,6 +76,7 @@ export interface N8NInstancesOptions { serviceEnvironment: Record; userEnvironment?: Record; usePostgres: boolean; + engine?: EngineMode; baseUrl?: string; allocatedPort?: number; resourceQuota?: { memory?: number; cpu?: number }; @@ -116,6 +118,7 @@ function computeEnvironment(options: N8NInstancesOptions): Record = { 'CREATE EXTENSION IF NOT EXISTS pg_stat_statements;', ]); + if (ctx?.config.engine) await createEngineDatabase(container); + return { container, meta: { @@ -108,6 +111,45 @@ export const postgres: Service = { }, }; +/** + * Gives the engine 2.0 data plane its own database, next to the n8n one. The + * separation is deliberate: neither plane can read the other's tables, so a + * cross-plane query fails in a test instead of passing by accident. + * + * `CREATE DATABASE` has no `IF NOT EXISTS`, so ask the catalog first. A reused + * container already has the database, and two stacks can reach this at the + * same time, so a failed create asks again before it gives up. + */ +async function createEngineDatabase(container: StartedPostgreSqlContainer): Promise { + const psql = async (sql: string) => + await container.exec([ + 'psql', + '-U', + container.getUsername(), + '-d', + container.getDatabase(), + '-tAc', + sql, + ]); + + const exists = async () => { + const result = await psql(`SELECT 1 FROM pg_database WHERE datname = '${ENGINE_DATABASE}';`); + + if (result.exitCode !== 0) { + throw new Error(`Failed to look for the engine database: ${result.output}`); + } + + return result.output.trim() === '1'; + }; + + if (await exists()) return; + + const created = await psql(`CREATE DATABASE ${ENGINE_DATABASE};`); + if (created.exitCode === 0 || (await exists())) return; + + throw new Error(`Failed to create the engine database: ${created.output}`); +} + /** Runs introspection SQL via `psql` inside the container. */ export class PostgresHelper { constructor( @@ -117,12 +159,44 @@ export class PostgresHelper { /** Run an arbitrary SQL statement and return the raw psql output. */ async exec(sql: string): Promise { - const result = await this.container.exec([ + const { output } = await this.runIn(this.meta.database, sql); + return output; + } + + /** + * Empties the engine 2.0 data plane database. The E2E reset endpoint cannot: + * it runs on the control plane connection, and Postgres does not read across + * databases. Every table but the migration bookkeeping, so a new engine table + * needs no change here. + */ + async truncateEngineDatabase(): Promise { + const result = await this.runIn( + ENGINE_DATABASE, + `DO $$ + DECLARE target text; + BEGIN + FOR target IN + SELECT tablename FROM pg_tables + WHERE schemaname = 'public' AND tablename <> 'migrations' + LOOP + EXECUTE format('TRUNCATE TABLE %I RESTART IDENTITY CASCADE', target); + END LOOP; + END $$;`, + ); + + // The caller gets no rows back, so a failure has no other way to show. + if (result.exitCode !== 0) { + throw new Error(`Failed to empty the engine database: ${result.output}`); + } + } + + private async runIn(database: string, sql: string) { + return await this.container.exec([ 'psql', '-U', this.meta.username, '-d', - this.meta.database, + database, '-A', // unaligned output '-t', // tuples only (no headers) '-F', @@ -130,7 +204,6 @@ export class PostgresHelper { '-c', sql, ]); - return result.output; } /** Reset pg_stat_statements counters — call before measuring. */ diff --git a/packages/testing/containers/services/types.ts b/packages/testing/containers/services/types.ts index e300c1562bf..43491da19b7 100644 --- a/packages/testing/containers/services/types.ts +++ b/packages/testing/containers/services/types.ts @@ -1,5 +1,7 @@ import type { StartedTestContainer, StartedNetwork } from 'testcontainers'; +import type { EngineMode } from './engine'; + /** Hostname that containers use to reach the host machine (Docker Desktop built-in) */ export const EXTERNAL_HOST = 'host.docker.internal'; @@ -77,6 +79,8 @@ export interface StackConfig { /** Dedicated `n8n webhook` procs. Forces queue mode when > 0. */ webhooks?: number; postgres?: boolean; + /** Runs engine 2.0. Needs `postgres: true`, one main, no workers, no webhook procs. */ + engine?: EngineMode; env?: Record; projectName?: string; resourceQuota?: { memory?: number; cpu?: number }; diff --git a/packages/testing/containers/stack.ts b/packages/testing/containers/stack.ts index 5410ee9db92..4f0aa3914ca 100644 --- a/packages/testing/containers/stack.ts +++ b/packages/testing/containers/stack.ts @@ -9,6 +9,7 @@ import { } from './helpers/utils'; import { waitForNetworkQuiet } from './network-stabilization'; import { ResourceTracker, type CleanupReport } from './resource-tracker'; +import { assertEngineSupported } from './services/engine'; import type { LoadBalancerResult } from './services/load-balancer'; import { createN8NInstances, @@ -120,6 +121,7 @@ export async function createN8NStack(config: N8NConfig = {}): Promise workers = 0, webhooks = 0, postgres: usePostgresConfig = false, + engine, env = {}, projectName, resourceQuota, @@ -144,6 +146,9 @@ export async function createN8NStack(config: N8NConfig = {}): Promise const isQueueMode = mains > 1 || workers > 0 || webhooks > 0; const needsLoadBalancer = mains > 1 || webhooks > 0; const usePostgres = usePostgresConfig || isQueueMode || enabledServices.includes('keycloak'); + + assertEngineSupported({ engine, isQueueMode, usePostgres }); + const uniqueProjectName = projectName ?? `n8n-stack-${Math.random().toString(36).substring(7)}`; let allocatedMainPort: number | undefined; @@ -353,6 +358,7 @@ export async function createN8NStack(config: N8NConfig = {}): Promise serviceEnvironment: environment, userEnvironment: env, usePostgres, + engine, baseUrl: needsLoadBalancer ? undefined : baseUrl, allocatedPort: needsLoadBalancer ? undefined : allocatedMainPort, resourceQuota, @@ -573,6 +579,9 @@ export async function createN8NStack(config: N8NConfig = {}): Promise serviceEnvironment: environment, userEnvironment: { ...env, ...options.env }, usePostgres, + // Without this the replacement main drops the engine-v2 module, and a + // workflow that still asks for engine 2.0 fails far from the cause. + engine, baseUrl, // The same host port keeps `baseUrl` valid across the swap. allocatedPort: allocatedMainPort, diff --git a/packages/testing/containers/tests/engine.test.ts b/packages/testing/containers/tests/engine.test.ts new file mode 100644 index 00000000000..08ea338c3c9 --- /dev/null +++ b/packages/testing/containers/tests/engine.test.ts @@ -0,0 +1,85 @@ +import { describe, expect, test } from 'vitest'; + +import { applyEngineEnv, assertEngineSupported, ENGINE_DATABASE } from '../services/engine'; + +const postgresEnv: Record = { + DB_TYPE: 'postgresdb', + DB_POSTGRESDB_HOST: 'postgres', + DB_POSTGRESDB_PORT: '5432', + DB_POSTGRESDB_DATABASE: 'n8n_db', + DB_POSTGRESDB_USER: 'n8n_user', + DB_POSTGRESDB_PASSWORD: 'test_password', +}; + +describe('applyEngineEnv', () => { + test('leaves the env untouched when no engine mode is set', () => { + const env = { ...postgresEnv }; + + applyEngineEnv(env, { engine: undefined, isQueueMode: false }); + + expect(env).toEqual(postgresEnv); + }); + + test('enables the engine-v2 module', () => { + const env = { ...postgresEnv }; + + applyEngineEnv(env, { engine: 'in-process', isQueueMode: false }); + + expect(env.N8N_ENABLED_MODULES).toBe('engine-v2'); + }); + + test('keeps modules the caller already enabled', () => { + const env = { ...postgresEnv, N8N_ENABLED_MODULES: 'insights,engine-v2' }; + + applyEngineEnv(env, { engine: 'in-process', isQueueMode: false }); + + expect(env.N8N_ENABLED_MODULES).toBe('insights,engine-v2'); + }); + + test('points the data plane at its own database on the stack Postgres', () => { + const env = { ...postgresEnv }; + + applyEngineEnv(env, { engine: 'in-process', isQueueMode: false }); + + expect(env.N8N_ENGINE_DATABASE_URL).toBe( + `postgres://n8n_user:test_password@postgres:5432/${ENGINE_DATABASE}`, + ); + }); + + test('rejects a stack without Postgres', () => { + const env: Record = { DB_TYPE: 'sqlite' }; + + // The connection-env error also says "Postgres", so match what only this + // error says. Otherwise the test passes when the guard is gone. + expect(() => applyEngineEnv(env, { engine: 'in-process', isQueueMode: false })).toThrow( + /set `postgres: true`/, + ); + }); + + test('rejects an env that is missing Postgres connection values', () => { + // Every key the URL reads, so dropping one from the check fails here. + const env: Record = { DB_TYPE: 'postgresdb' }; + + expect(() => applyEngineEnv(env, { engine: 'in-process', isQueueMode: false })).toThrow( + /missing DB_POSTGRESDB_USER, DB_POSTGRESDB_PASSWORD, DB_POSTGRESDB_HOST, DB_POSTGRESDB_PORT/, + ); + }); + + test('rejects queue mode', () => { + const env = { ...postgresEnv }; + + expect(() => applyEngineEnv(env, { engine: 'in-process', isQueueMode: true })).toThrow( + /queue mode/, + ); + }); +}); + +describe('assertEngineSupported', () => { + // `createN8NStack` calls this for every stack, so the no-op keeps the stacks + // that do not run engine 2.0 starting at all. + test('accepts any stack that does not run engine 2.0', () => { + expect(() => + assertEngineSupported({ engine: undefined, isQueueMode: true, usePostgres: false }), + ).not.toThrow(); + }); +}); diff --git a/packages/testing/containers/tests/postgres-helper.test.ts b/packages/testing/containers/tests/postgres-helper.test.ts new file mode 100644 index 00000000000..eb2a83100ac --- /dev/null +++ b/packages/testing/containers/tests/postgres-helper.test.ts @@ -0,0 +1,37 @@ +import type { StartedTestContainer } from 'testcontainers'; +import { describe, expect, test, vi } from 'vitest'; + +import { PostgresHelper } from '../services/postgres'; + +function helperWith(result: { exitCode: number; output: string }) { + const exec = vi.fn().mockResolvedValue(result); + const container = { exec } as unknown as StartedTestContainer; + const helper = new PostgresHelper(container, { + database: 'n8n_db', + username: 'n8n_user', + password: 'test_password', + }); + + return { helper, exec }; +} + +describe('PostgresHelper.truncateEngineDatabase', () => { + test('empties the engine database', async () => { + const { helper, exec } = helperWith({ exitCode: 0, output: '' }); + + await helper.truncateEngineDatabase(); + + const [args] = exec.mock.calls[0] as [string[]]; + expect(args).toContain('n8n_engine'); + expect(args.at(-1)).toContain('TRUNCATE TABLE'); + }); + + test('reports a psql failure instead of leaving the worker on stale rows', async () => { + const { helper } = helperWith({ + exitCode: 1, + output: 'FATAL: database "n8n_engine" does not exist', + }); + + await expect(helper.truncateEngineDatabase()).rejects.toThrow(/does not exist/); + }); +}); diff --git a/packages/testing/playwright/README.md b/packages/testing/playwright/README.md index 3d0b55ef3d5..908ce4c0b56 100644 --- a/packages/testing/playwright/README.md +++ b/packages/testing/playwright/README.md @@ -193,6 +193,37 @@ test('enterprise feature @licensed', ...) // Requires enterprise licen | `@chaostest` | Chaos engineering tests | Tests that intentionally break things | | `@auth:X` | Authentication role (owner, admin, member, none) | Tests requiring specific user role | | `@db:reset` | Reset database before each test (container-only) | Tests that need fresh DB state per test (e.g., MFA tests) | +| `@engine:v2` | Must pass on engine 2.0 as well | Runs under the `engine-v2:e2e` project (see below) | +| `@engine:v1-only` | Engine 2.0 will never support this | Skipped under `engine-v2:e2e`; tracking only | +| `@engine:v2-pending` | Engine 2.0 will support this, but not yet | Expected to fail under `engine-v2:e2e`; an unexpected pass tells you to promote it to `@engine:v2` | + +### Engine 2.0 parity + +The `engine-v2:e2e` project runs the regular `tests/e2e` specs against a stack +that runs engine 2.0 in the main process (`containerConfig.engine: +'in-process'`, Postgres, single main). Under that stack every workflow the API +helpers create gets `settings.engineType = 'v2'`, so a spec proves parity +without changes. + +`@db:reset` clears the control plane only, so data plane execution rows live +on inside a worker. The engine database is emptied once, when the worker takes +its container. + +A workflow built in the UI does not get the setting, and a workflow without it +runs on the legacy engine. A tagged spec must therefore create its workflow +through `api.workflows` and run it through `api.workflows.runManually`, which +fails the test when the run did not reach engine 2.0. A tag the parity buckets +do not know also fails the test, and names the three valid tags. + +The project only picks up specs with an `@engine:*` tag for now. Tag a spec +`@engine:v2` once it passes on both engines; use the other two tags to track +specs that engine 2.0 does not run yet: + +```bash +pnpm --filter=n8n-playwright test:container:engine-v2:e2e tests/e2e/api/manual-run-outcome.spec.ts +``` + +For a local stack with the engine: `pnpm --filter n8n-containers stack --engine`. ### Worker Isolation (Fresh Database) diff --git a/packages/testing/playwright/composables/TestEntryComposer.ts b/packages/testing/playwright/composables/TestEntryComposer.ts index 059f38d4477..81b60cc7976 100644 --- a/packages/testing/playwright/composables/TestEntryComposer.ts +++ b/packages/testing/playwright/composables/TestEntryComposer.ts @@ -2,6 +2,7 @@ import type { Page } from '@playwright/test'; import { setupDefaultInterceptors } from '../config/intercepts'; import type { n8nPage } from '../pages/n8nPage'; +import { ApiHelpers } from '../services/api-helper'; import type { TestUser } from '../services/user-api-helper'; /** @@ -91,9 +92,7 @@ export class TestEntryComposer { await action(); const newPage = await newPagePromise; await newPage.waitForLoadState('domcontentloaded'); - // Use the constructor from the current instance to avoid circular dependency - const n8nPageConstructor = this.n8n.constructor as new (page: Page) => n8nPage; - return new n8nPageConstructor(newPage); + return this.wrapPage(newPage); } /** @@ -104,8 +103,20 @@ export class TestEntryComposer { */ async newTab(): Promise { const newPage = await this.n8n.page.context().newPage(); - const n8nPageConstructor = this.n8n.constructor as new (page: Page) => n8nPage; - return new n8nPageConstructor(newPage); + return this.wrapPage(newPage); + } + + /** + * Wraps a page in a new n8nPage that keeps this instance's API options, so a + * workflow created from the new page lands on the same engine. + * Uses the constructor from the current instance to avoid a circular import. + */ + private wrapPage(page: Page): n8nPage { + const n8nPageConstructor = this.n8n.constructor as new (page: Page, api: ApiHelpers) => n8nPage; + return new n8nPageConstructor( + page, + new ApiHelpers(page.context().request, this.n8n.api.options), + ); } /** @@ -133,7 +144,7 @@ export class TestEntryComposer { const context = await browser.newContext(); await setupDefaultInterceptors(context); const page = await context.newPage(); - const newN8n = new (this.n8n.constructor as new (page: Page) => n8nPage)(page); + const newN8n = this.wrapPage(page); await newN8n.api.login({ email: user.email, password: user.password }); return newN8n; } diff --git a/packages/testing/playwright/fixtures/base.ts b/packages/testing/playwright/fixtures/base.ts index c2da458325f..b56f08ce6c8 100644 --- a/packages/testing/playwright/fixtures/base.ts +++ b/packages/testing/playwright/fixtures/base.ts @@ -12,6 +12,7 @@ import { type CapabilityOption, } from './capabilities'; import { consoleErrorFixtures } from './console-error-monitor'; +import { engineParityDisposition, workflowSettingsFor } from './engine-parity'; import { N8N_AUTH_COOKIE } from '../config/constants'; import { setupDefaultInterceptors } from '../config/intercepts'; import { backendV8CoverageFixtures } from '../fixtures/backend-v8-coverage'; @@ -51,6 +52,8 @@ type TestFixtures = { * unless COVERAGE_ENABLED. */ backendCoverage: undefined; containerRequirement: undefined; + /** Internal auto fixture: sorts a test into its engine 2.0 parity bucket by tag. */ + engineParity: undefined; }; type WorkerFixtures = { @@ -105,6 +108,18 @@ export const test = base.extend< // Option for test.use({ capability: 'proxy' }) - transformed into N8NStack by n8nContainer capability: [undefined, { scope: 'worker', option: true }], + // Rejects an unknown @engine:* tag anywhere; only an engine 2.0 stack skips or + // expects failure. See fixtures/engine-parity.ts for the tags. + engineParity: [ + async ({ n8nStackConfig }, use, testInfo) => { + const disposition = engineParityDisposition(testInfo.tags, n8nStackConfig.engine); + if (disposition.action === 'skip') testInfo.skip(true, disposition.reason); + if (disposition.action === 'expect-fail') testInfo.fail(true, disposition.reason); + await use(undefined); + }, + { auto: true }, + ], + // Service requirements now come from test.use(), so local projects cannot filter them by title. containerRequirement: [ async ({ capability }, use, testInfo) => { @@ -211,13 +226,16 @@ export const test = base.extend< ], dbSetup: [ - async ({ n8nContainer }, use) => { + async ({ n8nContainer, n8nStackConfig }, use) => { if (n8nContainer) { console.log('Resetting database for new container'); const apiContext = await request.newContext({ baseURL: n8nContainer.baseUrl }); const api = new ApiHelpers(apiContext); await api.resetDatabase(); await apiContext.dispose(); + + // The reset endpoint only reaches the control plane database. + if (n8nStackConfig.engine) await n8nContainer.services.postgres.truncateEngineDatabase(); } await use(undefined); }, @@ -229,7 +247,8 @@ export const test = base.extend< await use(frontendUrl); }, - n8n: async ({ context, backendUrl, frontendUrl }, use, testInfo) => { + n8n: async ({ context, backendUrl, frontendUrl, n8nStackConfig }, use, testInfo) => { + const apiOptions = { workflowSettings: workflowSettingsFor(n8nStackConfig) }; await setupDefaultInterceptors(context); const page = await context.newPage(); @@ -243,7 +262,7 @@ export const test = base.extend< if (useSeparateApiContext) { const apiContext = await request.newContext({ baseURL: backendUrl }); - const api = new ApiHelpers(apiContext); + const api = new ApiHelpers(apiContext, apiOptions); const n8nInstance = new n8nPage(page, api); await n8nInstance.api.setupFromTags(testInfo.tags); @@ -291,16 +310,16 @@ export const test = base.extend< await use(n8nInstance); await apiContext.dispose(); } else { - const n8nInstance = new n8nPage(page); + const n8nInstance = new n8nPage(page, new ApiHelpers(page.context().request, apiOptions)); await n8nInstance.api.setupFromTags(testInfo.tags); await n8nInstance.start.withProjectFeatures(); await use(n8nInstance); } }, - api: async ({ backendUrl }, use, testInfo) => { + api: async ({ backendUrl, n8nStackConfig }, use, testInfo) => { const context = await request.newContext({ baseURL: backendUrl }); - const api = new ApiHelpers(context); + const api = new ApiHelpers(context, { workflowSettings: workflowSettingsFor(n8nStackConfig) }); await api.setupFromTags(testInfo.tags); const hasAuthTag = testInfo.tags.some((tag) => tag.startsWith('@auth:')); @@ -320,7 +339,7 @@ export const test = base.extend< await use(urls); }, - createApiForMain: async ({ n8nContainer }, use, testInfo) => { + createApiForMain: async ({ n8nContainer, n8nStackConfig }, use, testInfo) => { const contexts: Array<{ dispose: () => Promise }> = []; const createApi = async (mainIndex: number): Promise => { @@ -335,7 +354,9 @@ export const test = base.extend< const context = await request.newContext({ baseURL: mainUrls[mainIndex] }); contexts.push(context); - const api = new ApiHelpers(context); + const api = new ApiHelpers(context, { + workflowSettings: workflowSettingsFor(n8nStackConfig), + }); await api.setupFromTags(testInfo.tags.filter((tag) => tag.toLowerCase() !== '@db:reset')); const hasAuthTag = testInfo.tags.some((tag) => tag.startsWith('@auth:')); diff --git a/packages/testing/playwright/fixtures/engine-parity.test.ts b/packages/testing/playwright/fixtures/engine-parity.test.ts new file mode 100644 index 00000000000..aa201a93b11 --- /dev/null +++ b/packages/testing/playwright/fixtures/engine-parity.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, test } from 'vitest'; + +import { + ENGINE_TAG_PREFIX, + ENGINE_TAGS, + engineParityDisposition, + workflowSettingsFor, +} from './engine-parity'; + +describe('ENGINE_TAGS', () => { + test('every tag starts with the prefix the engine project greps for', () => { + for (const tag of Object.values(ENGINE_TAGS)) { + expect(tag.startsWith(ENGINE_TAG_PREFIX)).toBe(true); + } + }); +}); + +describe('workflowSettingsFor', () => { + test('returns no defaults for a stack without engine 2.0', () => { + expect(workflowSettingsFor({ postgres: true })).toBeUndefined(); + }); + + test('opts every workflow into engine 2.0 when the stack runs it', () => { + expect(workflowSettingsFor({ postgres: true, engine: 'in-process' })).toEqual({ + engineType: 'v2', + }); + }); +}); + +describe('engineParityDisposition', () => { + test('runs every test as-is on the legacy engine, whatever its tag', () => { + for (const tag of Object.values(ENGINE_TAGS)) { + expect(engineParityDisposition([tag], undefined)).toEqual({ action: 'run' }); + } + }); + + test('runs an untagged test on engine 2.0', () => { + expect(engineParityDisposition(['@auth:owner'], 'in-process')).toEqual({ action: 'run' }); + }); + + test('runs a test tagged as supported on engine 2.0', () => { + expect(engineParityDisposition([ENGINE_TAGS.supported], 'in-process')).toEqual({ + action: 'run', + }); + }); + + test('skips a test that engine 2.0 will never support', () => { + expect(engineParityDisposition([ENGINE_TAGS.unsupported], 'in-process')).toMatchObject({ + action: 'skip', + reason: expect.stringContaining('never'), + }); + }); + + test('expects a test that engine 2.0 does not support yet to fail', () => { + expect(engineParityDisposition([ENGINE_TAGS.pending], 'in-process')).toMatchObject({ + action: 'expect-fail', + reason: expect.stringContaining('yet'), + }); + }); + + test('rejects a misspelled engine tag instead of treating it as supported', () => { + expect(() => engineParityDisposition(['@engine:v2-pendign'], 'in-process')).toThrow( + /@engine:v2-pendign/, + ); + }); + + test('rejects a misspelled engine tag on the legacy engine too', () => { + expect(() => engineParityDisposition(['@engine:v1only'], undefined)).toThrow(/@engine:v1only/); + }); +}); diff --git a/packages/testing/playwright/fixtures/engine-parity.ts b/packages/testing/playwright/fixtures/engine-parity.ts new file mode 100644 index 00000000000..72e92393579 --- /dev/null +++ b/packages/testing/playwright/fixtures/engine-parity.ts @@ -0,0 +1,77 @@ +import type { N8NConfig } from 'n8n-containers/stack'; +import type { IWorkflowSettings } from 'n8n-workflow'; + +/** The `engine-v2:e2e` project greps for this prefix, so every tag below is selected. */ +export const ENGINE_TAG_PREFIX = '@engine:'; + +/** + * Title tags that sort a spec into an engine 2.0 parity bucket. They only + * matter under a stack that runs engine 2.0 (`engine-v2:e2e`); everywhere + * else the spec runs as usual. + */ +export const ENGINE_TAGS = { + /** Must pass on both engines. Runs there. */ + supported: `${ENGINE_TAG_PREFIX}v2`, + /** Engine 2.0 will never support this behaviour. Skipped there. */ + unsupported: `${ENGINE_TAG_PREFIX}v1-only`, + /** Engine 2.0 will support this, but does not yet. Expected to fail there. */ + pending: `${ENGINE_TAG_PREFIX}v2-pending`, +} as const; + +/** + * A tag that starts with the prefix but names no bucket is a typo. The + * `engine-v2:e2e` project greps for the prefix, so it would be selected and + * then treated as supported: a spec meant to be skipped or to fail becomes a + * required green, and the required check goes red with nothing naming the tag. + */ +function assertKnownTags(tags: readonly string[]): void { + const known: readonly string[] = Object.values(ENGINE_TAGS); + const unknown = tags.filter((tag) => tag.startsWith(ENGINE_TAG_PREFIX) && !known.includes(tag)); + + if (unknown.length > 0) { + // The separator carries no behaviour, and pinning the whole sentence to catch + // it breaks on any rewording. + throw new Error( + // Stryker disable next-line StringLiteral + `Unknown engine 2.0 tag: ${unknown.join(', ')}. Use one of: ${known.join(', ')}.`, + ); + } +} + +export type EngineParityDisposition = + | { action: 'run' } + | { action: 'skip' | 'expect-fail'; reason: string }; + +/** + * What the parity fixture does with a test, given its tags and the engine + * mode of the stack it runs on. + */ +export function engineParityDisposition( + tags: readonly string[], + engine: N8NConfig['engine'], +): EngineParityDisposition { + assertKnownTags(tags); + + if (!engine) return { action: 'run' }; + + if (tags.includes(ENGINE_TAGS.unsupported)) { + return { action: 'skip', reason: 'Engine 2.0 will never support this test' }; + } + + if (tags.includes(ENGINE_TAGS.pending)) { + return { action: 'expect-fail', reason: 'Engine 2.0 does not support this test yet' }; + } + + return { action: 'run' }; +} + +/** + * Workflow settings every API-created workflow gets on this stack. A stack + * that runs engine 2.0 routes every workflow to it, so the same spec proves + * parity without changes. + */ +export function workflowSettingsFor(config: N8NConfig): Partial | undefined { + if (!config.engine) return undefined; + + return { engineType: 'v2' }; +} diff --git a/packages/testing/playwright/package.json b/packages/testing/playwright/package.json index dba97cc6ab2..6ec8fcfba3b 100644 --- a/packages/testing/playwright/package.json +++ b/packages/testing/playwright/package.json @@ -26,6 +26,7 @@ "test:container:queue": "playwright test --project='queue:*'", "test:container:multi-main": "playwright test --project='multi-main:*'", "test:container:multi-main:e2e": "playwright test --project='multi-main:e2e'", + "test:container:engine-v2:e2e": "playwright test --project='engine-v2:e2e'", "test:container:coverage": "COVERAGE_ENABLED=true N8N_COVERAGE_DIR=\"$PWD/coverage/.backend-v8\" playwright test --project=coverage", "test:workflows:setup": "tsx ./tests/cli-workflows/setup-workflow-tests.ts", "test:workflows": "playwright test --project=cli-workflows", diff --git a/packages/testing/playwright/pages/n8nPage.ts b/packages/testing/playwright/pages/n8nPage.ts index 15bcb465d33..e5321a67b0b 100644 --- a/packages/testing/playwright/pages/n8nPage.ts +++ b/packages/testing/playwright/pages/n8nPage.ts @@ -163,6 +163,9 @@ export class n8nPage { constructor(page: Page, api?: ApiHelpers) { this.page = page; + // The fallback helper carries no options, so it applies no stack workflow + // settings and its engine 2.0 routing check stays quiet. Pass a helper, or + // use `start.newTab()`, for a page that creates or runs workflows. this.api = api ?? new ApiHelpers(page.context().request); // Pages diff --git a/packages/testing/playwright/playwright-projects.ts b/packages/testing/playwright/playwright-projects.ts index 5105396dc17..0decc5585e4 100644 --- a/packages/testing/playwright/playwright-projects.ts +++ b/packages/testing/playwright/playwright-projects.ts @@ -2,6 +2,7 @@ import type { Project } from '@playwright/test'; import type { N8NConfig } from 'n8n-containers/stack'; import { ALLOW_CONTAINER_ONLY, CONTAINER_ONLY_MODES, LICENSED_TAG } from './fixtures/capabilities'; +import { ENGINE_TAG_PREFIX } from './fixtures/engine-parity'; import { getBackendUrl, getFrontendUrl } from './utils/url-helper'; // Tests that require container environment (won't run against local n8n). @@ -235,6 +236,23 @@ export function getProjects(): Project[] { ); } + // Engine 2.0 parity: the same e2e specs against a main that routes every + // workflow to the new engine. Opt-in by tag while the engine matures: any + // `@engine:*` tag selects the spec, and the parity fixture then runs, skips + // or expects failure by bucket. Drop the grep once the suite is triaged. + // The CI job e2e-engine blocks merges, so a spec this grep selects fails the + // PR when it misses the outcome its bucket asks for. + projects.push({ + name: 'engine-v2:e2e', + testDir: './tests/e2e', + grep: new RegExp(ENGINE_TAG_PREFIX), + timeout: 180000, + // One worker, one stack. Every worker boots its own Postgres and main, + // and the CI job asks for one worker anyway. + workers: 1, + use: { containerConfig: { postgres: true, engine: 'in-process' } }, + }); + projects.push({ name: 'coverage', testDir: './tests/e2e', diff --git a/packages/testing/playwright/services/api-helper.ts b/packages/testing/playwright/services/api-helper.ts index c64eba94a4b..f7535bad00d 100644 --- a/packages/testing/playwright/services/api-helper.ts +++ b/packages/testing/playwright/services/api-helper.ts @@ -7,6 +7,7 @@ import type { InstanceAiThreadInfo, } from '@n8n/api-types'; import { request, type APIRequestContext } from '@playwright/test'; +import type { IWorkflowSettings } from 'n8n-workflow'; import { setTimeout as wait } from 'node:timers/promises'; import type { UserCredentials } from '../config/test-users'; @@ -36,6 +37,14 @@ import { VariablesApiHelper } from './variables-api-helper'; import { WebhookApiHelper } from './webhook-api-helper'; import { WorkflowApiHelper } from './workflow-api-helper'; +export interface ApiHelpersOptions { + /** + * Settings merged over every workflow this helper creates. Set per stack, so + * a project that runs engine 2.0 routes every workflow to it. + */ + workflowSettings?: Partial; +} + export interface LoginResponseData { id: string; [key: string]: unknown; @@ -90,7 +99,10 @@ export class ApiHelpers { publicApi: PublicApiHelper; - constructor(requestContext: APIRequestContext) { + constructor( + requestContext: APIRequestContext, + readonly options: ApiHelpersOptions = {}, + ) { this.request = requestContext; this.workflows = new WorkflowApiHelper(this); this.webhooks = new WebhookApiHelper(this); @@ -411,7 +423,7 @@ export class ApiHelpers { */ async createApiForUser(user: Pick): Promise { const userContext = await request.newContext(); - const userApi = new ApiHelpers(userContext); + const userApi = new ApiHelpers(userContext, this.options); await userApi.login({ email: user.email, password: user.password }); return userApi; } diff --git a/packages/testing/playwright/services/workflow-api-helper.test.ts b/packages/testing/playwright/services/workflow-api-helper.test.ts new file mode 100644 index 00000000000..22960112f2d --- /dev/null +++ b/packages/testing/playwright/services/workflow-api-helper.test.ts @@ -0,0 +1,139 @@ +import { describe, expect, test, vi } from 'vitest'; + +import type { ApiHelpers } from './api-helper'; +import { WorkflowApiHelper } from './workflow-api-helper'; + +function apiWith(options: ApiHelpers['options']) { + const post = vi.fn().mockResolvedValue({ + ok: () => true, + json: async () => ({ data: { id: 'wf-1', versionId: 'v-1' } }), + }); + const api = { request: { post }, options } as unknown as ApiHelpers; + return { api, post }; +} + +function postedSettings(post: ReturnType) { + const [, { data }] = post.mock.calls[0] as [string, { data: { settings?: unknown } }]; + return data.settings; +} + +describe('WorkflowApiHelper workflow settings defaults', () => { + test('leaves a workflow without settings untouched when there are no defaults', async () => { + const { api, post } = apiWith({}); + + await new WorkflowApiHelper(api).createWorkflow({ name: 'wf' }); + + expect(postedSettings(post)).toBeUndefined(); + }); + + test('applies the defaults to a created workflow', async () => { + const { api, post } = apiWith({ workflowSettings: { engineType: 'v2' } }); + + await new WorkflowApiHelper(api).createWorkflow({ name: 'wf' }); + + expect(post).toHaveBeenCalledWith('/rest/workflows', expect.anything()); + expect(postedSettings(post)).toEqual({ engineType: 'v2' }); + }); + + test('puts a created workflow in the project the caller names', async () => { + const { api, post } = apiWith({}); + + await new WorkflowApiHelper(api).createWorkflow({ name: 'wf' }, 'project-1'); + + const [, { data }] = post.mock.calls[0] as [string, { data: unknown }]; + expect(data).toMatchObject({ name: 'wf', projectId: 'project-1' }); + }); + + test('lets the defaults win over the settings in the definition', async () => { + const { api, post } = apiWith({ workflowSettings: { engineType: 'v2' } }); + + await new WorkflowApiHelper(api).createWorkflow({ + name: 'wf', + settings: { executionOrder: 'v1', engineType: 'v1' }, + }); + + expect(postedSettings(post)).toEqual({ executionOrder: 'v1', engineType: 'v2' }); + }); + + test('applies the defaults to a workflow created in a project', async () => { + const { api, post } = apiWith({ workflowSettings: { engineType: 'v2' } }); + + await new WorkflowApiHelper(api).createInProject('project-1'); + + expect(postedSettings(post)).toEqual({ engineType: 'v2' }); + }); +}); + +describe('WorkflowApiHelper.waitForExecutionById', () => { + function apiReturning(statuses: string[]) { + const get = vi.fn(); + for (const status of statuses) { + get.mockResolvedValueOnce({ + ok: () => true, + json: async () => ({ data: { id: 'exec-1', status } }), + }); + } + return { api: { request: { get }, options: {} } as unknown as ApiHelpers, get }; + } + + test('returns the execution once it reaches a settled status', async () => { + const { api, get } = apiReturning(['running', 'running', 'success']); + + const execution = await new WorkflowApiHelper(api).waitForExecutionById('exec-1', 2000, 0); + + expect(execution.status).toBe('success'); + expect(get).toHaveBeenCalledTimes(3); + }); + + test('returns a failed execution instead of waiting for success', async () => { + const { api } = apiReturning(['error']); + + const execution = await new WorkflowApiHelper(api).waitForExecutionById('exec-1', 2000, 0); + + expect(execution.status).toBe('error'); + }); + + test('throws when the execution does not settle in time', async () => { + const { api } = apiReturning(Array(50).fill('running')); + + await expect(new WorkflowApiHelper(api).waitForExecutionById('exec-1', 20, 5)).rejects.toThrow( + /did not settle/, + ); + }); +}); + +describe('WorkflowApiHelper.runManually engine routing', () => { + function apiReturningExecutionId(executionId: string, options: ApiHelpers['options']) { + const post = vi.fn().mockResolvedValue({ + ok: () => true, + json: async () => ({ data: { executionId } }), + }); + return { api: { request: { post }, options } as unknown as ApiHelpers }; + } + + test('accepts the engine 2.0 execution id a routed run mints', async () => { + const id = '0199c3a1-8f4e-7c2b-9a1d-2f6b8e4c1a77'; + const { api } = apiReturningExecutionId(id, { workflowSettings: { engineType: 'v2' } }); + + await expect(new WorkflowApiHelper(api).runManually('wf-1', 'Trigger')).resolves.toEqual({ + executionId: id, + }); + }); + + test('rejects a legacy execution id when the stack routes to engine 2.0', async () => { + const { api } = apiReturningExecutionId('1783', { workflowSettings: { engineType: 'v2' } }); + + // The guidance half of the message is part of the contract, so match it too. + await expect(new WorkflowApiHelper(api).runManually('wf-1', 'Trigger')).rejects.toThrow( + /1783[\s\S]*settings\.engineType[\s\S]*api\.workflows/, + ); + }); + + test('accepts a legacy execution id on a stack without engine 2.0', async () => { + const { api } = apiReturningExecutionId('1783', {}); + + await expect(new WorkflowApiHelper(api).runManually('wf-1', 'Trigger')).resolves.toEqual({ + executionId: '1783', + }); + }); +}); diff --git a/packages/testing/playwright/services/workflow-api-helper.ts b/packages/testing/playwright/services/workflow-api-helper.ts index 2b8b91792b4..9536ea08c1e 100644 --- a/packages/testing/playwright/services/workflow-api-helper.ts +++ b/packages/testing/playwright/services/workflow-api-helper.ts @@ -1,6 +1,6 @@ import type { APIResponse } from '@playwright/test'; import { readFileSync } from 'fs'; -import type { IWorkflowBase, ExecutionSummary } from 'n8n-workflow'; +import { isTerminalExecutionStatus, type IWorkflowBase, type ExecutionSummary } from 'n8n-workflow'; import { nanoid } from 'nanoid'; // Type for execution responses from the n8n API @@ -23,12 +23,16 @@ type WorkflowImportResult = { webhookMethod?: 'GET' | 'POST' | 'PUT' | 'DELETE' | 'PATCH' | 'HEAD'; }; +/** Engine 2.0 mints uuidv7 execution ids; the legacy engine mints numbers. */ +const UUID = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + export class WorkflowApiHelper { constructor(private api: ApiHelpers) {} async createWorkflow(workflow: Partial, projectId?: string) { + const data = this.withDefaultSettings(workflow); const response = await this.api.request.post('/rest/workflows', { - data: projectId ? { ...workflow, projectId } : workflow, + data: projectId ? { ...data, projectId } : data, }); if (!response.ok()) { @@ -49,7 +53,7 @@ export class WorkflowApiHelper { ): Promise<{ name: string; id: string; versionId: string }> { const workflowName = options?.name ?? `Test Workflow ${nanoid(8)}`; - const workflow = { + const workflow = this.withDefaultSettings({ name: workflowName, nodes: [], connections: {}, @@ -57,7 +61,7 @@ export class WorkflowApiHelper { active: false, projectId: project, ...(options?.folder && { parentFolderId: options.folder }), - }; + }); const response = await this.api.request.post('/rest/workflows', { data: workflow }); @@ -75,6 +79,14 @@ export class WorkflowApiHelper { }; } + /** The stack-wide defaults win, so a spec proves parity on whatever engine the stack runs. */ + private withDefaultSettings>(workflow: T): T { + const defaults = this.api.options.workflowSettings; + if (!defaults) return workflow; + + return { ...workflow, settings: { ...workflow.settings, ...defaults } }; + } + async activate(workflowId: string, versionId: string) { const response = await this.api.request.post(`/rest/workflows/${workflowId}/activate`, { data: { versionId }, @@ -118,7 +130,32 @@ export class WorkflowApiHelper { } const result = await response.json(); - return result.data ?? result; + const run = (result.data ?? result) as { executionId: string }; + this.assertRoutedToEngine(run.executionId); + return run; + } + + /** + * Fails a run that the stack meant for engine 2.0 but the control plane kept. + * A v1 id is numeric and a v2 id is a uuid, which the engine validates on the + * wire (`packages/cli/src/executions/execution-id.ts`), so the shape says + * which plane ran it. Without this a workflow that missed `engineType` runs + * on v1 and the spec still passes, which is parity evidence that proves + * nothing. + * + * Only the manual-run entry point is checked. A spec that starts a run + * through a webhook or a trigger gets no such guard yet. + */ + private assertRoutedToEngine(executionId: string): void { + if (this.api.options.workflowSettings?.engineType !== 'v2') return; + if (UUID.test(executionId)) return; + + throw new TestError( + `Expected an engine 2.0 execution id, got "${executionId}", so the run stayed on the ` + + 'legacy engine. Either the workflow missed `settings.engineType`, which ' + + '`api.workflows` applies from the stack, or this main does not run the `engine-v2` ' + + 'module.', + ); } /** @@ -406,6 +443,35 @@ export class WorkflowApiHelper { return result.data ?? result; } + /** + * Polls one execution by id until it settles. {@link waitForExecution} watches + * the list for a row that was not there before and matches it by mode, which + * needs the list to page and order the way it expects. This takes the id the + * run already returned, so nothing about the list can mislead it. + */ + async waitForExecutionById( + executionId: string, + timeoutMs = 10000, + pollIntervalMs = 250, + ): Promise { + const deadline = Date.now() + timeoutMs; + + let execution = await this.getExecution(executionId); + while (!isTerminalExecutionStatus(execution.status)) { + // Stryker disable next-line EqualityOperator: `>` differs only on the exact + // deadline millisecond, which costs fake timers to reach and proves nothing. + if (Date.now() >= deadline) { + throw new TestError( + `Execution ${executionId} did not settle within ${timeoutMs}ms (status: ${execution.status})`, + ); + } + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + execution = await this.getExecution(executionId); + } + + return execution; + } + /** Stops a running or waiting execution and returns the stopped execution summary. */ async stopExecution(executionId: string): Promise { const response = await this.api.request.post(`/rest/executions/${executionId}/stop`); diff --git a/packages/testing/playwright/tests/e2e/api/manual-run-outcome.spec.ts b/packages/testing/playwright/tests/e2e/api/manual-run-outcome.spec.ts new file mode 100644 index 00000000000..9b129eb4d85 --- /dev/null +++ b/packages/testing/playwright/tests/e2e/api/manual-run-outcome.spec.ts @@ -0,0 +1,33 @@ +import flatted from 'flatted'; + +import { test, expect } from '../../../fixtures/base'; + +// Manual Trigger -> Set (test = 'a') -> If (test == 'b') -> NoOp | Set (test2 = paired item's test) +const WORKFLOW_FILE = 'Test_Workflow_pairedItem_incomplete_manual_bug.json'; +const TRIGGER_NAME = 'When clicking ‘Execute workflow’'; + +test.describe( + 'Manual run outcome', + { + annotation: [{ type: 'owner', description: 'Catalysts' }], + }, + () => { + test('should run a branching workflow and report its output through the executions API @engine:v2', async ({ + api, + }) => { + const { workflowId } = await api.workflows.importWorkflowFromFile(WORKFLOW_FILE); + + const { executionId } = await api.workflows.runManually(workflowId, TRIGGER_NAME); + const execution = await api.workflows.waitForExecutionById(executionId); + + expect(execution.status).toBe('success'); + + // The false branch ran, and its Set read the paired item from two nodes + // upstream, so `test2` carries what `Init Data` put in `test`. + const { resultData } = flatted.parse(execution.data); + const output = resultData.runData['Test Expression'][0].data.main[0][0].json; + // "Include Other Input Fields" is off, so test2 is the only key. + expect(output).toEqual({ test2: 'a' }); + }); + }, +); diff --git a/packages/testing/playwright/vitest.config.ts b/packages/testing/playwright/vitest.config.ts index 856df864b64..651e0bc5cbc 100644 --- a/packages/testing/playwright/vitest.config.ts +++ b/packages/testing/playwright/vitest.config.ts @@ -8,6 +8,7 @@ export default defineConfig({ 'scripts/**/*.test.{ts,mjs}', 'fixtures/**/*.test.ts', 'reporters/**/*.test.ts', + 'services/**/*.test.ts', 'utils/**/*.test.ts', '*.test.ts', ],