cli: fix handling of non-TTY environment #6970
Closed
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
Replace all remaining places invoking generator from tests via
`helpers.run()` to use `executeGenerator` instead, to ensure that
generator errors are correctly propagated back via a rejected promise.
Before this change, a failed generator behaved as if it succeeded. A
test suite would continue and fail later when one of the assumptions
are not true. As a result, the test error messages were cryptic and
made troubleshooting difficult.
For example, when running the following command from `packages/cli`:
CI=1 lb-mocha test/acceptance/app-run.acceptance.js < /dev/null
Before my change, the suite would fail as follows:
1) app-generator (SLOW)
"before all" hook: install dependencies for·
"passes `npm test` for the generated project":
ValidationError: No packages remain after filtering [ '@loopback/sandbox-app' ]
at filterPackages (node_modules/@lerna/filter-packages/filter-packages.js:54:13)
With my change in place, the suite correctly fails in the scaffolding
step:
1) app-generator (SLOW)
"before all" hook: scaffold a new application for
"passes `npm test` for the generated project":
SyntaxError: Unexpected end of JSON input
at JSON.parse (<anonymous>)
at AppGenerator._readJSONFromStdin (lib/base-generator.js:132:19)
Signed-off-by: Miroslav Bajtoš <[email protected]>
Before this change, when the config read from `stdin` was an empty
string, the generator would fail with the following message:
SyntaxError: Unexpected end of JSON input
at JSON.parse (<anonymous>)
at AppGenerator._readJSONFromStdin (lib/base-generator.js:132:19)
With this fix in place, an empty JSON string is treated as an empty
config object `{}` and the generator can continue its work.
Please note this change is probably a partial fix only. AFAICT,
the application generator is still not correctly scaffolding the project
when running in a non-TTY mode, despite the fact that it reports success.
Signed-off-by: Miroslav Bajtoš <[email protected]>
bajtos
commented
Jan 4, 2021
| if (jsonStr === '') { | ||
| debug('Received an empty string, returning back empty config: {}'); | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
Do we want to add a test to verify this fix? Such test would have to spawn a new child process to invoke the generator. That seems like a too much effort to me, considering that other parts of _readJSONFromStdin are not covered by tests either, at least AFAIK.
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.
An attempt to fix our CLI to correctly work in non-TTY environments, such as GitHub Actions runners.
See also #5110 and #4607
Checklist
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈