Fix Windows compatibility #78
Conversation
| const error = await t.throwsAsync(cpy('license', t.context.EPERM), /EPERM/); | ||
| t.true(error instanceof CpyError); | ||
| const cpy = cpyMockedError('cp-file'); | ||
| await t.throwsAsync(cpy('license', t.context.dir), {message: /cp-file/, instanceOf: CpyError}); | ||
| }); | ||
|
|
There was a problem hiding this comment.
These tests are checking for proper handling of exceptions thrown by dependencies. Previously, the test were implemented with the contrived scenario of a no-access directory which would force these modules to fail: fs.mkdirSync(t.context.EPERM, 0); on *nix systems. This has been replaced with mocking of the dependencies to fail immediately. With this, the additional cp-file + glob test also becomes redundant.
While a similarly contrived example could have been created for Windows ( e.g. writing to a known folder ) successful test execution would have been dependent on the environment the test was run in and whether the shell had elevated privileges.
| return path.join(destination, dirname, basename); | ||
| const dirname = path.dirname(source); | ||
| const parsedDirectory = path.parse(dirname); | ||
| return path.join(destination, dirname.replace(parsedDirectory.root, path.sep), basename); |
There was a problem hiding this comment.
destination and dirname can both be absolute paths. When this occurs you end up with the following:
# MacOS & Linux
/tmp/example + /tmp/example/cwd => /tmp/example/tmp/example/cwd
# Windows
C:\\tmp\\example + C:\\tmp\\example\\cwd => C:\\tmp\\example\\C:\\tmp\\example\\cwdThe resulting Windows path is invalid. This strips the root ( C:\\ ) from the directory path before joining by replacing it with the path seperator. This will be a NOOP on non-Windows.
There was a problem hiding this comment.
Good catch. Would you be able to add a test for that?
There was a problem hiding this comment.
This is covered by the 'path structure' test which was failing prior to this fix.
|
Thanks for fixing this :) |
Resolves #64