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

spawn() fails to properly exercise terminal applications #136

Closed
bcoe opened this issue May 15, 2016 · 7 comments
Closed

spawn() fails to properly exercise terminal applications #136

bcoe opened this issue May 15, 2016 · 7 comments
Milestone

Comments

@bcoe
Copy link

bcoe commented May 15, 2016

I noticed while attempting to recreate a failing test case related to yargs #497 that I could not get citgm to fail (as expected) on a test outputting a large chunk of text to the terminal:

info:    module name:        | yargs               
info:    version:            | 4.7.0               
info:    done                | The smoke test has passed.

After digging further, I realized that it is related to spawn() being run with the default pipe behavior. If I switch to stdio: "inherit", tests start failing:

screen shot 2016-05-15 at 2 39 00 pm

Unfortunately, using inherit results in very noisy output.

I'm not sure what the best fix is here, but I think we'd be smart to eliminate the discrepancy between a user interacting with the terminal, and citgm consuming a spawned terminal output.

@bcoe
Copy link
Author

bcoe commented May 17, 2016

@jasnell @thealphanerd silly question, could we live with the noisy output? the value is that we exercise libraries that interact with stdout/stderr in a more realistic way ... we'd now be in a position to catch nodejs/node#6456 in the future.

@MylesBorins
Copy link
Contributor

if the test suite still passes and it doesn't cause CI to run super slow I'm all for it

@jasnell
Copy link
Member

jasnell commented May 17, 2016

I'm ok with it also as long as it makes the tests more reliable and we still get the output we need.

@bcoe
Copy link
Author

bcoe commented May 17, 2016

@jasnell @thealphanerd I'll submit a working branch that has the noisy output, and a failing run of yargs related to 6456.

@MylesBorins
Copy link
Contributor

Would landing nodejs/node#4598 on v4.x help?

We are no longer support 0.10 / 0.12

@MylesBorins MylesBorins added this to the v2.0.0 milestone Jan 4, 2017
@targos
Copy link
Member

targos commented May 18, 2019

Is there still something to do here?

@bcoe
Copy link
Author

bcoe commented May 18, 2019

@targos I think you could probably close this, it's pretty stale.

@BethGriggs BethGriggs closed this as not planned Won't fix, can't repro, duplicate, stale Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants