Repository navigation
feat(file-safety): land safeWriteText as its own unit (u1 of the #2357 split plan, content unchanged) #2385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
easonLiangWorldedtech
wants to merge
7
commits into
Zoo-Code-Org:main
Choose a base branch
from
easonLiangWorldedtech:vps2f-2357-u1-writer
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+3,916
−4
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
fc2dc75
u1-writer content applied alone onto main tip e487cd7b2a
easonLiangWorldedtech b6a8e82
file-safety: name the commit rather than a rename, and pin backup cle…
easonLiangWorldedtech 5ac73bf
pin the lock artefact spelling for the duration of a write
easonLiangWorldedtech 970b3a4
propagate ENOENT raised after the target access check
easonLiangWorldedtech 0b40d66
file-safety: close the four pre-commit and post-commit gaps CodeRabbi…
easonLiangWorldedtech 20182be
fix(file-safety,utils): close the post-commit and staging-open gaps r…
easonLiangWorldedtech c4fc421
test(utils): canonicalize the expectation with the API the lock uses
easonLiangWorldedtech File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
70 changes: 70 additions & 0 deletions
70
src/services/file-safety/__tests__/safeWriteText.integration.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| import * as fs from "fs/promises" | ||
| import * as os from "os" | ||
| import * as path from "path" | ||
|
|
||
| import { safeWriteText } from "../safeWriteText" | ||
|
|
||
| // No fs mocks in this file: the point is to assert what a real filesystem ends up | ||
| // holding after a publish attempt, which the mocked spec cannot show. The failure is | ||
| // provoked with real filesystem semantics rather than with a stubbed call. | ||
| describe("safeWriteText against a real filesystem", () => { | ||
| let dir: string | ||
|
|
||
| beforeEach(async () => { | ||
| dir = await fs.mkdtemp(path.join(os.tmpdir(), "safe-write-text-int-")) | ||
| }) | ||
|
|
||
| afterEach(async () => { | ||
| await fs.rm(dir, { recursive: true, force: true }) | ||
| }) | ||
|
|
||
| it("publishes the new bytes and leaves no staging or backup residue", async () => { | ||
| const targetPath = path.join(dir, "target.txt") | ||
| await fs.writeFile(targetPath, "old bytes") | ||
|
|
||
| // No platform override: the real platform's own durability and ACL steps run, and this | ||
| // case is the successful restore. A failed restore now throws DaclRestoreError after | ||
| // the commit rename has already happened (the focused unit test "win32 DACL: a failed | ||
| // restore is an error and the dump is still unlinked" covers that path), so on a | ||
| // machine where icacls cannot put a saved ACL back into a throwaway temp directory | ||
| // this publish reports that error instead of resolving, and the residue assertions | ||
| // below are reached only where the restore worked. | ||
| await safeWriteText(targetPath, "new bytes", { backup: true }) | ||
|
|
||
| expect(await fs.readFile(targetPath, "utf8")).toBe("new bytes") | ||
| expect(await fs.readdir(dir)).toEqual(["target.txt"]) | ||
| }) | ||
|
|
||
| it("leaves the target bytes untouched when the commit cannot replace it", async () => { | ||
| // A regular file cannot be renamed over a directory, so the backup copy and | ||
| // the commit both fail on a real filesystem with no mocking at all. | ||
| const targetPath = path.join(dir, "target-dir") | ||
| await fs.mkdir(targetPath) | ||
| const inside = path.join(targetPath, "payload.txt") | ||
| await fs.writeFile(inside, "original bytes") | ||
|
|
||
| await expect(safeWriteText(targetPath, "new data", { backup: true })).rejects.toThrow() | ||
|
|
||
| // The directory and its content are exactly as they were, and no backup copy | ||
| // or staging directory was left behind next to them. | ||
| expect(await fs.readFile(inside, "utf8")).toBe("original bytes") | ||
| expect(await fs.readdir(dir)).toEqual(["target-dir"]) | ||
| }) | ||
|
|
||
| it("cleans up the staging file when the commit rename itself fails", async () => { | ||
| // With backup:false the backup step is skipped, so this is the only shape of | ||
| // this scenario that reaches Step 4: the rename of the staging FILE over the | ||
| // target DIRECTORY fails (EISDIR / ENOTDIR / EPERM depending on platform), and | ||
| // the cleanup must remove the staging file. The backup:true case above bails | ||
| // out in the backup step and never exercises the commit-failure path. | ||
| const targetPath = path.join(dir, "target-dir") | ||
| await fs.mkdir(targetPath) | ||
| const inside = path.join(targetPath, "payload.txt") | ||
| await fs.writeFile(inside, "original bytes") | ||
|
|
||
| await expect(safeWriteText(targetPath, "new data", { backup: false })).rejects.toThrow() | ||
|
|
||
| expect(await fs.readFile(inside, "utf8")).toBe("original bytes") | ||
| expect(await fs.readdir(dir)).toEqual(["target-dir"]) | ||
| }) | ||
| }) |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.