diff --git a/.github/workflows/test-unit-reusable.yml b/.github/workflows/test-unit-reusable.yml index 1d71ec8547e..4b7a2521856 100644 --- a/.github/workflows/test-unit-reusable.yml +++ b/.github/workflows/test-unit-reusable.yml @@ -108,8 +108,23 @@ jobs: with: node-version: ${{ inputs.nodeVersion }} - - name: Test Integration - run: pnpm test:ci:backend:integration --summarize + # Only cli is wired into janitor integration scoping today, so the + # turbo run filter excludes it from the bulk integration run and a + # dedicated step invokes the scoped variant — same shape as the unit + # job above. This avoids turbo silently no-op'ing + # `test:integration:changed` for backend packages that don't define it. + # + # --concurrency=1 is preserved from the pre-existing + # `test:ci:backend:integration` script: backend integration suites + # share state (DB sockets, ports, fixtures) and have historically been + # run serially to avoid cross-package interference. Unit jobs above + # don't need this because they isolate per-process. + - name: Test Integration (backend, except cli) + run: pnpm turbo test:integration --continue --concurrency=1 --filter='!./packages/frontend/**' --filter='!n8n' --summarize + + - name: Test Integration (cli, scoped) + if: ${{ !cancelled() }} + run: pnpm turbo test:integration:changed --filter=n8n --summarize - name: Send Test Stats if: ${{ !cancelled() }} diff --git a/package.json b/package.json index d8cb3062a62..539dfc7d3aa 100644 --- a/package.json +++ b/package.json @@ -47,6 +47,7 @@ "test:ci:backend": "turbo run test --continue --concurrency=1 --filter='!./packages/frontend/**'", "test:ci:backend:unit": "turbo run test:unit --continue --filter='!./packages/frontend/**'", "test:ci:backend:integration": "turbo run test:integration --continue --concurrency=1 --filter='!./packages/frontend/**'", + "test:ci:backend:integration:changed": "turbo run test:integration:changed --continue --concurrency=1 --filter='!./packages/frontend/**'", "test:affected": "turbo run test --affected --concurrency=1", "test:with:docker": "pnpm --filter=n8n-playwright test:container:standard", "test:show:report": "pnpm --filter=n8n-playwright exec playwright show-report", diff --git a/packages/cli/package.json b/packages/cli/package.json index a90adf17ae9..af3c46b6d0f 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -23,6 +23,7 @@ "test:unit": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite jest --config=jest.config.unit.js", "test:unit:changed": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite janitor test-scoped --runner=jest -- --config=jest.config.unit.js", "test:integration": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite jest --config=jest.config.integration.js", + "test:integration:changed": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite janitor test-scoped --runner=jest --jest-variant=integration -- --config=jest.config.integration.js", "test:dev": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite jest --watch", "test:sqlite": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite jest --config=jest.config.integration.js --no-coverage", "test:sqlite:migrations": "N8N_LOG_LEVEL=silent DB_SQLITE_POOL_SIZE=4 DB_TYPE=sqlite jest --config=jest.config.migration.js --no-coverage", diff --git a/packages/testing/janitor/src/cli.ts b/packages/testing/janitor/src/cli.ts index ba1acbb5689..c6e67c1b748 100644 --- a/packages/testing/janitor/src/cli.ts +++ b/packages/testing/janitor/src/cli.ts @@ -625,6 +625,7 @@ function runTestScopedCmd(options: CliOptions): void { rootDir: findWorkspaceRoot(process.cwd()), changedFiles, passthroughArgs: options.passthroughArgs, + jestVariant: options.jestVariant, }); process.exit(exitCode); } @@ -640,6 +641,7 @@ function runScope(options: CliOptions): void { packageDir: options.packageDir ?? process.cwd(), changedFiles: readChangedFiles(options), rootDir: findWorkspaceRoot(process.cwd()), + jestVariant: options.jestVariant, }); console.log(formatScope(result)); } diff --git a/packages/testing/janitor/src/cli/arg-parser.test.ts b/packages/testing/janitor/src/cli/arg-parser.test.ts index 5fd4d612614..f81cd88edde 100644 --- a/packages/testing/janitor/src/cli/arg-parser.test.ts +++ b/packages/testing/janitor/src/cli/arg-parser.test.ts @@ -209,6 +209,23 @@ describe('arg-parser', () => { const result = parseArgs(); expect(result.testCommand).toBe('pnpm test'); }); + + it('parses --jest-variant=unit', () => { + setArgs(['--jest-variant=unit']); + const result = parseArgs(); + expect(result.jestVariant).toBe('unit'); + }); + + it('parses --jest-variant=integration', () => { + setArgs(['--jest-variant=integration']); + const result = parseArgs(); + expect(result.jestVariant).toBe('integration'); + }); + + it('throws on unknown --jest-variant value', () => { + setArgs(['--jest-variant=e2e']); + expect(() => parseArgs()).toThrow(/Unknown --jest-variant=e2e/); + }); }); describe('combined arguments', () => { diff --git a/packages/testing/janitor/src/cli/arg-parser.ts b/packages/testing/janitor/src/cli/arg-parser.ts index 5d4f1754bd2..e7d471accf1 100644 --- a/packages/testing/janitor/src/cli/arg-parser.ts +++ b/packages/testing/janitor/src/cli/arg-parser.ts @@ -57,6 +57,7 @@ export interface CliOptions { // Affected-packages / scope options changedFiles?: string; runner?: 'jest' | 'vitest'; + jestVariant?: 'unit' | 'integration'; packageDir?: string; /** Anything after `--` — forwarded to the test runner by `test-scoped`. */ passthroughArgs: string[]; @@ -193,6 +194,13 @@ const VALUE_FLAG_HANDLERS: Record [--package-dir=] [--changed-files=] + janitor scope --runner= [--jest-variant=] [--package-dir=] [--changed-files=] --package-dir: defaults to cwd (matches how pnpm/turbo invoke test scripts). --changed-files: newline- OR comma-separated repo-root-relative paths. Defaults to $CHANGED_FILES env var. + --jest-variant: 'integration' widens the bailout set to catch runtime- + coupled changes invisible to jest --findRelatedTests + (entities, repositories, migrations, shared fixtures). + Defaults to 'unit'. Output (single line on stdout): SKIP No in-package files changed @@ -258,11 +264,15 @@ export function showTestScopedHelp(): void { Test-Scoped - Compute scope and spawn jest/vitest with the right flags Usage: - janitor test-scoped --runner= [--package-dir=] [--changed-files=] [extra runner args] + janitor test-scoped --runner= [--jest-variant=] [--package-dir=] [--changed-files=] [extra runner args] --package-dir: defaults to cwd (matches how pnpm/turbo invoke test scripts). --changed-files: newline- OR comma-separated repo-root-relative paths. Defaults to $CHANGED_FILES env var. + --jest-variant: 'integration' widens the bailout set to catch runtime- + coupled changes invisible to jest --findRelatedTests + (entities, repositories, migrations, shared fixtures). + Defaults to 'unit'. Local dev (no $CHANGED_FILES set): runs the full suite. CI: scopes via jest --findRelatedTests / vitest related --run, or skips diff --git a/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts b/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts index d6c91b9ceb1..4bde96648cb 100644 --- a/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts +++ b/packages/testing/janitor/src/core/affected-packages-analyzer.test.ts @@ -106,6 +106,23 @@ describe('affectedPackages', () => { expect(affectedPackages({ rootDir, changedFiles: ['package.json'] })).toEqual(['a', 'b']); }); + it('expands all packages when packages/@n8n/db/** changes (runtime-coupled schema)', () => { + const rootDir = makeFixture({ + patterns: ['packages/*', 'packages/@n8n/*'], + packages: { + 'packages/@n8n/db': { name: '@n8n/db' }, + 'packages/cli': { name: 'n8n' }, + 'packages/unrelated': { name: 'unrelated' }, + }, + }); + expect( + affectedPackages({ + rootDir, + changedFiles: ['packages/@n8n/db/src/entities/user.entity.ts'], + }), + ).toEqual(['@n8n/db', 'n8n', 'unrelated']); + }); + it('handles turbo extra-inputs pointing at another package', () => { const rootDir = makeFixture({ patterns: ['packages/*'], diff --git a/packages/testing/janitor/src/core/affected-packages-analyzer.ts b/packages/testing/janitor/src/core/affected-packages-analyzer.ts index 54ed3063c9d..dd17974ef91 100644 --- a/packages/testing/janitor/src/core/affected-packages-analyzer.ts +++ b/packages/testing/janitor/src/core/affected-packages-analyzer.ts @@ -35,6 +35,16 @@ export interface AnalyzeOptions { } const GLOBAL_TRIGGER_FILES = new Set(['pnpm-lock.yaml', 'package.json']); +// Directory prefixes whose contents force RUN_FULL across the workspace. Used +// when a package's contract is consumed at runtime by every other package and +// the workspace symlink/dep-graph alone can't catch the coupling: +// - packages/@n8n/db — schema + entities resolved by cli integration tests +// through the DI container at runtime; jest --findRelatedTests on the +// test file alone wouldn't see the migration/entity file in the import +// graph and would silently SKIP. Bailing the workspace is over-broad on +// rare db PRs but keeps the failure mode "ran too much" rather than +// "ran nothing". +const GLOBAL_TRIGGER_PREFIXES = ['packages/@n8n/db/']; function loadWorkspacePackages(rootDir: string): WorkspacePackage[] { const wsFile = join(rootDir, 'pnpm-workspace.yaml'); @@ -121,7 +131,14 @@ export function affectedPackages(options: AnalyzeOptions): string[] { // No signal (local dev, missing env) → safest default: everything. if (options.changedFiles === null) return allNames; - if (options.changedFiles.some((f) => GLOBAL_TRIGGER_FILES.has(f))) return allNames; + if ( + options.changedFiles.some( + (f) => + GLOBAL_TRIGGER_FILES.has(f) || + GLOBAL_TRIGGER_PREFIXES.some((prefix) => f.startsWith(prefix)), + ) + ) + return allNames; const direct = new Set(); for (const file of options.changedFiles) { diff --git a/packages/testing/janitor/src/core/scope-analyzer.test.ts b/packages/testing/janitor/src/core/scope-analyzer.test.ts index 329af6ca819..b8dd709a2a9 100644 --- a/packages/testing/janitor/src/core/scope-analyzer.test.ts +++ b/packages/testing/janitor/src/core/scope-analyzer.test.ts @@ -143,4 +143,113 @@ describe('computeScope', () => { }); expect(result.kind).toBe('full'); }); + + describe('jest integration variant', () => { + it('does NOT bail on entity changes for the unit variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/modules/foo/foo.entity.ts'], + }); + expect(result.kind).toBe('scoped'); + }); + + it('bails to full on entity changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/modules/foo/foo.entity.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('bails to full on repository changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/databases/repositories/user.repository.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('bails to full on migration changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/modules/foo/database/AddFoo.migration.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('bails to full on shared integration test fixture changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/test/integration/shared/workflow.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('bails to full on test/migration changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/test/migration/some-helper.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('bails to full on src/modules//database changes for the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/modules/insights/database/repositories/insights.ts'], + }); + expect(result.kind).toBe('full'); + }); + + it('still scopes when only an ordinary src file changes under the integration variant', () => { + const rootDir = makePackageDir('packages/cli'); + const result = computeScope({ + runner: 'jest', + jestVariant: 'integration', + packageDir: 'packages/cli', + rootDir, + changedFiles: ['packages/cli/src/controllers/auth.controller.ts'], + }); + expect(result.kind).toBe('scoped'); + }); + + it('jest-variant=integration has no effect on the vitest runner', () => { + const rootDir = makePackageDir('packages/frontend/editor-ui'); + const result = computeScope({ + runner: 'vitest', + jestVariant: 'integration', + packageDir: 'packages/frontend/editor-ui', + rootDir, + changedFiles: ['packages/frontend/editor-ui/src/foo.entity.ts'], + }); + expect(result.kind).toBe('scoped'); + }); + }); }); diff --git a/packages/testing/janitor/src/core/scope-analyzer.ts b/packages/testing/janitor/src/core/scope-analyzer.ts index f9a51dad92d..3085dc24ece 100644 --- a/packages/testing/janitor/src/core/scope-analyzer.ts +++ b/packages/testing/janitor/src/core/scope-analyzer.ts @@ -11,6 +11,15 @@ import { isAbsolute, relative, resolve } from 'node:path'; import { toPosix } from './path-utils.js'; export type Runner = 'jest' | 'vitest'; +/** + * Jest has two test surfaces in this workspace: + * - unit — pure modules + mocked deps; only the unit bailouts apply + * - integration — HTTP / DI container; transitive deps that don't appear in + * the import graph (db schema, shared fixtures, migrations) + * force RUN_FULL. See JEST_INTEGRATION_BAILOUT for the list. + * `undefined` collapses to the unit variant for callers that don't specify. + */ +export type JestVariant = 'unit' | 'integration'; // Bailout patterns are centralised here (vs the original DEVP-194 spec's // per-package `n8nTestChanged.inPackageBailouts` field) because the n8n @@ -30,6 +39,27 @@ const JEST_BAILOUT = [ /(?:^|\/)(?:jest|test)\.setup\.[cm]?[jt]s$/, /(?:^|\/)__tests__\/setup\.[cm]?[jt]s$/, ]; +// Integration tests hit HTTP + the DI container, so changes to types that +// aren't import-graph-visible to the test file still flow through at runtime: +// - entity / repository changes — touched at runtime via `Container.get`, +// not imported by the test +// - migrations — never imported but every integration test depends on the +// resulting schema +// - `src/databases/**` / `src/modules/*/database/**` — schema scaffolding +// - `test/integration/shared/**` — shared fixtures coupled at runtime +// - `test/migration/**` — migration test infrastructure +// Without these, jest --findRelatedTests would return zero tests and CI +// would falsely report green on changes that genuinely break integration. +const JEST_INTEGRATION_BAILOUT = [ + ...JEST_BAILOUT, + /\.entity\.[cm]?ts$/, + /\.repository\.[cm]?ts$/, + /\.migration\.[cm]?ts$/, + /^src\/databases\//, + /^src\/modules\/[^/]+\/database\//, + /^test\/integration\/shared\//, + /^test\/migration\//, +]; // Frontend packages use vite.config.* for the vitest config too (vitest reads // vite.config). Setup files live at src/__tests__/setup.ts per the shared // @n8n/vitest-config convention. @@ -47,6 +77,12 @@ export interface ComputeScopeOptions { rootDir: string; /** `null` = no signal → RUN_FULL (local dev with unset env). */ changedFiles: string[] | null; + /** + * Only relevant when `runner === 'jest'`. Selects the bailout set: + * `'integration'` widens it to catch runtime-coupled changes (entities, + * repositories, migrations, shared fixtures). Defaults to `'unit'`. + */ + jestVariant?: JestVariant; } export type ScopeResult = @@ -71,7 +107,12 @@ export function computeScope(options: ComputeScopeOptions): ScopeResult { ); if (inPackage.length === 0) return { kind: 'skip', reason: 'No changed files in package' }; - const bailout = options.runner === 'jest' ? JEST_BAILOUT : VITEST_BAILOUT; + const bailout = + options.runner === 'jest' + ? options.jestVariant === 'integration' + ? JEST_INTEGRATION_BAILOUT + : JEST_BAILOUT + : VITEST_BAILOUT; for (const file of inPackage) { const relInPkg = file.slice(pkgPrefixSlash.length); if (bailout.some((p) => p.test(relInPkg))) { diff --git a/packages/testing/janitor/src/core/test-scoped-runner.ts b/packages/testing/janitor/src/core/test-scoped-runner.ts index 2cd17d3c9f9..c834674bf89 100644 --- a/packages/testing/janitor/src/core/test-scoped-runner.ts +++ b/packages/testing/janitor/src/core/test-scoped-runner.ts @@ -3,7 +3,7 @@ import { spawnSync } from 'node:child_process'; import { isAbsolute, resolve } from 'node:path'; -import { computeScope, type Runner, type ScopeResult } from './scope-analyzer.js'; +import { computeScope, type JestVariant, type Runner, type ScopeResult } from './scope-analyzer.js'; export interface TestScopedOptions { runner: Runner; @@ -11,6 +11,7 @@ export interface TestScopedOptions { rootDir: string; changedFiles: string[] | null; passthroughArgs: string[]; + jestVariant?: JestVariant; } /** @@ -43,6 +44,7 @@ export function runTestScoped(options: TestScopedOptions): number { packageDir: options.packageDir, rootDir: options.rootDir, changedFiles: options.changedFiles, + jestVariant: options.jestVariant, }); if (scope.kind === 'skip') { diff --git a/turbo.json b/turbo.json index 724c4c033f5..36a3d088bbc 100644 --- a/turbo.json +++ b/turbo.json @@ -64,6 +64,12 @@ "outputs": ["coverage/**", "*.xml"], "env": ["COVERAGE_ENABLED"] }, + "test:integration:changed": { + "dependsOn": ["^build", "build"], + "outputs": ["coverage/**", "*.xml"], + "env": ["COVERAGE_ENABLED", "CHANGED_FILES"], + "cache": false + }, "watch": { "cache": false, "persistent": true }, "dev": { "cache": false, "persistent": true }, "install-browsers": {