-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Use outputStream instead of stdout for output file #4858
Conversation
Oh wow that are a lot of failing tests that I was not expecting, I'll look into them |
packages/jest-cli/src/run_jest.js
Outdated
} | ||
} | ||
return options.onComplete && options.onComplete(runResults); | ||
}; | ||
|
||
export default async function runJest({ | ||
export default (async function runJest({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why paren?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rogeliog: mind pushing this over the finish line so we can ship Jest 22? |
3fa33d7
to
ed5fb8a
Compare
@@ -77,7 +77,7 @@ const processResults = (runResults, options) => { | |||
const filePath = path.resolve(process.cwd(), outputFile); | |||
|
|||
fs.writeFileSync(filePath, JSON.stringify(formatTestResults(runResults))); | |||
process.stdout.write( | |||
options.outputStream.write( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only ended up changing it for the case when --json --outputFile and --useStderr
are used at the same time. Changing it for --json --useStderr
wihtout --outputFIle
broke a lot of integration tests, which made me curious if that was even the expected behavior. That is why I left it the way it was.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense this way. Does it still fix the issue?
Codecov Report
@@ Coverage Diff @@
## master #4858 +/- ##
==========================================
- Coverage 61.66% 61.66% -0.01%
==========================================
Files 213 213
Lines 7070 7069 -1
Branches 3 3
==========================================
- Hits 4360 4359 -1
Misses 2709 2709
Partials 1 1
Continue to review full report at Codecov.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary
Fixes #4795
Test plan
This uses the output stream instead of a hardcoded
stdout