mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-19 08:36:02 +02:00
test(workflow): retire legacy expression test project
This commit is contained in:
parent
85963cfa5f
commit
bc8fac9bd6
|
|
@ -3,8 +3,7 @@ export default {
|
|||
packageManager: 'pnpm',
|
||||
testRunner: 'vitest',
|
||||
plugins: ['@stryker-mutator/vitest-runner'],
|
||||
// Use the Stryker-specific vitest config (legacy-engine only) — see
|
||||
// vitest.stryker.config.ts for the rationale.
|
||||
// Use the QuickJS project to avoid VM teardown failures in Stryker workers.
|
||||
vitest: {
|
||||
configFile: 'vitest.stryker.config.ts',
|
||||
},
|
||||
|
|
@ -23,7 +22,7 @@ export default {
|
|||
jsonReporter: { fileName: 'reports/mutation/raw.json' },
|
||||
// Cap low: a string/utils mutant running 15s+ is a spinning loop-condition mutant; fail it fast.
|
||||
timeoutMS: 15_000,
|
||||
// Each Stryker worker spawns vitest with one project (legacy-engine, via
|
||||
// Each Stryker worker spawns vitest with one project (QuickJS, via
|
||||
// vitest.stryker.config.ts). Default 4 is fine on CI runners; lower
|
||||
// locally if you hit OOM during the initial dry run via
|
||||
// `STRYKER_CONCURRENCY=1`.
|
||||
|
|
|
|||
49
packages/workflow/test/EXPRESSION_ENGINE_VALIDATION.md
Normal file
49
packages/workflow/test/EXPRESSION_ENGINE_VALIDATION.md
Normal file
|
|
@ -0,0 +1,49 @@
|
|||
# Expression engine test matrix
|
||||
|
||||
The workflow Vitest suite runs on VM and QuickJS. Both projects run the same
|
||||
expression contracts. The runtime `legacy` option remains available, but the
|
||||
normal suite no longer compares each case against it. Stryker uses QuickJS
|
||||
because VM teardown aborts in its workers (isolated-vm #464).
|
||||
|
||||
## Local performance check
|
||||
|
||||
Run from `packages/testing/performance`:
|
||||
|
||||
```bash
|
||||
pnpm bench benchmarks/expression-engine/patterns-vm.bench.ts benchmarks/expression-engine/patterns-legacy.bench.ts
|
||||
```
|
||||
|
||||
One local run on Node 24.15.0 measured warm expression evaluation through
|
||||
`Workflow.expression`. Lower mean latency is better. Results depend on the
|
||||
runner; these numbers are not a CI performance baseline.
|
||||
|
||||
| Workload | VM mean | Legacy mean |
|
||||
| --- | ---: | ---: |
|
||||
| Simple property, small data | 0.159 ms | 0.033 ms |
|
||||
| Nested property, depth 4 | 0.185 ms | 0.034 ms |
|
||||
| Extension call, `isEmpty` | 0.217 ms | 0.036 ms |
|
||||
| Map 100 items | 0.830 ms | 0.037 ms |
|
||||
| Filter 100 items | 1.701 ms | 0.036 ms |
|
||||
| Map 10,000 items | 61.665 ms | 0.054 ms |
|
||||
|
||||
VM is slower on these workloads, especially on array iteration. The ticket
|
||||
does not state a numeric acceptance limit. This run does not establish that
|
||||
VM meets a performance bar. Use the same benchmark on production-like hardware
|
||||
and agree on a limit before treating performance as validated.
|
||||
|
||||
## Compatibility check
|
||||
|
||||
Before removing the legacy project, the compatibility corpus, array proxy,
|
||||
and `$item(index)` suites passed on legacy, VM, and QuickJS (231 tests per
|
||||
project). All three projects shared the same contract cases. The only
|
||||
engine-specific expected result in that corpus was `Number.format()`:
|
||||
legacy returned `undefined`; VM and QuickJS return the unformatted number
|
||||
as a string. Array and object writes also differ: legacy changes the input
|
||||
data, while VM and QuickJS keep writes within the evaluation. The VM and
|
||||
QuickJS assertions still cover both behaviors they share. The matrix no
|
||||
longer checks new changes against legacy automatically.
|
||||
|
||||
After the change, `pnpm test` in `packages/workflow` passed 180 test files
|
||||
across VM and QuickJS (6,521 passed; one skipped). The Stryker QuickJS
|
||||
configuration passed all 90 test files (3,260 passed; one skipped). A narrow
|
||||
Stryker run on `get-connected-nodes.ts:37` killed three of three mutants.
|
||||
|
|
@ -23,8 +23,7 @@ export const workflow = new Workflow({
|
|||
});
|
||||
export const expression = workflow.expression;
|
||||
|
||||
// acquireIsolate/releaseIsolate are no-ops for the legacy engine, so these
|
||||
// hooks are safe to register unconditionally.
|
||||
// Acquire and release the isolate for both VM and QuickJS projects.
|
||||
beforeAll(async () => {
|
||||
await expression.acquireIsolate();
|
||||
});
|
||||
|
|
|
|||
|
|
@ -4,9 +4,7 @@ import * as Helpers from './helpers';
|
|||
import type { INodeExecutionData } from '../src/interfaces';
|
||||
import { Workflow } from '../src/workflow';
|
||||
|
||||
// Engine-parity tests for `$json` arrays beyond plain indexed access. Return
|
||||
// values must match on both engines; mutation persistence intentionally
|
||||
// diverges (see the isVm branches below).
|
||||
// `$json` array behavior beyond plain indexed access on VM and QuickJS.
|
||||
|
||||
describe('Expression — array proxy semantics (engine parity)', () => {
|
||||
const workflow = new Workflow({
|
||||
|
|
@ -39,7 +37,7 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
return expression.getParameterValue(value, null, 0, 0, 'node', data, 'manual', {});
|
||||
};
|
||||
|
||||
// Both engines reject property-descriptor access from inside an expression:
|
||||
// Both projects reject property-descriptor access from inside an expression:
|
||||
// `getOwnPropertyDescriptor` is on the sanitizer's blocklist, so the
|
||||
// expression is rejected before evaluation. Documented so a future
|
||||
// divergence is caught; neither engine intends to expose the data this way.
|
||||
|
|
@ -81,58 +79,51 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
]);
|
||||
});
|
||||
|
||||
// Mutating array methods run natively on both engines. Return values are
|
||||
// identical; persistence intentionally diverges: the vm engine mutates an
|
||||
// evaluation-scoped copy (fixture untouched), the legacy engine writes
|
||||
// through to the underlying workflow data.
|
||||
// Mutating array methods change only an evaluation-scoped copy.
|
||||
describe('mutating array methods follow native semantics', () => {
|
||||
const isVm = process.env.N8N_EXPRESSION_ENGINE !== 'legacy';
|
||||
|
||||
it('sort() returns the sorted array', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
|
||||
const sorted = ['Apple', 'Kiwi', 'Mango', 'Orange'];
|
||||
expect(evaluate('={{ $json.arr.sort() }}', json)).toEqual(sorted);
|
||||
expect(json.arr).toEqual(isVm ? ['Mango', 'Apple', 'Kiwi', 'Orange'] : sorted);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
|
||||
});
|
||||
|
||||
it('sort() forwards the comparator', () => {
|
||||
const json = { arr: [3, 1, 10, 2] };
|
||||
expect(evaluate('={{ $json.arr.sort((a, b) => b - a) }}', json)).toEqual([10, 3, 2, 1]);
|
||||
expect(json.arr).toEqual(isVm ? [3, 1, 10, 2] : [10, 3, 2, 1]);
|
||||
expect(json.arr).toEqual([3, 1, 10, 2]);
|
||||
});
|
||||
|
||||
it('splice() returns the removed elements', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
|
||||
expect(evaluate('={{ $json.arr.splice(0, 2) }}', json)).toEqual(['Mango', 'Apple']);
|
||||
expect(json.arr).toEqual(isVm ? ['Mango', 'Apple', 'Kiwi', 'Orange'] : ['Kiwi', 'Orange']);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
|
||||
});
|
||||
|
||||
it('fill() returns the filled array', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
|
||||
const filled = ['X', 'X', 'Kiwi', 'Orange'];
|
||||
expect(evaluate('={{ $json.arr.fill("X", 0, 2) }}', json)).toEqual(filled);
|
||||
expect(json.arr).toEqual(isVm ? ['Mango', 'Apple', 'Kiwi', 'Orange'] : filled);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
|
||||
});
|
||||
|
||||
it('shift() returns the removed first element', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi'] };
|
||||
expect(evaluate('={{ $json.arr.shift() }}', json)).toBe('Mango');
|
||||
expect(json.arr).toEqual(isVm ? ['Mango', 'Apple', 'Kiwi'] : ['Apple', 'Kiwi']);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi']);
|
||||
});
|
||||
|
||||
it('unshift() returns the new length', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi'] };
|
||||
expect(evaluate('={{ $json.arr.unshift("Peach", "Grape") }}', json)).toBe(5);
|
||||
expect(json.arr).toEqual(
|
||||
isVm ? ['Mango', 'Apple', 'Kiwi'] : ['Peach', 'Grape', 'Mango', 'Apple', 'Kiwi'],
|
||||
);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi']);
|
||||
});
|
||||
|
||||
it('copyWithin() returns the copied-within array', () => {
|
||||
const json = { arr: ['Mango', 'Apple', 'Kiwi', 'Orange'] };
|
||||
const copied = ['Kiwi', 'Orange', 'Kiwi', 'Orange'];
|
||||
expect(evaluate('={{ $json.arr.copyWithin(0, 2, 4) }}', json)).toEqual(copied);
|
||||
expect(json.arr).toEqual(isVm ? ['Mango', 'Apple', 'Kiwi', 'Orange'] : copied);
|
||||
expect(json.arr).toEqual(['Mango', 'Apple', 'Kiwi', 'Orange']);
|
||||
});
|
||||
|
||||
it('mutation is visible to a subsequent sibling read', () => {
|
||||
|
|
@ -186,20 +177,14 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
});
|
||||
});
|
||||
|
||||
// Direct writes on $json data. Both engines apply the write within the
|
||||
// evaluation; they intentionally diverge on persistence: the vm engine's
|
||||
// proxies are copy-on-write scoped to a single evaluation, while the legacy
|
||||
// engine writes through to the underlying workflow data (long-standing
|
||||
// behaviour for non-scripting nodes, where data is not augmented).
|
||||
// Direct writes on $json data stay within the current evaluation.
|
||||
describe('direct writes on $json data', () => {
|
||||
const isVm = process.env.N8N_EXPRESSION_ENGINE !== 'legacy';
|
||||
|
||||
it('index assignment is visible to a later read in the same evaluation', () => {
|
||||
const json = { arr: ['a', 'b', 'c'] };
|
||||
expect(evaluate('={{ (() => { $json.arr[0] = "X"; return $json.arr[0]; })() }}', json)).toBe(
|
||||
'X',
|
||||
);
|
||||
expect(json.arr).toEqual(isVm ? ['a', 'b', 'c'] : ['X', 'b', 'c']);
|
||||
expect(json.arr).toEqual(['a', 'b', 'c']);
|
||||
});
|
||||
|
||||
it('existing object-key assignment is visible to a later read', () => {
|
||||
|
|
@ -207,7 +192,7 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
expect(
|
||||
evaluate('={{ (() => { $json.user.name = "Zed"; return $json.user.name; })() }}', json),
|
||||
).toBe('Zed');
|
||||
expect(json.user.name).toBe(isVm ? 'Alice' : 'Zed');
|
||||
expect(json.user.name).toBe('Alice');
|
||||
});
|
||||
|
||||
it('delete removes the key for the rest of the evaluation', () => {
|
||||
|
|
@ -218,7 +203,7 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
json,
|
||||
),
|
||||
).toBe(true);
|
||||
expect('email' in json.user).toBe(isVm);
|
||||
expect('email' in json.user).toBe(true);
|
||||
});
|
||||
|
||||
it('push() updates length within the evaluation', () => {
|
||||
|
|
@ -226,13 +211,13 @@ describe('Expression — array proxy semantics (engine parity)', () => {
|
|||
expect(
|
||||
evaluate('={{ (() => { $json.arr.push("d"); return $json.arr.length; })() }}', json),
|
||||
).toBe(4);
|
||||
expect(json.arr.length).toBe(isVm ? 3 : 4);
|
||||
expect(json.arr.length).toBe(3);
|
||||
});
|
||||
|
||||
it('writes do not leak into a subsequent evaluation on the vm engine', () => {
|
||||
it('writes do not leak into a subsequent evaluation', () => {
|
||||
const json = { arr: ['a', 'b', 'c'] };
|
||||
evaluate('={{ (() => { $json.arr[0] = "X"; return $json.arr[0]; })() }}', json);
|
||||
expect(evaluate('={{ $json.arr[0] }}', json)).toBe(isVm ? 'a' : 'X');
|
||||
expect(evaluate('={{ $json.arr[0] }}', json)).toBe('a');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ import { Workflow } from '../src/workflow';
|
|||
|
||||
// Compatibility corpus for expression syntax.
|
||||
//
|
||||
// Both engines run every case (see `vitest.config.ts`).
|
||||
// The VM and QuickJS projects run every case (see `vitest.config.ts`).
|
||||
|
||||
describe('Expression — compatibility corpus', () => {
|
||||
const workflow = new Workflow({
|
||||
|
|
@ -251,8 +251,6 @@ describe('Expression — compatibility corpus', () => {
|
|||
$pageCount: 2,
|
||||
};
|
||||
|
||||
const ENGINE = process.env.N8N_EXPRESSION_ENGINE ?? 'vm';
|
||||
|
||||
const evaluate = (expr: string) =>
|
||||
expression.getParameterValue(
|
||||
expr,
|
||||
|
|
@ -882,12 +880,7 @@ describe('Expression — compatibility corpus', () => {
|
|||
expect(() => evaluate(expr)).toThrow();
|
||||
});
|
||||
|
||||
// ── known engine divergences ──────────────────────────────────────────
|
||||
const ENGINE_DIVERGENCES: Array<[string, string, unknown, unknown]> = [
|
||||
['Number.format()', "={{ (1234.5678).format('0,0.00') }}", undefined, '1234.5678'],
|
||||
];
|
||||
|
||||
it.each(ENGINE_DIVERGENCES)('divergent: %s', (_name, expr, legacyWant, vmWant) => {
|
||||
expect(evaluate(expr)).toStrictEqual(ENGINE === 'legacy' ? legacyWant : vmWant);
|
||||
it('returns the original number from Number.format()', () => {
|
||||
expect(evaluate("={{ (1234.5678).format('0,0.00') }}")).toBe('1234.5678');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -13,8 +13,7 @@ import { Workflow } from '../src/workflow';
|
|||
// `$item(0).$nodeId` resolved to undefined at runtime while the legacy engine
|
||||
// (and the editor preview) returned the correct value (CAT-3496).
|
||||
//
|
||||
// These cases run under both the legacy and VM engines (see `vitest.config.ts`)
|
||||
// so any divergence fails exactly one project.
|
||||
// VM and QuickJS run these contracts (see `vitest.config.ts`).
|
||||
|
||||
describe('Expression — legacy $item(index) accessor surface (engine parity)', () => {
|
||||
const workflow = new Workflow({
|
||||
|
|
|
|||
|
|
@ -8,10 +8,8 @@ import { Workflow } from '../src/workflow';
|
|||
// Engine-parity tests for `$items()` boundary behaviour.
|
||||
//
|
||||
// `$items()` was migrated to a typed-RPC under the VM engine in CAT-2982.
|
||||
// This file pins the behaviour of edge-case `runIndex` values across both
|
||||
// the legacy and VM engines so any divergence in how the runtime handles
|
||||
// host-side errors fails exactly one project. See `vitest.config.ts` for
|
||||
// the dual-engine setup.
|
||||
// This file pins edge-case `runIndex` values on VM and QuickJS.
|
||||
// See `vitest.config.ts` for the two projects.
|
||||
|
||||
describe('Expression — $items() boundary behaviour (engine parity)', () => {
|
||||
const workflow = new Workflow({
|
||||
|
|
@ -97,19 +95,17 @@ describe('Expression — $items() boundary behaviour (engine parity)', () => {
|
|||
|
||||
it('$items("Source", 0, 999) throws ExpressionError "Run X not found"', () => {
|
||||
// Host throws a structured ExpressionError when the runIndex is past
|
||||
// the known length. Both engines should surface this as-is.
|
||||
// the known length. Both projects should surface this as-is.
|
||||
expect(() => evaluate('={{ $items("Source", 0, 999) }}')).toThrowError(ExpressionError);
|
||||
});
|
||||
|
||||
it('$items("Source", 0, -2) returns undefined — host TypeError is swallowed by both engines', () => {
|
||||
it('$items("Source", 0, -2) returns undefined when the host TypeError is swallowed', () => {
|
||||
// Host's `getNodeExecutionData` only special-cases -1; any other
|
||||
// negative runIndex skips the `length <= runIndex` bounds check
|
||||
// (because length > negative) and dereferences `runData[name][-2].data`,
|
||||
// which throws a plain TypeError. Both engines' E() handler swallows
|
||||
// which throws a plain TypeError. Both projects' E() handler swallows
|
||||
// non-ExpressionError throws, so the observable result is undefined.
|
||||
// If a future change makes the VM engine surface the TypeError as an
|
||||
// error (instead of undefined), this test fails on the vm-engine
|
||||
// project and the divergence is caught.
|
||||
// If an engine starts surfacing the TypeError, this contract fails.
|
||||
expect(evaluate('={{ $items("Source", 0, -2) }}')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,9 +5,7 @@ import type { INodeExecutionData } from '../src/interfaces';
|
|||
import { Workflow } from '../src/workflow';
|
||||
|
||||
// Feature-parity regressions for `$json` access across nested data shapes.
|
||||
// This file lives in `packages/workflow/test/` so vitest runs it under both the
|
||||
// `legacy-engine` and `vm-engine` projects (see vitest.config.ts). Any divergence
|
||||
// between the two engines fails one project but not the other.
|
||||
// Vitest runs these contracts under VM and QuickJS (see vitest.config.ts).
|
||||
|
||||
describe('Expression — nested $json shapes (engine parity)', () => {
|
||||
const workflow = new Workflow({
|
||||
|
|
|
|||
|
|
@ -79,8 +79,7 @@ const nodeWithParameters = (parameters: INodeParameters) => {
|
|||
return { workflow, node: workflow.getNode('Webhook')! };
|
||||
};
|
||||
|
||||
// This file runs under both vitest projects (legacy and vm), so parity is
|
||||
// pinned against both engines; the vm engine needs an acquired isolate.
|
||||
// Both VM and QuickJS need an acquired isolate for this expression path.
|
||||
const viaEngine = async (parameters: INodeParameters, template: string) => {
|
||||
const { workflow, node } = nodeWithParameters(parameters);
|
||||
await workflow.expression.acquireIsolate();
|
||||
|
|
|
|||
|
|
@ -18,13 +18,6 @@ export default defineConfig({
|
|||
env: { N8N_EXPRESSION_ENGINE: 'vm' },
|
||||
},
|
||||
},
|
||||
{
|
||||
test: {
|
||||
...sharedTestConfig,
|
||||
name: 'legacy-engine',
|
||||
env: { N8N_EXPRESSION_ENGINE: 'legacy' },
|
||||
},
|
||||
},
|
||||
{
|
||||
test: {
|
||||
...sharedTestConfig,
|
||||
|
|
|
|||
|
|
@ -1,16 +1,12 @@
|
|||
// Vitest config used by Stryker only — NOT by `pnpm test`, NOT by CI unit runs.
|
||||
//
|
||||
// Runs the legacy expression engine, not vm/isolated-vm: the vm evaluator
|
||||
// SIGABRTs in Stryker's worker on teardown (upstream isolated-vm #464, repros on
|
||||
// Node 22 + 24). Trade-off: vm-only branches in expression.ts — the production
|
||||
// default engine — go unscored. Swap
|
||||
// back to N8N_EXPRESSION_ENGINE=vm once #464 is fixed or we pnpm-patch the guard.
|
||||
// Stryker uses QuickJS because VM teardown aborts in Stryker workers
|
||||
// (isolated-vm #464 on Node 22 and 24). The normal suite uses VM and QuickJS.
|
||||
|
||||
import { defineConfig } from 'vitest/config';
|
||||
import { createBaseInlineConfig } from '@n8n/vitest-config/node';
|
||||
|
||||
const { reporters, outputFile, ...sharedTestConfig } = createBaseInlineConfig({
|
||||
include: ['test/**/*.test.ts'],
|
||||
setupFiles: ['./test/setup-vm-evaluator.ts'],
|
||||
});
|
||||
|
||||
export default defineConfig({
|
||||
|
|
@ -21,7 +17,8 @@ export default defineConfig({
|
|||
{
|
||||
test: {
|
||||
...sharedTestConfig,
|
||||
name: 'legacy-engine',
|
||||
name: 'quickjs-engine',
|
||||
env: { N8N_EXPRESSION_ENGINE: 'quickjs' },
|
||||
},
|
||||
},
|
||||
],
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user