Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let stdin be passed through to jest-cli #5017

Closed
seanpoulter opened this issue Dec 5, 2017 · 18 comments
Closed

Let stdin be passed through to jest-cli #5017

seanpoulter opened this issue Dec 5, 2017 · 18 comments
Labels

Comments

@seanpoulter
Copy link
Contributor

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

What is the current behavior?
When Jest runs in watch mode, jest-cli only listens for input from text terminals that send characters one at a time in "raw mode". This dependency on tty.ReadStream#setReadMode in jest/packages/jest-cli/src/watch.js prevents anyone who's spawned Jest as a child process from interacting with the watcher (e.g.: IDE extensions like vscode-jest).

What is the expected behaviour?
How do you feel about using setRawMode when it's available, and letting whatever data through if a feature toggle is set (e.g.: a CLI arg or process.env)? Any thoughts on the feature toggle?

Suggested change:
From:

  if (typeof stdin.setRawMode === 'function') {
    stdin.setRawMode(true);
    stdin.resume();
    stdin.setEncoding('hex');
    stdin.on('data', onKeypress);
  }

To:

  const useRawInput = typeof stdin.setRawMode === 'function';

  if (useRawInput) {
    stdin.setRawMode(true);
  }
  if (useRawInput || roughEdgeFeatureToggle) {
    stdin.resume();
    stdin.setEncoding('hex');
    stdin.on('data', onKeypress);
  }

On my end it's the lowest overhead of:

  • using stdin that's piped in
  • kill/respawn Jest processes when things in the "globalConfig" change
  • refactor watch.js to extract the dependency on input method

Thanks!

@SimenB
Copy link
Member

SimenB commented Dec 5, 2017

Any drawbacks to this? /cc @rogeliog @orta

@seanpoulter
Copy link
Contributor Author

Ugh. There's also a simple workaround, sorry. Got too caught up in the problem to see that we can spawn a child process using Node's --require arg to pre-load a stub for setRawMode. I'll close the issue since this is WAY easier for such a low reward Issue.

@SimenB
Copy link
Member

SimenB commented Dec 5, 2017

Should it be in the docs?

@seanpoulter
Copy link
Contributor Author

Is there a spot you had in mind to document it? I think we'll be OK. This feature is will end up with a PR to extend jest-editor-support so we can interact with watch from vscode-jest. Since both of those packages are already cited as examples for how to interact with Jest, I can make sure to add extra context in the paper trail.

@SimenB
Copy link
Member

SimenB commented Dec 6, 2017

Yeah, might not be any fitting place for it as we don't really document any programmatic APIs

@seanpoulter
Copy link
Contributor Author

With @segrey thinking the node --require workaround "doesn't look safe enough" I'd say we've got enough interest to support non-raw mode terminals. Let's figure this one out.

What's our next steps @segrey?

@seanpoulter seanpoulter reopened this Jan 12, 2018
@SimenB
Copy link
Member

SimenB commented Jan 12, 2018

I still think the original suggestion makes sense

@segrey
Copy link

segrey commented Jan 12, 2018

What concerns me a bit is that the node --require workaround may stop working for piped stdin at some point in future. A possible idea is make non-raw terminal case prominent in code, in hope that subsequent changes in this area will respect it. Something like the proposed change by @seanpoulter in the issue description would be nice. Additionally, we can remove MockStdin.setRawMode method.

@seanpoulter
Copy link
Contributor Author

Should we go ahead and add a CLI option then? I don't really want to add clutter, but it seems to be the best spot for our use case and to document the feature.

Non-raw mode is called "cooked" or "canonical" but it could also be "rare". As much as I want to suggest "allow cooked stdin" as an option, how about:

--acceptPipedStdin:
Use this flag to interact with Jest from another process when stdin is not in raw mode.

Other than a few more tests, I think that's the scope of the change.

@segrey
Copy link

segrey commented Jan 12, 2018

Maybe we can support piped stdin always? (Are there cases when it should be disabled?)
If yes, then adding a test for piped stdin to prevent regressions would be enough imho.

@seanpoulter
Copy link
Contributor Author

Maybe we can support piped stdin always?

Oooh, brave. 👍 Right now Jest stays open waiting for input that'll never come, right? Can only get better from there. If you enter an unrecognized key in the watch usage prompt there's no action, so we're good on that end too (observed at the prompt, not via a code review).

@seanpoulter
Copy link
Contributor Author

You might like to have a look at #5399 @segrey. It's an interesting proposal to build on the watch mode plugin API. It's operating right in the same block of code where isInteractive is false and we can't see the prompt.

@segrey
Copy link

segrey commented Apr 19, 2018

Unfortunately, after #4958, no watch usage is shown if jest process is running in non-interactive environment. Since IDEs/tools start jest test runner with redirected streams, no watch usage is shown here.

@wintercounter
Copy link

I'd like to spawn a jest-cli child-process dynamically but it simply breaks on first input. Would be nice to find a permanent solution to this.

@PabloSzx

This comment has been minimized.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants