Repository navigation
fix(tests): make cache-removal and tarball tests pass on Windows (#36) - #38
Merged
Merged
Conversation
…on parsing - Derive ROOT via fileURLToPath and join paths with path.join instead of string concatenation (new URL().pathname yields /C:/... on Windows). - Accept npm pack --dry-run --json output as either a legacy array or the name-keyed object emitted by newer npm.
CacheCleaner and Installer now accept an optional rm function (defaulting to fs/promises rm). On Windows the read-only attribute does not block recursive removal, so chmod-based failure injection silently succeeded and never exercised the warn-and-continue path. Both unit tests now inject a failing rm for one target while delegating the rest — works identically on win32 and POSIX.
The CLI test spawns a subprocess, so failure cannot be injected at a code seam. Keep chmod 0o500 on POSIX; on Windows park a short-lived helper process whose cwd is inside the blocked tree — Windows refuses to delete a directory that is a process's working directory, so the removal fails with EBUSY and the warn-but-exit-0 path is exercised cross-platform.
diegohb
changed the title
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.
Summary
CacheCleanerandInstallernow accept an optionalrmfunction (defaulting tofs/promisesrm), so tests inject removal failures at a code seam instead of relying onchmod 0o500.chmod 0o500on POSIX; on Windows it parks a short-lived helper process whose cwd is inside the blocked tree, so removal fails withEBUSYand the warn-but-exit-0 path is exercised cross-platform.ROOTviafileURLToPathand join paths withpath.joininstead of string concatenation, which previously produced/C:/...paths on Windows.npm pack --dry-run --jsonoutput is now accepted as either a legacy array or the name-keyed object emitted by newer npm.Why
Several tests were failing on Windows: the read-only attribute set by
chmod 0o500does not block recursive removal there, so the failure injection silently succeeded and the warn-and-continue path was never exercised. Additionally,new URL().pathnamestring concatenation yields invalid root paths on win32, andnpm pack --jsonoutput shape varies across npm versions.Testing
CacheCleanerandInstallerunit tests inject a failingrmfor one target while delegating the rest; behavior is identical on win32 and POSIX.chmod 0o500on POSIX, cwd-holding helper process on Windows.pack --jsonoutput shapes.