Sitelet https://web.archive.org/web/20201123153742/https://github.com/shelljs/shelljs/issues/622
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

Echo tests unnecessarily run tests in own process #622

Closed
freitagbr opened this issue Dec 14, 2016 · 6 comments
Closed

Echo tests unnecessarily run tests in own process #622

freitagbr opened this issue Dec 14, 2016 · 6 comments
Assignees

Comments

@freitagbr
Copy link
Contributor

@freitagbr freitagbr commented Dec 14, 2016

To prevent writing to the process' own stdout, the echo tests were written to run each test in its own process, and the stdout of that process was captured. Because ava runs each test in its own process anyway, the echo tests no longer need to run in separate processes.

@freitagbr freitagbr added the test label Dec 14, 2016
@freitagbr freitagbr self-assigned this Dec 14, 2016
@nfischer
Copy link
Member

@nfischer nfischer commented Dec 14, 2016

Ava only runs tests in their own processes if running in parallel mode. Even still, I don't think it captures the stdio, I think it just goes straight through to the console, which isn't what we want.

We could do away with the stdout capturing if we mock process.stdout and process.stderr.

@nfischer nfischer mentioned this issue Feb 26, 2017
4 of 5 tasks complete
@nfischer
Copy link
Member

@nfischer nfischer commented Apr 10, 2017

@freitagbr ping. Any progress on this?

@freitagbr
Copy link
Contributor Author

@freitagbr freitagbr commented Apr 20, 2017

When I run the echo tests in concurrent mode, echo still prints to stdout. So, I think the best think to do in this situation would be to mock process.stdout and process.stderr. The std-mocks module looks like it might be useful for this purpose.

@nfischer
Copy link
Member

@nfischer nfischer commented Apr 22, 2017

Yeah, that module seems fine

@nfischer
Copy link
Member

@nfischer nfischer commented Apr 22, 2017

I did something similar for shx here and here

The downside to mocking like this is that we'll need to mock/restore within each test case (I don't think you can do it in setup/teardown). This is lots of boilerplate 😦

An alternative approach is to refactor echo() to take a configurable stream. It's always process.stdout in production, but we can set it to some other stream for tests only. But maybe that's too Java-ish 😜

@freitagbr
Copy link
Contributor Author

@freitagbr freitagbr commented May 6, 2017

Fixed in #708

@freitagbr freitagbr closed this May 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.