From bfc0bd0b40caf2264d29202a11ecf325f20f1f4d Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 1 Oct 2026 14:53:57 -0700 Subject: [PATCH 1/3] test(lint): lint the cycle-guard fixture without building a TS program The fixture lint ran the full config with projectService on, building a TypeScript program per run. Under a loaded full-suite run that pushed the test past the 20s testTimeout. import-x/no-cycle needs no type info, so the lint now disables projectService and filters to that one rule. --- packages/cli/test/import-cycle-lint.test.ts | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/packages/cli/test/import-cycle-lint.test.ts b/packages/cli/test/import-cycle-lint.test.ts index 03930128..5b07a0c9 100644 --- a/packages/cli/test/import-cycle-lint.test.ts +++ b/packages/cli/test/import-cycle-lint.test.ts @@ -72,7 +72,24 @@ describe("import-x/no-cycle", () => { 'import { a } from "./a";\n\nexport const b = (): string => a();\n' ); - const results = await createESLint().lintFiles([join(directory, "*.ts")]); + // Lint the fixture with type-aware parsing off and only this one rule. + // `projectService: true` makes the parser build a TypeScript program for + // the project, which is CPU-heavy. Under a loaded full-suite run, with + // every worker competing for CPU, it slowed this test 15-20x and pushed it + // past the 20s testTimeout (taskless/cli#420). The cycle rule needs none + // of it: edges come from the import-x resolver and `import-x/extensions`, + // both still read from the real config, so what this test exists to prove + // is unchanged. The type-checked rules have to be filtered out too, since + // they throw when the parser has no program to give them. + const eslint = new ESLint({ + cwd: REPO_ROOT, + overrideConfig: { + files: ["**/*.ts"], + languageOptions: { parserOptions: { projectService: false } }, + }, + ruleFilter: ({ ruleId }) => ruleId === "import-x/no-cycle", + }); + const results = await eslint.lintFiles([join(directory, "*.ts")]); const cycleMessages = results.flatMap((result) => result.messages.filter( (message) => message.ruleId === "import-x/no-cycle" From 5ae75e9366929728a0264ceff33d4a3c9af4c568 Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 1 Oct 2026 15:40:21 -0700 Subject: [PATCH 2/3] test(lint): name the cycle rule once and note the second test's narrowed config --- packages/cli/test/import-cycle-lint.test.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/packages/cli/test/import-cycle-lint.test.ts b/packages/cli/test/import-cycle-lint.test.ts index 5b07a0c9..cb17b673 100644 --- a/packages/cli/test/import-cycle-lint.test.ts +++ b/packages/cli/test/import-cycle-lint.test.ts @@ -11,7 +11,10 @@ import { afterAll, describe, expect, it } from "vitest"; * This does NOT re-implement cycle detection — that would be exactly the * "re-derive what the tool already knows" mistake the style guide forbids. * Every assertion below asks ESLint, running the repository's real - * `eslint.config.js`, and checks what it answers. + * `eslint.config.js`, and checks what it answers. The second test layers two + * narrowings on top of that config, type-aware parsing off and only this rule + * run, neither of which touches the settings the rule resolves imports with; + * the comment at that call explains why. * * It exists because the rule's failure mode is silence. While this rule was * being added, the config resolved correctly, matched the right files, and @@ -25,6 +28,7 @@ import { afterAll, describe, expect, it } from "vitest"; const REPO_ROOT = resolve(import.meta.dirname, "..", "..", ".."); const CLI_SOURCE = resolve(REPO_ROOT, "packages/cli/src"); +const RULE_ID = "import-x/no-cycle"; const temporaryDirectories: string[] = []; @@ -40,7 +44,7 @@ function createESLint(): ESLint { return new ESLint({ cwd: REPO_ROOT }); } -describe("import-x/no-cycle", () => { +describe(RULE_ID, () => { it("is enabled as an error for files in packages/cli/src", async () => { const config = (await createESLint().calculateConfigForFile( join(CLI_SOURCE, "index.ts") @@ -48,8 +52,8 @@ describe("import-x/no-cycle", () => { // "error" is 2 once ESLint normalizes it. A config block that stopped // matching `packages/cli/src` would leave this undefined. - expect(config.rules?.["import-x/no-cycle"]).toBeDefined(); - expect((config.rules?.["import-x/no-cycle"] as unknown[])[0]).toBe(2); + expect(config.rules?.[RULE_ID]).toBeDefined(); + expect((config.rules?.[RULE_ID] as unknown[])[0]).toBe(2); }); it("reports a value cycle written into packages/cli/src", async () => { @@ -87,13 +91,11 @@ describe("import-x/no-cycle", () => { files: ["**/*.ts"], languageOptions: { parserOptions: { projectService: false } }, }, - ruleFilter: ({ ruleId }) => ruleId === "import-x/no-cycle", + ruleFilter: ({ ruleId }) => ruleId === RULE_ID, }); const results = await eslint.lintFiles([join(directory, "*.ts")]); const cycleMessages = results.flatMap((result) => - result.messages.filter( - (message) => message.ruleId === "import-x/no-cycle" - ) + result.messages.filter((message) => message.ruleId === RULE_ID) ); expect(cycleMessages.length).toBeGreaterThan(0); From 7b59c0fd3a69853c560a30fd21467e4617fd55cc Mon Sep 17 00:00:00 2001 From: Jakob Heuser Date: Thu, 1 Oct 2026 17:01:13 -0700 Subject: [PATCH 3/3] test(lint): drop the cycle-guard test; a disabled rule is a review question The test linted a fixture to prove import-x/no-cycle was still effective. Whether the rule is on is visible in the eslint.config.js diff and belongs in code review, not in a test that builds a TS program on every run and writes into packages/cli/src while other tests read it. --- .gitignore | 13 --- eslint.config.js | 4 +- packages/cli/test/import-cycle-lint.test.ts | 103 -------------------- 3 files changed, 2 insertions(+), 118 deletions(-) delete mode 100644 packages/cli/test/import-cycle-lint.test.ts diff --git a/.gitignore b/.gitignore index 4cb66a50..32e445c2 100644 --- a/.gitignore +++ b/.gitignore @@ -46,16 +46,3 @@ __pycache__/ # Agent and manual git worktrees (full second checkouts; see worktrees-pnpm skill) worktrees/ - -# Scratch fixture written by `packages/cli/test/import-cycle-lint.test.ts`. That -# test writes a real a.ts <-> b.ts cycle INSIDE `packages/cli/src` and asserts -# `import-x/no-cycle` reports it. The location is forced, not a convenience: the -# rule only sees files the flat config matches, and the type-aware block needs -# the file inside a tsconfig (`packages/cli/tsconfig.json` includes `src`) — a -# fixture in the OS temp dir is refused as outside the config base path, and one -# elsewhere in the repo fails to parse and reports zero cycles, which is the -# vacuous green the test exists to rule out. Cleanup runs in `afterAll`, so a -# killed run (Ctrl+C, OOM, CI cancellation) can strand a live cycle in the -# source tree. This keeps that debris out of commits; delete the directory, not -# this line. -/packages/cli/src/__cycle-guard-*/ diff --git a/eslint.config.js b/eslint.config.js index dfc0c733..81374cb4 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -123,8 +123,8 @@ export default tseslint.config( // the rule resolves our files correctly, walks into them, finds nothing, // and reports no cycles — on a tree that provably contains one. A lint // run that is green because the rule is inert looks exactly like a lint - // run that is green because the code is clean, which is why the - // reintroduced-cycle check in this PR's description exists. + // run that is green because the code is clean, so treat a change to + // this list as a change to whether the rule runs at all. "import-x/extensions": [ ".ts", ".tsx", diff --git a/packages/cli/test/import-cycle-lint.test.ts b/packages/cli/test/import-cycle-lint.test.ts deleted file mode 100644 index cb17b673..00000000 --- a/packages/cli/test/import-cycle-lint.test.ts +++ /dev/null @@ -1,103 +0,0 @@ -import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { join, resolve } from "node:path"; - -import { ESLint } from "eslint"; -import { afterAll, describe, expect, it } from "vitest"; - -/** - * A guard that `import-x/no-cycle` is actually ON and actually reaching - * `packages/cli/src`. - * - * This does NOT re-implement cycle detection — that would be exactly the - * "re-derive what the tool already knows" mistake the style guide forbids. - * Every assertion below asks ESLint, running the repository's real - * `eslint.config.js`, and checks what it answers. The second test layers two - * narrowings on top of that config, type-aware parsing off and only this rule - * run, neither of which touches the settings the rule resolves imports with; - * the comment at that call explains why. - * - * It exists because the rule's failure mode is silence. While this rule was - * being added, the config resolved correctly, matched the right files, and - * reported `import-x/no-cycle` as an enabled error — and still found nothing on - * a tree that provably contained a cycle, because `import-x/extensions` - * defaults to `['.js', '.mjs', '.cjs']` and so every `.ts` file was dropped - * before its imports were read. A green `pnpm lint` looked identical whether - * the rule was working or inert. Nothing but an actual cycle distinguishes - * those two states, which is why the second test below writes one. - */ - -const REPO_ROOT = resolve(import.meta.dirname, "..", "..", ".."); -const CLI_SOURCE = resolve(REPO_ROOT, "packages/cli/src"); -const RULE_ID = "import-x/no-cycle"; - -const temporaryDirectories: string[] = []; - -afterAll(async () => { - await Promise.all( - temporaryDirectories.map(async (directory) => - rm(directory, { recursive: true, force: true }) - ) - ); -}); - -function createESLint(): ESLint { - return new ESLint({ cwd: REPO_ROOT }); -} - -describe(RULE_ID, () => { - it("is enabled as an error for files in packages/cli/src", async () => { - const config = (await createESLint().calculateConfigForFile( - join(CLI_SOURCE, "index.ts") - )) as { rules?: Record }; - - // "error" is 2 once ESLint normalizes it. A config block that stopped - // matching `packages/cli/src` would leave this undefined. - expect(config.rules?.[RULE_ID]).toBeDefined(); - expect((config.rules?.[RULE_ID] as unknown[])[0]).toBe(2); - }); - - it("reports a value cycle written into packages/cli/src", async () => { - // Written inside `packages/cli/src` on purpose: the point of the check is - // that the rule reaches THIS tree, so linting a fixture parked somewhere - // the config does not match would prove nothing. The directory name is - // prefixed so it is obviously not product code if cleanup is ever missed. - const directory = await mkdtemp(join(CLI_SOURCE, "__cycle-guard-")); - temporaryDirectories.push(directory); - - // A -> B -> A over VALUE imports. Kept to real value edges because - // type-only edges are erased before the module runs and the rule ignores - // them by design; see the note in eslint.config.js. - await writeFile( - join(directory, "a.ts"), - 'import { b } from "./b";\n\nexport const a = (): string => b();\n' - ); - await writeFile( - join(directory, "b.ts"), - 'import { a } from "./a";\n\nexport const b = (): string => a();\n' - ); - - // Lint the fixture with type-aware parsing off and only this one rule. - // `projectService: true` makes the parser build a TypeScript program for - // the project, which is CPU-heavy. Under a loaded full-suite run, with - // every worker competing for CPU, it slowed this test 15-20x and pushed it - // past the 20s testTimeout (taskless/cli#420). The cycle rule needs none - // of it: edges come from the import-x resolver and `import-x/extensions`, - // both still read from the real config, so what this test exists to prove - // is unchanged. The type-checked rules have to be filtered out too, since - // they throw when the parser has no program to give them. - const eslint = new ESLint({ - cwd: REPO_ROOT, - overrideConfig: { - files: ["**/*.ts"], - languageOptions: { parserOptions: { projectService: false } }, - }, - ruleFilter: ({ ruleId }) => ruleId === RULE_ID, - }); - const results = await eslint.lintFiles([join(directory, "*.ts")]); - const cycleMessages = results.flatMap((result) => - result.messages.filter((message) => message.ruleId === RULE_ID) - ); - - expect(cycleMessages.length).toBeGreaterThan(0); - }); -});