test(chain-storage): exercise the EXDEV fallback in movePath for real - #3363
Open
Crypto2099 wants to merge 1 commit into
Open
test(chain-storage): exercise the EXDEV fallback in movePath for real#3363Crypto2099 wants to merge 1 commit into
Crypto2099 wants to merge 1 commit into
Conversation
movePath falls back to copy-then-remove when fs.move fails with EXDEV, which is what happens when chain storage is relocated to another disk. Every migration spec stubs the function out with jest.spyOn(manager, '_movePath'), so that branch had never executed in a test — despite being the code that moves a user's entire chain directory, potentially tens of gigabytes. The obstacle was assumed to be privileges: loopback mounts need root. It is not. /dev/shm is a tmpfs on a different device, present and writable both on a developer machine and inside the Nix build sandbox, so a genuine cross-device move is testable with no special setup. Adds chainStorageManagerShared.realfs.spec.ts covering the same-device move, the cross-device move through the EXDEV fallback, and the rethrow of any other error. The cross-device test asserts its own preconditions — that the two roots differ by st_dev, and that a bare renameSync between them really raises EXDEV — because otherwise fs.move would succeed by rename and the test would pass without exercising the fallback. It fails loudly rather than skipping when no cross-device pair is available, since a silent skip on the only test covering this path would report coverage that does not exist. Verified inside the sandbox with nix build .#checks.x86_64-linux.jest, not only on the host: 67 suites, 781 tests.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
movePathfalls back to copy-then-remove whenfs.movefails withEXDEV—what happens when a user relocates chain storage to another disk.
Every migration spec stubs it out with
jest.spyOn(manager, '_movePath'), sothat branch had never executed in a test, despite being the code that moves a
user's entire chain directory.
The obstacle was not what it looked like
Testing
EXDEVwas assumed to need loopback mounts and therefore root./dev/shmis a tmpfs on a different device, present and writable on a developermachine and inside the Nix build sandbox:
So a genuine cross-device move needs no privileges and no setup.
Coverage
fs.movesucceeds by renameTwo deliberate choices
The cross-device test asserts its own preconditions — that the two roots
differ by
st_dev, and that a barerenameSyncbetween them really raisesEXDEV. Without those,fs.movecould succeed by rename and the test would passgreen while exercising nothing.
It fails loudly rather than skipping when no cross-device pair is available.
A silent skip on the only test covering this path would report coverage that does
not exist.
Verification
Run inside the sandbox with
nix build .#checks.x86_64-linux.jest, not only onthe host, because the whole question was whether
/dev/shmis available there:67 suites, 781 tests. Lint, compile, stylelint and treefmt also pass.