From c55b93d2e6867f0801a47d8890a1cfd18ca02ccc Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sun, 9 Aug 2026 13:35:20 +0200 Subject: [PATCH 1/2] fix(test): exclude the cwd-exclusion pair from the Node and Bun runners Both runners decide whether a test is Deno-only by reading that file's own source for `Deno.`. The cwd-exclusion pair keeps its Deno usage in the helper it imports, so the heuristic missed it and `deno task test:node` began failing on `Deno.chdir` being undefined. Named explicitly rather than by making the files mention `Deno.` to satisfy the heuristic: the pair asserts a property of `deno test --parallel` itself -- that test files sharing one process do not share a working directory -- so it is Deno-only by subject and not merely by which API it happens to call. CI runs neither task, which is why this survived review. --- tests/bun/run-tests.mjs | 5 +++++ tests/node/run-tests.mjs | 11 ++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/bun/run-tests.mjs b/tests/bun/run-tests.mjs index 84c447a568..f1c0f58f6d 100644 --- a/tests/bun/run-tests.mjs +++ b/tests/bun/run-tests.mjs @@ -45,6 +45,11 @@ const includePatterns = (process.env.BUN_TEST_INCLUDE || process.env.VF_TEST_INC .map((value) => value.trim()) .filter(Boolean); const runtimeIncompatibleTests = [ + // Asserts a property of `deno test --parallel` itself -- that test files + // sharing one process do not share a working directory. The heuristic below + // misses it because its Deno usage lives in the helper it imports rather + // than in the test file's own source. + "src/testing/cwd-exclusion-*.test.ts", "src/config/env.test.ts", "src/proxy/handler.test.ts", "src/proxy/oauth-client.test.ts", diff --git a/tests/node/run-tests.mjs b/tests/node/run-tests.mjs index 89f0d06a2e..2474cfa685 100644 --- a/tests/node/run-tests.mjs +++ b/tests/node/run-tests.mjs @@ -44,7 +44,16 @@ const includePatterns = (process.env.NODE_TEST_INCLUDE || process.env.VF_TEST_IN .map((value) => value.trim()) .filter(Boolean); // Exclude Deno-specific test files that use Deno.test directly -const denoOnlyTests = ["src/issues/**", "src/cache/backend.test.ts"]; +// The exclusion heuristic below reads each test file's own source for `Deno.`, +// which misses a file whose Deno usage lives in the helper it imports. The +// cwd-exclusion pair asserts a property of `deno test --parallel` itself -- +// that test files sharing one process do not share a working directory -- so it +// is Deno-only by subject, not merely by API. +const denoOnlyTests = [ + "src/issues/**", + "src/cache/backend.test.ts", + "src/testing/cwd-exclusion-*.test.ts", +]; const runtimeIncompatibleTests = [ "src/proxy/handler.test.ts", "src/proxy/oauth-client.test.ts", From 62efdc295edcd5fee3fae3bb45d7d4a3b47f551f Mon Sep 17 00:00:00 2001 From: Koji Wakayama Date: Sun, 9 Aug 2026 13:49:18 +0200 Subject: [PATCH 2/2] test: give the runtime exclusion list one home and a test The reviewer asked for coverage on both runners' filters, which the exclusion had none of: it was a literal in each runner, duplicated, and nothing noticed if it stopped matching. Renaming the pair or moving it out of `src/testing/` would leave a pattern matching nothing and `deno task test:node` would fail again, which is the regression this list was added for. The patterns move to `tests/deno-only-tests.mjs`, imported by both runners so they cannot drift, and `tests/runtime-test-filters.test.ts` asserts both directions: the pair is excluded, and its neighbours -- including `cwd.test.ts`, which shares a prefix -- stay eligible. Both halves matter; a filter that excludes too much shrinks the suite silently. Checked against a list emptied and a pattern widened, and it fails on each. Also fixes an unrelated Deno-only import that made `deno task test:bun` fail before it reached any of this: `extensions/ext-bundler-esbuild` wrote `npm:es-module-lexer@2.3.1` inline while its own deno.json already maps the bare specifier, as it does for esbuild. Only Deno resolves the inline form. --- .../src/es-module-lexer.ts | 5 +- tests/bun/run-tests.mjs | 8 +-- tests/deno-only-tests.mjs | 25 ++++++++ tests/node/run-tests.mjs | 12 ++-- tests/runtime-test-filters.test.ts | 61 +++++++++++++++++++ 5 files changed, 98 insertions(+), 13 deletions(-) create mode 100644 tests/deno-only-tests.mjs create mode 100644 tests/runtime-test-filters.test.ts diff --git a/extensions/ext-bundler-esbuild/src/es-module-lexer.ts b/extensions/ext-bundler-esbuild/src/es-module-lexer.ts index b937826fcc..29cbe14db1 100644 --- a/extensions/ext-bundler-esbuild/src/es-module-lexer.ts +++ b/extensions/ext-bundler-esbuild/src/es-module-lexer.ts @@ -9,7 +9,10 @@ */ import type { ImportSpecifier, ModuleLexer } from "veryfront/extensions/bundler"; -import { init, parse } from "npm:es-module-lexer@2.3.1"; +// Bare specifier, mapped to the pinned npm package by this extension's +// deno.json -- as `esbuild` is. Writing the `npm:` specifier inline instead +// resolves only under Deno, which is what broke `deno task test:bun`. +import { init, parse } from "es-module-lexer"; /** es-module-lexer-backed {@link ModuleLexer} implementation. */ export class EsModuleLexer implements ModuleLexer { diff --git a/tests/bun/run-tests.mjs b/tests/bun/run-tests.mjs index f1c0f58f6d..f20b688e39 100644 --- a/tests/bun/run-tests.mjs +++ b/tests/bun/run-tests.mjs @@ -5,6 +5,7 @@ import os from "node:os"; import { readFileSync } from "node:fs"; import { filterTestFiles, listTestFiles, splitIntoShards } from "../test-file-utils.mjs"; import { ensureNpmNodeModulesLinks } from "../ensure-npm-links.mjs"; +import { DENO_ONLY_TESTS } from "../deno-only-tests.mjs"; function resolveConcurrency(envKeys) { for (const key of envKeys) { @@ -45,11 +46,8 @@ const includePatterns = (process.env.BUN_TEST_INCLUDE || process.env.VF_TEST_INC .map((value) => value.trim()) .filter(Boolean); const runtimeIncompatibleTests = [ - // Asserts a property of `deno test --parallel` itself -- that test files - // sharing one process do not share a working directory. The heuristic below - // misses it because its Deno usage lives in the helper it imports rather - // than in the test file's own source. - "src/testing/cwd-exclusion-*.test.ts", + // Files the `Deno.`-in-source heuristic below cannot see; see the shared list. + ...DENO_ONLY_TESTS, "src/config/env.test.ts", "src/proxy/handler.test.ts", "src/proxy/oauth-client.test.ts", diff --git a/tests/deno-only-tests.mjs b/tests/deno-only-tests.mjs new file mode 100644 index 0000000000..c3659238e4 --- /dev/null +++ b/tests/deno-only-tests.mjs @@ -0,0 +1,25 @@ +/** + * Test files that only make sense under `deno test`, shared by the Node and Bun + * runners. + * + * Both runners already drop a file whose own source mentions `Deno.`, which + * covers almost everything. It does not cover a file whose Deno usage lives in + * a helper it imports -- the heuristic reads one file, not the module graph -- + * and it says nothing about files that are Deno-only by *subject* rather than + * by which API they happen to call. + * + * The pair below is both. It asserts a property of `deno test --parallel` + * itself: that test files sharing one process do not share a working directory. + * Node and Bun give each file its own process, so there is no property there to + * assert even if the APIs existed. + * + * Kept here rather than duplicated in each runner so the two cannot drift, and + * so it can be tested -- see ./runtime-test-filters.test.ts. + * + * @module tests/deno-only-tests + */ + +/** Glob patterns for tests that must not run outside Deno. */ +export const DENO_ONLY_TESTS = [ + "src/testing/cwd-exclusion-*.test.ts", +]; diff --git a/tests/node/run-tests.mjs b/tests/node/run-tests.mjs index 2474cfa685..c98c6ad66d 100644 --- a/tests/node/run-tests.mjs +++ b/tests/node/run-tests.mjs @@ -5,6 +5,7 @@ import os from "node:os"; import { readFileSync } from "node:fs"; import { filterTestFiles, listTestFiles, splitIntoShards } from "../test-file-utils.mjs"; import { ensureNpmNodeModulesLinks } from "../ensure-npm-links.mjs"; +import { DENO_ONLY_TESTS } from "../deno-only-tests.mjs"; function resolveConcurrency(envKeys) { for (const key of envKeys) { @@ -43,16 +44,13 @@ const includePatterns = (process.env.NODE_TEST_INCLUDE || process.env.VF_TEST_IN .split(",") .map((value) => value.trim()) .filter(Boolean); -// Exclude Deno-specific test files that use Deno.test directly -// The exclusion heuristic below reads each test file's own source for `Deno.`, -// which misses a file whose Deno usage lives in the helper it imports. The -// cwd-exclusion pair asserts a property of `deno test --parallel` itself -- -// that test files sharing one process do not share a working directory -- so it -// is Deno-only by subject, not merely by API. +// Exclude Deno-specific test files that use Deno.test directly. Files the +// `Deno.`-in-source heuristic below cannot see live in ./deno-only-tests.mjs, +// shared with the Bun runner. const denoOnlyTests = [ "src/issues/**", "src/cache/backend.test.ts", - "src/testing/cwd-exclusion-*.test.ts", + ...DENO_ONLY_TESTS, ]; const runtimeIncompatibleTests = [ "src/proxy/handler.test.ts", diff --git a/tests/runtime-test-filters.test.ts b/tests/runtime-test-filters.test.ts new file mode 100644 index 0000000000..f559354173 --- /dev/null +++ b/tests/runtime-test-filters.test.ts @@ -0,0 +1,61 @@ +/** + * The Node and Bun runners must keep the Deno-only tests out, and everything + * else in. + * + * Both halves matter. A filter that excludes too little lets + * `src/testing/cwd-exclusion-*.test.ts` run on a runtime without `Deno.chdir`, + * which is the regression this list was added for. A filter that excludes too + * much silently shrinks the suite, which nothing else would notice. + * + * The list is easy to break by accident: renaming those files, or moving them + * out of `src/testing/`, leaves a pattern matching nothing and the runner fails + * again the next time someone runs `deno task test:node`. Neither task runs in + * CI, so this is the only thing standing between that and a surprised human. + * + * @module tests/runtime-test-filters + */ + +import { describe, it } from "#veryfront/testing/bdd.ts"; +import { assert, assertEquals } from "#veryfront/testing/assert.ts"; +import { DENO_ONLY_TESTS } from "./deno-only-tests.mjs"; +import { filterTestFiles } from "./test-file-utils.mjs"; + +/** The files the shared list exists to exclude. */ +const DENO_ONLY_FILES = [ + "src/testing/cwd-exclusion-a.test.ts", + "src/testing/cwd-exclusion-b.test.ts", +]; + +/** Ordinary tests, including neighbours of the excluded pair. */ +const ELIGIBLE_FILES = [ + "src/testing/cwd.test.ts", + "src/testing/isolation.test.ts", + "src/errors/error-registry.test.ts", + "cli/router.test.ts", +]; + +describe("runtime test filters", () => { + it("excludes the Deno-only tests from non-Deno runners", () => { + const kept = filterTestFiles(DENO_ONLY_FILES, { exclude: DENO_ONLY_TESTS }); + + assertEquals(kept, [], "these cannot run without Deno.chdir"); + }); + + it("keeps every other test eligible", () => { + const kept = filterTestFiles(ELIGIBLE_FILES, { exclude: DENO_ONLY_TESTS }); + + // `cwd.test.ts` sits beside the excluded pair and starts with the same + // three letters, so an over-broad pattern would take it too. + assertEquals(kept, ELIGIBLE_FILES, "the filter must not shrink the suite"); + }); + + it("matches files that actually exist", async () => { + // A renamed or moved file leaves a pattern matching nothing, and the + // exclusion silently stops working. Cheaper to catch here than in a failing + // `test:node` run. + for (const path of DENO_ONLY_FILES) { + const stat = await Deno.stat(new URL(`../${path}`, import.meta.url)); + assert(stat.isFile, `${path} is named in the exclusion list but is missing`); + } + }); +});