diff --git a/docs/api-reference/veryfront/fs.md b/docs/api-reference/veryfront/fs.md index 65f4c6698f..675e36f4b5 100644 --- a/docs/api-reference/veryfront/fs.md +++ b/docs/api-reference/veryfront/fs.md @@ -46,22 +46,22 @@ const configPath = resolve(cwd(), "veryfront.config.ts"); | Name | Description | Source | | ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------- | | `basename` | Return the last path segment. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/path/basic-operations.ts#L43) | -| `createFileSystem` | Create the runtime-native filesystem implementation. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L438) | +| `createFileSystem` | Create the runtime-native filesystem implementation. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L449) | | `cwd` | Return the current working directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/lifecycle.ts#L26) | | `dirname` | Return the parent directory path. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/path/basic-operations.ts#L36) | -| `exists` | Return false for a missing path and propagate every other filesystem error. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L511) | +| `exists` | Return false for a missing path and propagate every other filesystem error. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L522) | | `extname` | Return the file extension for a path. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/path/basic-operations.ts#L51) | | `isFileSnapshotChangedError` | Return whether a value is a framework-created file snapshot change error. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/adapters/file-snapshot-error.ts#L32) | | `isNotFoundError` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/not-found-error.ts#L210) | | `join` | Join and normalize path segments using their detected path flavor. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/path/basic-operations.ts#L23) | -| `lstat` | Read file metadata without following a terminal symbolic link. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L521) | -| `mkdir` | Create a directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L561) | -| `readDir` | Read directory entries. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L571) | -| `readTextFile` | Read a file as text. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L491) | -| `realPath` | Resolve a path to its canonical absolute form, following symlinks. Throws if the path does not exist. Useful for containment checks where a symlink could otherwise escape an intended directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L607) | -| `remove` | Remove a file or directory, rejecting when the path does not exist. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L566) | +| `lstat` | Read file metadata without following a terminal symbolic link. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L532) | +| `mkdir` | Create a directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L572) | +| `readDir` | Read directory entries. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L582) | +| `readTextFile` | Read a file as text. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L502) | +| `realPath` | Resolve a path to its canonical absolute form, following symlinks. Throws if the path does not exist. Useful for containment checks where a symlink could otherwise escape an intended directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L618) | +| `remove` | Remove a file or directory, rejecting when the path does not exist. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L577) | | `resolve` | Resolve path segments to an absolute, normalized path. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/path/resolution.ts#L24) | -| `writeTextFile` | Write text to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L501) | +| `writeTextFile` | Write text to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L512) | ### Classes diff --git a/docs/api-reference/veryfront/testing.md b/docs/api-reference/veryfront/testing.md index 278260df9b..936d6dcc6e 100644 --- a/docs/api-reference/veryfront/testing.md +++ b/docs/api-reference/veryfront/testing.md @@ -55,44 +55,44 @@ describe("math", () => { | `assertThrows` | Assert that a synchronous function throws. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/assert.ts#L296) | | `beforeAll` | Register a hook before all BDD tests in a group. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/bdd.ts#L744) | | `beforeEach` | Register a hook before each BDD test. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/bdd.ts#L725) | -| `chmod` | Change file permissions, rejecting operational failures. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L588) | -| `createFileSystem` | Create the runtime-native filesystem implementation. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L438) | +| `chmod` | Change file permissions, rejecting operational failures. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L599) | +| `createFileSystem` | Create the runtime-native filesystem implementation. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L449) | | `cwd` | Return the current working directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/lifecycle.ts#L26) | | `deepEquals` | ********************* Shared utility functions for cross-runtime testing. ********************* | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/utils.ts#L5) | | `delay` | Wait for a duration in milliseconds. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L123) | | `deleteEnv` | Delete a process environment variable. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/env.ts#L205) | | `describe` | Group related BDD tests. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/bdd.ts#L611) | | `env` | Read and write process environment variables. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/env.ts#L33) | -| `exists` | Return false for a missing path and propagate every other filesystem error. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L511) | +| `exists` | Return false for a missing path and propagate every other filesystem error. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L522) | | `exit` | Exit the current process. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L128) | | `fail` | Fail the current assertion immediately. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/assert.ts#L336) | | `getArgs` | Get command-line arguments (cross-runtime: Deno.args or process.argv). | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/lifecycle.ts#L10) | | `getEnv` | Read an environment variable from the active project scope. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/env.ts#L106) | | `getTestTimeScale` | Return test time scale. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/timing.ts#L4) | -| `isAlreadyExistsError` | Error shape for is already exists. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L617) | +| `isAlreadyExistsError` | Error shape for is already exists. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L628) | | `isNotFoundError` | | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/not-found-error.ts#L210) | | `it` | Define a BDD test case. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/bdd.ts#L667) | -| `makeTempDir` | Atomically create a unique directory beneath the operating-system temp root. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L583) | +| `makeTempDir` | Atomically create a unique directory beneath the operating-system temp root. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L594) | | `makeTempDirWithOptions` | Options accepted by make temp dir with. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L73) | | `makeTempFile` | Create temp file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L48) | -| `mkdir` | Create a directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L561) | -| `readDir` | Read directory entries. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L571) | -| `readFile` | Read a file as bytes. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L496) | -| `readTextFile` | Read a file as text. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L491) | +| `mkdir` | Create a directory. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L572) | +| `readDir` | Read directory entries. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L582) | +| `readFile` | Read a file as bytes. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L507) | +| `readTextFile` | Read a file as text. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L502) | | `registerTestCleanup` | Registers test cleanup. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/isolation.ts#L22) | -| `remove` | Remove a file or directory, rejecting when the path does not exist. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L566) | +| `remove` | Remove a file or directory, rejecting when the path does not exist. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L577) | | `resetAllTestState` | Comprehensive reset of ALL test state across the application. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/isolation.ts#L64) | | `safeStringify` | Serialize unknown values safely for test output. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/utils.ts#L34) | | `scaleMs` | Scale a duration for the current test runtime. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/timing.ts#L9) | | `setEnv` | Sets env. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/process/env.ts#L185) | -| `stat` | Read file metadata. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L516) | +| `stat` | Read file metadata. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L527) | | `testDelay` | Wait for a test-scaled duration. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/timing.ts#L15) | | `waitFor` | Wait until a condition succeeds. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L99) | | `withEnv` | Applies env. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L186) | | `withTempDir` | Applies temp dir. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L138) | | `withTempFile` | Applies temp file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/testing/deno-compat.ts#L162) | -| `writeFile` | Write bytes to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L506) | -| `writeTextFile` | Write text to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L501) | +| `writeFile` | Write bytes to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L517) | +| `writeTextFile` | Write text to a file. | [source](https://github.com/veryfront/veryfront-code/blob/main/src/platform/compat/fs.ts#L512) | ### Types diff --git a/src/platform/adapters/runtime/shared/node-filesystem-adapter.ts b/src/platform/adapters/runtime/shared/node-filesystem-adapter.ts index f03681905e..0379fe55b8 100644 --- a/src/platform/adapters/runtime/shared/node-filesystem-adapter.ts +++ b/src/platform/adapters/runtime/shared/node-filesystem-adapter.ts @@ -562,7 +562,18 @@ export class NodeCompatibleFileSystemAdapter implements FileSystemAdapter { async remove(path: string, options?: { recursive?: boolean }): Promise { const fs = await import("node:fs/promises"); - await fs.rm(path, { recursive: options?.recursive, force: true }); + const recursive = options?.recursive ?? false; + try { + await fs.rm(path, { recursive, force: true }); + } catch (error) { + // Same divergence as `platform/compat/fs.ts`: Deno removes an empty + // directory without `recursive`, `node:fs` `rm` refuses one, and `force` + // does not cover it -- it suppresses a missing path, not a directory. + if (recursive) throw error; + const info = await fs.lstat(path).catch(() => undefined); + if (!info?.isDirectory()) throw error; + await fs.rmdir(path); + } } async makeTempDir(prefix: string): Promise { diff --git a/src/platform/compat/fs-remove-portable.test.ts b/src/platform/compat/fs-remove-portable.test.ts new file mode 100644 index 0000000000..b9a3968e2b --- /dev/null +++ b/src/platform/compat/fs-remove-portable.test.ts @@ -0,0 +1,54 @@ +/** + * Removal behaviour that must agree on every runtime. + * + * Deliberately free of any reference to the Deno namespace, and that is + * load-bearing rather than stylistic: both `tests/node/run-tests.mjs` and + * `tests/bun/run-tests.mjs` drop a file whose own source names it -- including, + * on a first draft of this comment, a file that only named it to explain the + * rule. So the sibling suite in + * `fs.test.ts` runs on Deno alone. Deno selects `DenoFileSystem`, which already + * behaved; `NodeFileSystem` is the implementation that did not, and it refuses + * to initialise under Deno at all. Coverage of the fix therefore has to come + * from a file the other runners will actually pick up. + * + * Everything below goes through the module's own exports, so each runtime + * exercises whichever implementation it selects. + * + * @module platform/compat/fs-remove-portable + */ + +import { assertEquals, assertRejects } from "#veryfront/testing/assert.ts"; +import { afterAll, beforeAll, describe, it } from "#veryfront/testing/bdd.ts"; +import { exists, makeTempDir, mkdir, remove, writeTextFile } from "./fs.ts"; +import { join } from "./path/index.ts"; + +let testDir: string; + +beforeAll(async () => { + testDir = await makeTempDir({ prefix: "vf-fs-remove-portable-" }); +}); + +afterAll(async () => { + await remove(testDir, { recursive: true }); +}); + +describe("remove, across runtimes", () => { + it("removes an empty directory without the recursive option", async () => { + const dirPath = join(testDir, "empty-dir"); + await mkdir(dirPath); + + await remove(dirPath); + + assertEquals(await exists(dirPath), false); + }); + + it("refuses a non-empty directory without the recursive option", async () => { + const dirPath = join(testDir, "populated-dir"); + await mkdir(dirPath); + await writeTextFile(join(dirPath, "file.txt"), "test"); + + await assertRejects(() => remove(dirPath), Error); + + assertEquals(await exists(dirPath), true, "the directory must survive the refusal"); + }); +}); diff --git a/src/platform/compat/fs.test.ts b/src/platform/compat/fs.test.ts index 5ded319ae4..35d03fcbdb 100644 --- a/src/platform/compat/fs.test.ts +++ b/src/platform/compat/fs.test.ts @@ -248,6 +248,23 @@ describe("Filesystem Compat", () => { assertEquals(await exists(dirPath), false); }); + it("removes an empty directory without the recursive option", async () => { + const dirPath = join(testDir, "to-remove-empty-dir"); + await mkdir(dirPath); + + await remove(dirPath); + assertEquals(await exists(dirPath), false); + }); + + it("refuses a non-empty directory without the recursive option", async () => { + const dirPath = join(testDir, "to-remove-populated-dir"); + await mkdir(dirPath); + await writeTextFile(join(dirPath, "file.txt"), "test"); + + await assertRejects(() => remove(dirPath), Error); + assertEquals(await exists(dirPath), true); + }); + it("surfaces a missing path even when recursive", async () => { await assertRejects( () => remove(join(testDir, "missing-recursive-remove"), { recursive: true }), diff --git a/src/platform/compat/fs.ts b/src/platform/compat/fs.ts index 82b51600b1..7f236cd92d 100644 --- a/src/platform/compat/fs.ts +++ b/src/platform/compat/fs.ts @@ -150,6 +150,7 @@ interface NodeFsPromises { }> >; rm(path: string, options?: { recursive?: boolean; force?: boolean }): Promise; + rmdir(path: string): Promise; mkdtemp(prefix: string): Promise; chmod(path: string, mode: number): Promise; } @@ -299,7 +300,17 @@ class NodeFileSystem implements FileSystem { async remove(path: string, options?: { recursive?: boolean }): Promise { await this.ensureInitialized(); const recursive = options?.recursive ?? false; - await this.getFs().rm(path, { recursive, force: false }); + try { + await this.getFs().rm(path, { recursive, force: false }); + } catch (error) { + // Deno removes an empty directory without `recursive`; `node:fs` `rm` + // refuses one, and refuses it with a different code on Node (ERR_FS_EISDIR) + // than on Bun (EFAULT). Ask the filesystem instead of reading the code. + if (recursive) throw error; + const info = await this.getFs().lstat(path).catch(() => undefined); + if (!info?.isDirectory()) throw error; + await this.getFs().rmdir(path); + } } async makeTempDir(options?: { prefix?: string }): Promise { diff --git a/tests/runtime-test-filters.test.ts b/tests/runtime-test-filters.test.ts index f559354173..7c152b02fa 100644 --- a/tests/runtime-test-filters.test.ts +++ b/tests/runtime-test-filters.test.ts @@ -49,6 +49,23 @@ describe("runtime test filters", () => { assertEquals(kept, ELIGIBLE_FILES, "the filter must not shrink the suite"); }); + it("keeps the portable removal suite scannable by the other runners", async () => { + // Both runners drop a file whose own source names the Deno namespace with a + // trailing dot. `fs-remove-portable.test.ts` exists precisely to run on Node + // and Bun, so that spelling must never appear in it -- not in code, and not + // in a comment explaining the rule, which is how a first draft excluded + // itself. Prose may say "Deno"; only "Deno." is disqualifying. + const source = await Deno.readTextFile( + new URL("../src/platform/compat/fs-remove-portable.test.ts", import.meta.url), + ); + + assertEquals( + /\bDeno\./.test(source), + false, + "naming the Deno namespace here silently removes this suite from Node and Bun", + ); + }); + 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