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

Fix console reporter #8739

Closed
wants to merge 1 commit into from
Closed

Conversation

palmerj3
Copy link
Contributor

Summary

Fixes a bug that set result.console to undefined for jest reporters. Seen here.

jest-community/jest-junit#45
#8499

Test plan

See that a jest reporter shows console data for suites with > 1 test.

@@ -40,7 +40,6 @@ export default class ReporterDispatcher {
// Release memory if unused later.
testResult.sourceMaps = undefined;
testResult.coverage = undefined;
testResult.console = undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the correct approach, see #8233 (especially the discussion about it breaking reporters)

/cc @scotthovestadt

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - would be great to remove console completely from the TestResults type then if it's functionally always set to undefined.

A lot of users of jest-junit want to include console output in the junit.xml output (which is compliant junit) but that's not possible without this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, we could maybe do a delete. Should deffo fix the typings if they are off. And clarify in the docs 🙂

(this was also probably a breaking change...)

@palmerj3
Copy link
Contributor Author

palmerj3 commented Jul 23, 2019

Another interesting find is that there is zero possibility of console output in reporters if you only have one test. I think due to https://github.com/facebook/jest/blob/master/packages/jest-core/src/runJest.ts#L230

@SimenB can you think of a reason to set verbose to true if there is only 1 test? The CustomConsole (which gets called if verbose is true) doesn't buffer console output and always returns undefined when getBuffer is called.

@SimenB
Copy link
Member

SimenB commented Jul 23, 2019

CustomConsole should buffer for reporters as well, that's a bug. Didn't know doing --verbose broke console for reporters 😬

It's set to verbose so you see every single test title. Not sure if useful or not...

@palmerj3
Copy link
Contributor Author

I'll close this PR for now - with the approach listed in the issue you linked I was able to get console output showing in jest-junit.

@palmerj3 palmerj3 closed this Jul 23, 2019
@palmerj3
Copy link
Contributor Author

Gives a good reason now for folks to use jest-junit as a reporter instead of a testResultsProcessor I guess :)

@github-actions
Copy link

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.
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 May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants