Fix Windows portability issues, move fakeJsdoc to jsdocStub #1
Conversation
This file contains 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
Should've been part of commit 3492207. This fixes the problem of not finding the `jsdoc` stub on Windows, but now the tests in main.test.js are failing on Windows with: Serialized Error: { errno: -4094, code: 'UNKNOWN', syscall: 'spawn' } Which is odd, because runJsdoc() calls spawn() and runJsdoc.test.js passes just fine. Investigating...
There were a few problems here. First, I added an error handler to the
process spawned within spawnMain() to make the tests fail faster instead
of timing out after five seconds. This made iterating on the following
problems much faster.
Today I learned that on Windows, we need to use process.env.Path, not
process.env.PATH. So now the new PATH_KEY constant is used in place of
raw env.PATH accesses and `{ PATH: "..." }` object literals. This was
the biggest of the problems.
Next, the jsdoc.CMD invocation kept echoing its commands to standard
output, causing tests to fail. The solution was to add `@echo off` as
the first line of the jsdoc.CMD script.
In runMainWithoutJsdoc from main.test.js, `jsdocDir` contained unescaped
backslashes on Windows, so the regular expression created from it didn't
match the jsdoc path. The fix was to replace all `\` in `jsdocDir` with
`\\` first.
Finally, spawnMain() from main.test.js now runs the mainPath script by
passing process.execPath (the path to the Node interpreter) as the first
argument. This is because Windows can't interpret the shebang at the top
of the mainPath script. This is also portable to macOS and Linux.
In the process (ha!), I also stopped manipulating process.env.PATH in
spawnMain() after remembering I could just define a new env object. This
also enabled returning the Promise directly from the function, rather
than using await internally to reset process.env.PATH afterwards.
This doesn't change any functionality, but better reflects the nature of the jsdoc test double scripts. "Fake" would imply that the doubles are fully functional, but merely scaled down implementations. These are really more "stubs," incomplete implementations to simulate specific operations and responses.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
There were a few problems here. It looks like a lot based on the description, but the actual overall change isn't very large.
First, I applied url.fileURLToPath to main.test.js, which should've been part of commit 3492207. This fixed the problem of not finding the
jsdocstub on Windows.I then added an error handler to the process spawned within spawnMain() to make the tests fail faster instead of timing out after five seconds. This made iterating on the following problems much faster.
Today I learned that on Windows, we need to use process.env.Path, not process.env.PATH. So now the new PATH_KEY constant is used in place of raw env.PATH accesses and
{ PATH: "..." }object literals. This was the biggest of the problems.Next, the jsdoc.CMD invocation kept echoing its commands to standard output, causing tests to fail. The solution was to add
@echo offas the first line of the jsdoc.CMD script.In runMainWithoutJsdoc from main.test.js,
jsdocDircontained unescaped backslashes on Windows, so the regular expression created from it didn't match the jsdoc path. The fix was to replace all\injsdocDirwith\\first.Finally, spawnMain() from main.test.js now runs the mainPath script by passing process.execPath (the path to the Node interpreter) as the first argument. This is because Windows can't interpret the shebang at the top of the mainPath script. This is also portable to macOS and Linux.
In the process (ha!), I also stopped manipulating process.env.PATH in spawnMain() after remembering I could just define a new env object. This also enabled returning the Promise directly from the function, rather than using await internally to reset process.env.PATH afterwards.
And one more housekeeping thing: I renamed the fakeJsdoc test fixture to jsdocStub. This didn't change any functionality, but better reflects the nature of the jsdoc test double scripts.
"Fake" would imply that the doubles are fully functional, but merely scaled down implementations. These are really more "stubs," incomplete implementations to simulate specific operations and responses.