DOP-4701: Preserve stdout in ShellCommandExecutor #1043
Merged
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.
Stories/Links:
DOP-4701
Notes
The actual fix is the one line change in
commandExecutor.ts
.The annoying part was making the test work:
The existing test incorrectly mocked the
child_process.exec()
interface.According to the node docs, the exception raised by
util.promisify(child_process.exec())
will always have astdout
property. But just in case the exception get raised by something else in the try block, it'll fall back to returning''
as in the current behavior.util.promisify(child_process.exec)
has a custom promisifier (child_process.exec[util.promisify.custom]
) that considerably changes the interface, but that jest loses when you calljest.mock('child_process')
, and because of jest magic tomfoolery, I couldn't find any tidy way to restore that custom hook, which is needed to actually make the interface behave correctly.So I thought, you know what? Why mock? What value do we get by mocking
child_process.exec
besides introducing surface areas for bugs?So this PR just executes
commandExecutor.execute(["echo 'stdout output'", "echo 'stderr output' >&2", 'exit 1'])
, which gives us everything we need to make sure ShellCommandExecutor actually works, using only shell builtins that we already require.README updates