diff --git a/packages/testing/janitor/src/cli.ts b/packages/testing/janitor/src/cli.ts index e63d6bf2eb9..cfc3fa439f2 100644 --- a/packages/testing/janitor/src/cli.ts +++ b/packages/testing/janitor/src/cli.ts @@ -79,7 +79,7 @@ import { } from './core/method-usage-analyzer.js'; import { createProject } from './core/project-loader.js'; import { readLockfileImporters } from './core/read-lockfile-importers.js'; -import { readManifestDiffs } from './core/read-manifest-diffs.js'; +import { readManifestDiffs, readTsconfigDiffs } from './core/read-manifest-diffs.js'; import { toJSON, toConsole } from './core/reporter.js'; import { filterToFailedSpecs } from './core/retry-filter.js'; import { computeScope, formatScope } from './core/scope-analyzer.js'; @@ -686,6 +686,8 @@ function runSelect(options: CliOptions): void { // devDependency-only classifier can drop a devDep-only lockfile change. // No base (local dev) → omit manifests → conservative (keep lockfile broad). const manifests = options.baseRef ? readManifestDiffs(changedFiles, options.baseRef) : undefined; + // Same for tsconfig diffs, feeding the resolution-key classifier. + const tsconfigs = options.baseRef ? readTsconfigDiffs(changedFiles, options.baseRef) : undefined; // Only parse the (large) lockfile when a RUNTIME dependency actually changed — // the only case the dep-graph selector (389) acts on. A devDep-only manifest // change would parse it for nothing. @@ -698,6 +700,7 @@ function runSelect(options: CliOptions): void { mapFile: options.mapFile, allSpecsFile: options.allSpecsFile, manifests, + tsconfigs, lockfileImporters, }); console.log(JSON.stringify(result)); diff --git a/packages/testing/janitor/src/core/read-manifest-diffs.ts b/packages/testing/janitor/src/core/read-manifest-diffs.ts index dd3c62317f5..2494b99396a 100644 --- a/packages/testing/janitor/src/core/read-manifest-diffs.ts +++ b/packages/testing/janitor/src/core/read-manifest-diffs.ts @@ -1,7 +1,7 @@ /** - * before/after content of each changed package.json, for the `@n8n/test-impact` - * devDependency classifier. Any read failure → '' so the classifier stays - * conservative (an unreadable manifest is treated as a non-devDep change, kept). + * before/after content of changed package.json / tsconfig files, for the + * `@n8n/test-impact` classifiers. Any read failure → '' so a classifier stays + * conservative (an unreadable file is treated as impactful). */ import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -9,6 +9,7 @@ import { join } from 'node:path'; import { getFileAtRef, getGitRoot } from '../utils/git-operations.js'; const isManifest = (file: string): boolean => /(^|\/)package\.json$/.test(file); +const isTsconfig = (file: string): boolean => /(^|\/)tsconfig([.\w-]*)\.json$/.test(file); /** Read the working-tree file, or '' on any failure (missing, unreadable, or a * TOCTOU delete between the existsSync check and the read). */ @@ -20,20 +21,35 @@ function readWorkingTree(abs: string): string { } } -export function readManifestDiffs( +/** before/after content of each changed file matching `predicate`. */ +function readFileDiffs( changedFiles: string[], baseRef: string, + predicate: (file: string) => boolean, ): Record { - const manifests = changedFiles.filter(isManifest); - if (manifests.length === 0) return {}; + const matched = changedFiles.filter(predicate); + if (matched.length === 0) return {}; const root = getGitRoot(process.cwd()); const out: Record = {}; - for (const file of manifests) { - const abs = join(root, file); + for (const file of matched) { out[file] = { before: getFileAtRef(file, baseRef) ?? '', - after: readWorkingTree(abs), + after: readWorkingTree(join(root, file)), }; } return out; } + +export function readManifestDiffs( + changedFiles: string[], + baseRef: string, +): Record { + return readFileDiffs(changedFiles, baseRef, isManifest); +} + +export function readTsconfigDiffs( + changedFiles: string[], + baseRef: string, +): Record { + return readFileDiffs(changedFiles, baseRef, isTsconfig); +} diff --git a/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts b/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts index a9c19092a52..c135dd2238e 100644 --- a/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts +++ b/packages/testing/playwright/composables/journeys/admin-views-executions-list.ts @@ -1,8 +1,8 @@ +import { workflow, trigger, node } from '@n8n/workflow-sdk'; import { expect } from '@playwright/test'; import type { IWorkflowBase } from 'n8n-workflow'; import { nanoid } from 'nanoid'; -import { workflow, trigger, node } from '../../../../@n8n/workflow-sdk/src'; import type { n8nPage } from '../../pages/n8nPage'; import type { ApiHelpers } from '../../services/api-helper'; import type { TestUser } from '../../services/user-api-helper'; diff --git a/packages/testing/playwright/tests/e2e/api/binary-data-xss.spec.ts b/packages/testing/playwright/tests/e2e/api/binary-data-xss.spec.ts index 4ae8a710822..d5f2241d1f7 100644 --- a/packages/testing/playwright/tests/e2e/api/binary-data-xss.spec.ts +++ b/packages/testing/playwright/tests/e2e/api/binary-data-xss.spec.ts @@ -1,8 +1,8 @@ +import { workflow, trigger, node } from '@n8n/workflow-sdk'; import flatted from 'flatted'; import type { IWorkflowBase } from 'n8n-workflow'; import { nanoid } from 'nanoid'; -import { workflow, trigger, node } from '../../../../../@n8n/workflow-sdk/src'; import { N8N_AUTH_COOKIE } from '../../../config/constants'; import { test, expect } from '../../../fixtures/base'; diff --git a/packages/testing/playwright/tests/e2e/nodes/code-node-api.spec.ts b/packages/testing/playwright/tests/e2e/nodes/code-node-api.spec.ts index eb6abd7f9e1..9e92a1d70cb 100644 --- a/packages/testing/playwright/tests/e2e/nodes/code-node-api.spec.ts +++ b/packages/testing/playwright/tests/e2e/nodes/code-node-api.spec.ts @@ -1,8 +1,8 @@ +import { workflow, trigger, node } from '@n8n/workflow-sdk'; import flatted from 'flatted'; import type { IWorkflowBase } from 'n8n-workflow'; import { nanoid } from 'nanoid'; -import { workflow, trigger, node } from '../../../../../@n8n/workflow-sdk/src'; import { test, expect } from '../../../fixtures/base'; const TRIGGER_NAME = 'Manual Trigger'; diff --git a/packages/testing/test-impact/src/changes.test.ts b/packages/testing/test-impact/src/changes.test.ts index 4c9c152658c..5f2ad2195dd 100644 --- a/packages/testing/test-impact/src/changes.test.ts +++ b/packages/testing/test-impact/src/changes.test.ts @@ -4,6 +4,8 @@ import { isNonImpactful, filterImpactfulChanges, forcesBroad, + isTsconfig, + tsconfigForcesBroad, classifyManifestChange, dropDevDepOnlyDeps, } from './changes.js'; @@ -25,8 +27,6 @@ describe('isNonImpactful', () => { 'assets/logo.svg', 'scripts/release/build.mjs', 'turbo.json', - 'tsconfig.json', - 'packages/cli/tsconfig.build.json', 'biome.jsonc', 'packages/testing/test-impact/vitest.config.ts', '.eslintrc.js', @@ -47,6 +47,9 @@ describe('isNonImpactful', () => { // Container + patches affect runtime / a dependency — never ignored 'docker/images/n8n/Dockerfile', 'patches/some-dep.patch', + // tsconfig can carry module-resolution keys → routed via tsconfigForcesBroad + 'tsconfig.json', + 'packages/cli/tsconfig.build.json', ])('treats %s as impactful', (file) => { expect(isNonImpactful(file)).toBe(false); }); @@ -96,6 +99,70 @@ describe('forcesBroad', () => { }); }); +describe('isTsconfig', () => { + it.each(['tsconfig.json', 'packages/cli/tsconfig.build.json', 'a/tsconfig.go.json'])( + 'recognises %s', + (file) => { + expect(isTsconfig(file)).toBe(true); + }, + ); + + it.each(['tsconfig.ts', 'packages/cli/package.json', 'config.json'])( + 'does not match %s', + (file) => { + expect(isTsconfig(file)).toBe(false); + }, + ); +}); + +describe('tsconfigForcesBroad', () => { + const ts = (compilerOptions = {}, extra = {}) => JSON.stringify({ compilerOptions, ...extra }); + + it('forces broad when compilerOptions.paths changes', () => { + const before = ts({ paths: { 'esprima-next': ['./x'] } }); + const after = ts({ paths: { 'n8n-workflow': ['./src/index.ts'], 'esprima-next': ['./x'] } }); + expect(tsconfigForcesBroad(before, after)).toBe(true); + }); + + it('forces broad when baseUrl changes', () => { + expect(tsconfigForcesBroad(ts({ baseUrl: '.' }), ts({ baseUrl: './src' }))).toBe(true); + }); + + it('forces broad when moduleResolution changes', () => { + expect( + tsconfigForcesBroad(ts({ moduleResolution: 'node' }), ts({ moduleResolution: 'bundler' })), + ).toBe(true); + }); + + it('forces broad when customConditions changes', () => { + const before = ts({ customConditions: ['development'] }); + const after = ts({ customConditions: ['production'] }); + expect(tsconfigForcesBroad(before, after)).toBe(true); + }); + + it('forces broad when extends changes', () => { + const before = ts({}, { extends: '@n8n/typescript-config/modern/tsconfig.json' }); + const after = ts({}, { extends: '@n8n/typescript-config/modern/tsconfig.go.json' }); + expect(tsconfigForcesBroad(before, after)).toBe(true); + }); + + it('does NOT force broad for a resolution-neutral flag change', () => { + const before = ts({ strict: true, paths: { a: ['./a'] } }); + const after = ts({ strict: false, target: 'ES2022', paths: { a: ['./a'] } }); + expect(tsconfigForcesBroad(before, after)).toBe(false); + }); + + it('tolerates comments and trailing commas', () => { + const before = '{\n // base\n "compilerOptions": { "paths": { "a": ["./a"] } },\n}'; + const after = '{\n // base\n "compilerOptions": { "paths": { "a": ["./b"] } },\n}'; + expect(tsconfigForcesBroad(before, after)).toBe(true); + }); + + it('forces broad when content is unparseable (conservative)', () => { + expect(tsconfigForcesBroad('{ not json', '{ still not')).toBe(true); + }); +}); + const pkg = (deps = {}, devDeps = {}, extra = {}) => JSON.stringify({ name: 'x', dependencies: deps, devDependencies: devDeps, ...extra }); diff --git a/packages/testing/test-impact/src/changes.ts b/packages/testing/test-impact/src/changes.ts index 3b0a376d9c0..290275a6ef6 100644 --- a/packages/testing/test-impact/src/changes.ts +++ b/packages/testing/test-impact/src/changes.ts @@ -13,8 +13,9 @@ * * NOT ignored (deliberately): `docker`/Dockerfiles (define the container the * E2E suite runs in), `pnpm-lock.yaml`/`package.json` (dependency changes — - * handled by the devDep classifier / dep-graph selector), and `patches/**` - * (edits a dependency's code = a dependency change). + * handled by the devDep classifier / dep-graph selector), `patches/**` + * (edits a dependency's code = a dependency change), and `tsconfig*.json` + * (can carry module-resolution keys — see {@link tsconfigForcesBroad}). */ const NON_IMPACTFUL: Array<(f: string) => boolean> = [ @@ -31,9 +32,10 @@ const NON_IMPACTFUL: Array<(f: string) => boolean> = [ (f) => /^(docs|assets)\/.*\.(png|jpe?g|gif|svg|webp)$/.test(f), // Repo automation scripts (not shipped, not exercised by E2E) (f) => f.startsWith('scripts/'), - // Named build / lint / test-runner config (NOT all json/yaml) + // Named build / lint / test-runner config (NOT all json/yaml). `tsconfig*` is + // deliberately absent — it's routed through tsconfigForcesBroad instead. (f) => - /(^|\/)(turbo\.json|tsconfig([.\w-]*)\.json|biome\.jsonc?|vitest\.config\.[cm]?[jt]s|\.eslintrc[\w.]*|\.prettierrc[\w.]*)$/.test( + /(^|\/)(turbo\.json|biome\.jsonc?|vitest\.config\.[cm]?[jt]s|\.eslintrc[\w.]*|\.prettierrc[\w.]*)$/.test( f, ), ]; @@ -158,6 +160,52 @@ export function changedRuntimeDepsFromManifests( const isManifest = (f: string): boolean => /(^|\/)package\.json$/.test(f); const isLockfile = (f: string): boolean => f === 'pnpm-lock.yaml'; +export const isTsconfig = (f: string): boolean => /(^|\/)tsconfig([.\w-]*)\.json$/.test(f); + +/** compilerOptions keys that change which module a bare import resolves to. */ +const TSCONFIG_RESOLUTION_KEYS = [ + 'paths', + 'baseUrl', + 'moduleResolution', + 'customConditions', +] as const; + +/** Tolerant parse for tsconfig (allows comments + trailing commas). Null when + * unparseable, which the caller treats as "force broad". */ +function parseTsconfig(raw: string): Record | null { + if (!raw.trim()) return null; + try { + return JSON.parse(raw) as Record; + } catch { + try { + const stripped = raw + .replace(/\/\*[\s\S]*?\*\//g, '') + .replace(/(^|[^:])\/\/.*$/gm, '$1') + .replace(/,(\s*[}\]])/g, '$1'); + return JSON.parse(stripped) as Record; + } catch { + return null; + } + } +} + +/** + * True when a tsconfig change touches a resolution key (`paths`/`baseUrl`/ + * `moduleResolution`/`customConditions`/`extends`), which re-points imports for + * every spec and can't be attributed in the coverage map → force the full + * suite. A type-check-only edit (`strict`, `target`, …) resolves to the same + * modules and is non-impactful. + */ +export function tsconfigForcesBroad(before: string, after: string): boolean { + const b = parseTsconfig(before); + const a = parseTsconfig(after); + if (!b || !a) return true; + if (JSON.stringify(b.extends) !== JSON.stringify(a.extends)) return true; + const bco = (b.compilerOptions ?? {}) as Record; + const aco = (a.compilerOptions ?? {}) as Record; + return TSCONFIG_RESOLUTION_KEYS.some((k) => JSON.stringify(bco[k]) !== JSON.stringify(aco[k])); +} + /** Remove the lockfile + every package.json from a changed-file set. */ export function stripDependencyFiles(files: string[]): string[] { return files.filter((f) => !isLockfile(f) && !isManifest(f)); diff --git a/packages/testing/test-impact/src/select.test.ts b/packages/testing/test-impact/src/select.test.ts index 97296b93ff9..70e40c6a859 100644 --- a/packages/testing/test-impact/src/select.test.ts +++ b/packages/testing/test-impact/src/select.test.ts @@ -216,6 +216,64 @@ describe('selectTests — fail-open contract', () => { expect(result.mode).toBe('broad'); }); + // A stray `paths` entry re-points every spec's imports — the regression this guards. + it('tsconfig resolution-key change (paths) → broad, never declared uncovered', () => { + const map: ImpactMap = { 'packages/cli/src/x.ts': { '10': ['tests/e2e/a.spec.ts'] } }; + const mapPath = path.join(tempDir, 'map-tsconfig-paths.json'); + fs.writeFileSync(mapPath, JSON.stringify(map)); + const result = selectTests({ + changedFiles: ['packages/workflow/tsconfig.json'], + mapFile: mapPath, + allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')), + tsconfigs: { + 'packages/workflow/tsconfig.json': { + before: JSON.stringify({ compilerOptions: { paths: { 'esprima-next': ['./x'] } } }), + after: JSON.stringify({ + compilerOptions: { + paths: { 'n8n-workflow': ['./src/index.ts'], 'esprima-next': ['./x'] }, + }, + }), + }, + }, + }); + expect(result.mode).toBe('broad'); + expect(result.specs).toEqual([...ALL_SPECS].sort()); + expect(result.uncovered).toBeUndefined(); + }); + + it('tsconfig resolution-neutral change (strict) → dropped, source still scopes', () => { + const map: ImpactMap = { 'packages/cli/src/x.ts': { '10': ['tests/e2e/a.spec.ts'] } }; + const mapPath = path.join(tempDir, 'map-tsconfig-neutral.json'); + fs.writeFileSync(mapPath, JSON.stringify(map)); + const result = selectTests({ + changedFiles: ['packages/cli/tsconfig.json', 'packages/cli/src/x.ts'], + mapFile: mapPath, + allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')), + tsconfigs: { + 'packages/cli/tsconfig.json': { + before: JSON.stringify({ compilerOptions: { strict: true } }), + after: JSON.stringify({ compilerOptions: { strict: false } }), + }, + }, + }); + expect(result.mode).toBe('scoped'); + expect(result.specs).toEqual(['tests/e2e/a.spec.ts']); + expect(result.uncovered).toBeUndefined(); + }); + + it('tsconfig change with no diff metadata → broad (cannot classify)', () => { + const map: ImpactMap = { 'packages/cli/src/x.ts': { '10': ['tests/e2e/a.spec.ts'] } }; + const mapPath = path.join(tempDir, 'map-tsconfig-nometa.json'); + fs.writeFileSync(mapPath, JSON.stringify(map)); + const result = selectTests({ + changedFiles: ['packages/workflow/tsconfig.json'], + mapFile: mapPath, + allSpecsFile: writeAllSpecs(ALL_SPECS.join('\n')), + }); + expect(result.mode).toBe('broad'); + expect(result.specs).toEqual([...ALL_SPECS].sort()); + }); + describe('--all-specs parsing', () => { const triggerBroad = (allSpecsFile: string) => selectTests({ diff --git a/packages/testing/test-impact/src/select.ts b/packages/testing/test-impact/src/select.ts index 14fad89b15a..6de4797b72b 100644 --- a/packages/testing/test-impact/src/select.ts +++ b/packages/testing/test-impact/src/select.ts @@ -18,7 +18,9 @@ import { dropDevDepOnlyDeps, filterImpactfulChanges, forcesBroad, + isTsconfig, stripDependencyFiles, + tsconfigForcesBroad, } from './changes.js'; import type { WorkspaceImporters } from './dep-graph.js'; import { @@ -44,6 +46,9 @@ export interface SelectTestsInput { * the lockfile + manifests from selection — a devDep can't reach the runtime * bundle the E2E suite exercises. */ manifests?: Record; + /** before/after content of each changed tsconfig (caller reads from git). + * Fed to {@link tsconfigForcesBroad}; omitted (local dev) → conservative broad. */ + tsconfigs?: Record; /** Workspace package dir → runtime dependency names it declares (parsed from * pnpm-lock.yaml's `importers`). With `manifests`, a changed runtime dep is * walked to its declaring packages and scoped via the map instead @@ -110,6 +115,18 @@ export function selectTests(input: SelectTestsInput): SelectTestsResult { const forcing = impactful.filter(forcesBroad); if (forcing.length > 0) return broad(forcing); + // A resolution-changing tsconfig edit forces broad; a resolution-neutral one + // is dropped (see tsconfigForcesBroad). No diff metadata → conservative broad. + const tsconfigChanges = impactful.filter(isTsconfig); + if (tsconfigChanges.length > 0) { + const forcingTsconfig = tsconfigChanges.filter((f) => { + const diff = input.tsconfigs?.[f]; + return !diff || tsconfigForcesBroad(diff.before, diff.after); + }); + if (forcingTsconfig.length > 0) return broad(forcingTsconfig); + impactful = impactful.filter((f) => !isTsconfig(f)); + } + // devDependency-only change can't reach the runtime bundle → dropped. if (input.manifests) impactful = dropDevDepOnlyDeps(impactful, input.manifests);