CLI incorrectly assumes non-interactive mode

[bajtos]

Our CLI app allows users to provide prompt answers in multiple ways:

• Some values can be configured via CLI arguments and options, e.g. name.
• It is possible to provide JSON object with values to all prompts via --config option.
• It is possible to stream a JSON object to stdin

At the moment, the logic determining when to read data from stdin is based on process.stdin.isTTY flag. Unfortunately, this is often causing confusing behavior.

• When debugging our CLI tool via Visual Studio Code, application's stdin stream is in non-TTY mode. As a result, CLI uses different way (code path) to prompt for options.
• The outcome of our test suite depends on whether the stdin stream is available and in TTY mode. When running the tests with stdin disabled (e.g. via npm test < /dev/null), many of the tests fail.

I would like us to improve the logic deciding how to prompt for options and make it more robust, less suspect to different ways how CLI can be legitimately invoked.

Acceptance Criteria

To be added by the team after discussing with @bajtos.

[bajtos]

Re-posting from https://github.com/strongloop/loopback-next/issues/4425#issuecomment-584100049:

We call BaseGenerator.prompt, which decides to call super.prompt. We ask a single prompt for name and receive an empty answers object back. At this point the process hangs.

I see two problems here:

1. Why is the process hanging? IMO, the generator should abort with an error. I think this is the problem I described in https://github.com/strongloop/loopback-next/issues/4425#issuecomment-578195144, this should be easy to debug & fix on any OS (e.g. MacOS).

AFAICT, when I run tests with stdin read from /dev/null, then we don't get as far as prompting for options like name, however the tests don't always pass. The outcome is different depending on how I limit the tests executed and whether Mocha is configured with no timeout.

Few examples (executed from packages/cli):

• lb-mocha "test/integration/generators/model.integration.js" -b -t 0 < /dev/null All tests pass

• $ lb-mocha "test/integration/generators/model.integration.js" -b < /dev/null The test does not run without package.json fails on 2s timeout. (More tests would fail on timeout too if I didn't provide -b argument to Mocha.)

• $ lb-mocha "test/**/*.js" -b < /dev/null

  Fails with the following report:

  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 (/Users/bajtos/src/loopback/next/node_modules/@lerna/filter-packages/filter-packages.js:54:13)
      at /Users/bajtos/src/loopback/next/node_modules/@lerna/filter-options/lib/get-filtered-packages.js:39:5

[deepakrkris]

In our last estimation meeting we had several doubts about this story. The most important question was,

1. Can the user still use the CLI by piping a JSON file ?
2. Is this story addressing one particular drawback in the CLI - user not being able to copy paste the JSON content into the terminal ?
3. Can the user still do without this feature in all environments, will this be a pain in user experience in any shell/env ?

cc: @bajtos

[bajtos]

Can the user still use the CLI by piping a JSON file ?

IIRC, some shells (Windows cmd.exe?) impose a limit on the maximum size of command-line arguments. IMO, it's important to allow the user to provide CLI config object by piping a JSON document to CLI's stdin, to avoid that limit.

However, I don't think it's necessary to auto-detect "JSON via stdin" scenario. Personally, I prefer tools to be explicit and have a special combinations of CLI arguments to enable this mode.

AFAICT from reading the source code, the following invocations is already supported and should work well:

$ cat config.json | lb4 app --config stdin

As I see it, the problem is caused by the following invocation that is also supported but the implementation is brittle:

$ cat config.json | lb4 app

Is this story addressing one particular drawback in the CLI - user not being able to copy paste the JSON content into the terminal ?

The goal of this story is to make CLI tests more robust and easier to debug in a debugger. I'd like to fix the following two use cases:

• When debugging our CLI tool via Visual Studio Code, application's stdin stream is in non-TTY mode. As a result, CLI uses different way (code path) to prompt for options.
• The outcome of our test suite depends on whether the stdin stream is available and in TTY mode. When running the tests with stdin disabled (e.g. via npm test < /dev/null), many of the tests fail.

IMO, the ability to paste the config JSON content into terminal should be preserved when the CLI is invoked via lb4 --config stdin, there is no need to change that.

What may change as a result of fixing the behavior: at the moment, the following command is triggering a non-interactive mode where the CLI is expecting JSON data in stdin:

$ cat | lb4

Without the current auto-detection, this may no longer work and users may need to call lb4 --config stdin instead.

---

I am proposing to remove the code checking process.stdin.isTTY and get the following behavior:

1. lb4 app is always running in interactive mode (gathering data via prompts UI).
2. lb4 app --config stdin is always running in non-interactive mode, reading JSON config from stdin.

@raymondfeng IIRC, you had strong opinions about isTTY-based behavior. Are you ok with my proposed change? Do you have any other suggestions how to improve robustness of our CLI under tests and in the debugger?

Essentially, I want our test suite to pass when it's executed via npm run mocha < /dev/null.

---

I re-run the example scenarios described in https://github.com/strongloop/loopback-next/issues/4607#issuecomment-584115492 and confirm that they still work the same way.

Executed from packages/cli:

• lb-mocha "test/integration/generators/model.integration.js" -b -t 0 < /dev/null passed
• lb-mocha "test/integration/generators/model.integration.js" -b < /dev/null failed on a timeout. I also tried with -t 5000, -t 10000 and even with -t 100000, but the tests still timed out.
• lb-mocha "test/**/*.js" -b < /dev/null failed with Error: The stdin is not a terminal. No prompt is allowed..

Executed from monorepo root:

• npm run mocha -- -b < /dev/nullfailed with Error: The stdin is not a terminal. No prompt is allowed.

[raymondfeng]

What's the use case behind < /dev/null. IIUC, /dev/null is mostly useful for output.

[bajtos]

What's the use case behind < /dev/null. IIUC, /dev/null is mostly useful for output.

< /dev/null is just an easy way how to reproduce the scenario when LB4 CLI is invoked with an empty input stream. This is often the case when invoking CLI programmatically, as part of a larger automated scenario.

It seems that GitHub Actions are running scripts this way, see https://github.com/strongloop/loopback-next/pull/5110#issuecomment-753851007

[bajtos]

What's the use case behind < /dev/null. IIUC, /dev/null is mostly useful for output.

< /dev/null is just an easy way how to reproduce the scenario when LB4 CLI is invoked with an empty input stream. This is often the case when invoking CLI programmatically, as part of a larger automated scenario.

It seems that GitHub Actions are running scripts this way, see #5110 (comment)

On the second thought, the purpose of < /dev/null may also be to trigger non-TTY mode. I am not sure now, it has been quite some time since I was looking into this problem last time.