Support processing keypress by watcher for non-tty stdin

[segrey]

Do you want to request a feature or report a bug?

Feature

What is the current behavior?

Currently, when jest is running with non-tty stdin, "Watch Usage" is printed:

Watch Usage
 › Press f to run only failed tests.
 › Press o to only run tests related to changed files.
 › Press p to filter by a filename regex pattern.
 › Press t to filter by a test name regex pattern.
 › Press q to quit watch mode.
 › Press Enter to trigger a test run.

However, no typed characters are processed (e.g. f, o, p, t, q, Enter). This is happening, because watcher registers keypress event handler if process.stdin.setRawMode is defined (https://github.com/facebook/jest/blob/v22.0.5/packages/jest-cli/src/watch.js#L311), and for non-tty stdin process.stdin.setRawMode is not defined.

What is the expected behavior?

Would be great to allow processing stdin for non-tty stdin too. E.g. make process.stdin.setRawMode(true) optional.

Please provide your exact Jest configuration and mention your Jest, node,
yarn/npm version and operating system.

jest: 22.0.5
node: 9.2.1

[seanpoulter]

Have you found #5017 @segrey? We've been discussing this for vscode-jest and @Raathigesh's Majestic. It seems like a high-risk messy effort to separate the IO method from the package (jest-cli?), so I've been considering using a hackish --require as a workaround.

#5242 discussed using the setupFiles to configure console.log. Do you think you could define process.stdin.setRawMode there as well? Let us know. ;)

[segrey]

Oops, missed #5017, sorry about the duplicate. Thanks @seanpoulter.

It seems like a high-risk messy effort to separate the IO method from the package (jest-cli?)

Sorry, not sure I understand it fully. What IO method is subject for separation here? AFAIU, your suggested change in #5017 is what's exactly needed for tools that run jest with piped stdin. Yeah, hackish --require workaround is also applied by IntelliJ integration, but it doesn't look safe enough imho. If jest can work with piped stdin, why not to support it?

#5242 discussed using the setupFiles to configure console.log. Do you think you could define process.stdin.setRawMode there as well? Let us know. ;)

Nope, setupFiles are required in worker processes. Even when running in a single process, setupFiles are required after registering the keypress handler in watcher.js.

[segrey]

Closing the issue as a duplicate of #5017. Hope #5017 will be reopened.

[seanpoulter]

What IO method is subject for separation here?

Sorry, that isn't clear at all. What I meant to say is, there's a lot of logic in jest-cli that's tightly coupled to the user interface using stdin. If we wanted to interact with Jest on the web or over IPC, it'd be useful to split the test scheduling, watch mode, etc. from jest-cli from the user interface.

If jest can work with piped stdin, why not to support it?

It sure can. The way I understood it, if we're handling input from non-raw terminals we'll have to split the stream to be one character at at time. It's not a technical challenge, but I'm not sure how you'd handle signals, unicode characters, etc..

Thanks for confirming setupFiles won't work. 👍


BTW, I closed #5017 which lets me re-open it. Let's get'r done.

[segrey]

Thanks, now I see your point about separating IO methods from jest-cli. Agree, it makes sense if we wanted to interact with Jest remotely (over socket/IPC). However, in this particular case, we want to communicate via stdin, so jest-cli looks like a good place to handle different types of stdin. It works great with tty streams already and reports test results correctly to piped stdout/stderr. Would be great to make it aware of piped stdin.

The way I understood it, if we're handling input from non-raw terminals we'll have to split the stream to be one character at at time. It's not a technical challenge, but I'm not sure how you'd handle signals, unicode characters, etc..

Yeah, splitting the stream to one character is not great. Maybe tools can do this work on their own and Jest can avoid splitting the stream. E.g. here is how IntelliJ integration sends input to Jest:

    // https://github.com/facebook/jest/blob/v21.2.1/packages/jest-cli/src/constants.js
    Map<Integer, Integer> keyCodeMapping = ContainerUtil.newHashMap(
      Pair.create(KeyEvent.VK_DOWN, 0x1b5b42),
      Pair.create(KeyEvent.VK_LEFT, 0x1b5b44),
      Pair.create(KeyEvent.VK_RIGHT, 0x1b5b43),
      Pair.create(KeyEvent.VK_UP, 0x1b5b41),
      Pair.create(KeyEvent.VK_BACK_SPACE, SystemInfo.isWindows ? 0x08 : 0x7f),
      Pair.create(KeyEvent.VK_ENTER, 0x0d),
      Pair.create(KeyEvent.VK_ESCAPE, 0x1b)
    );
    editor.getContentComponent().addKeyListener(new KeyAdapter() {

      @Override
      public void keyTyped(KeyEvent event) {
        if (event.getKeyChar() != 0) {
          sendCode(event.getKeyChar());
        }
      }

      @Override
      public void keyReleased(KeyEvent event) {
        Integer newKeyCode = keyCodeMapping.get(event.getKeyCode());
        if (newKeyCode != null) {
          sendCode(newKeyCode);
        }
      }

      private void sendCode(int keyCode) {
        if (!processHandler.isProcessTerminated()) {
          OutputStream input = processHandler.getProcessInput();
          if (input != null) {
            try {
              input.write(keyCode);
              input.flush();
            } catch (IOException e) {
              LOG.warn("Failed to send input to jest process", e);
            }
          }
        }
      }
    });

Regarding handling signals, explicitly sent signals (via kill) are still received, and signals generated by shell (e.g. Ctrl+C) are disabled by stdin.setRawMode(true) anyway.
Sorry, didn't get a possible issue with unicode characters. Seems it should work as long as a tool writes correct bytes to stdin. Though there might be some issues with unicode, unsure.

[seanpoulter]

I'm all smiles. Thanks for sharing that, and let's build this with the expectation that folks send the right things in their piped stdin.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.