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

reading custom matcher result from custom reporter #7594

Closed
kandros opened this issue Jan 9, 2019 · 14 comments
Closed

reading custom matcher result from custom reporter #7594

kandros opened this issue Jan 9, 2019 · 14 comments

Comments

@kandros
Copy link

kandros commented Jan 9, 2019

I'm looking to read the result of custom matcher from a custom reporter

in the codebase I found this https://github.com/facebook/jest/blob/master/packages/expect/src/index.js#L274
so it should already be possible, but i cannot find where in a custom reporter this data is available.

more details at americanexpress/jest-image-snapshot#51 (comment)


this is where reporters are called, but looking at the types i cannot find the result of the matcher
https://github.com/facebook/jest/blob/665a93eec70c4f2d7809d1848fbc8ca60cfd8361/packages/jest-cli/src/ReporterDispatcher.js

@SimenB
Copy link
Member

SimenB commented Jan 9, 2019

@kandros it'd be great if you could follow the issue template. In particular, what do you need it for?


It's lost here: https://github.com/facebook/jest/blob/722049ccd66947d48296dcb666bc99fccab86065/packages/jest-jasmine2/src/reporter.js#L148-L183
and here: https://github.com/facebook/jest/blob/722049ccd66947d48296dcb666bc99fccab86065/packages/jest-circus/src/utils.js#L307-L331

@thymikee @rickhanlonii seems like something we should be able to keep in the results object, no? Just keep matcherResult around if it's a property of the error we get in.

@kandros
Copy link
Author

kandros commented Jan 9, 2019

@SimenB I copied the wrong link, (edited first post)
this is the one correct americanexpress/jest-image-snapshot#51 (comment)

I need to be able to pass custom data from a matcher to a reporter, in the linked example (when result object gets added in the retun value of the matcher along pass and message).

This allows me to have a custom reporter that show the image (the path is in the result)
I also need this information to build a custom watch plugin

@SimenB
Copy link
Member

SimenB commented Jan 9, 2019

PR welcome 🙂 Adding it to this type makes sense to me: https://github.com/facebook/jest/blob/722049ccd66947d48296dcb666bc99fccab86065/types/TestResult.js#L102-L112

If the underlying error has matcherResult add it, otherwise set it to null

@kandros
Copy link
Author

kandros commented Jan 9, 2019

Thanks @SimenB

I can take it, not sure what to do with circus, can you point me to circus's equivalent of jest/packages/jest-jasmine2/src/reporter.js?

@SimenB
Copy link
Member

SimenB commented Jan 9, 2019

It's in that utils file I linked to. It has on real equivalent, but that function does essentially the same as the specResult.failedExpectations.forEach part of jasmine's reporter

@SimenB
Copy link
Member

SimenB commented Jan 9, 2019

Note that to run circus locally, set JEST_CIRCUS=1 in the terminal. Other than that, I think CONTRIBUTING.md has all the details 🙂

@rpgeeganage
Copy link
Contributor

@SimenB
Can I try to contribute to this feature please ?

@kandros
Copy link
Author

kandros commented May 4, 2019

@rpgeeganage due to lack of time I wasn't able to continue that, but I started working on it in January (it was before typescript rewrite, not sure if my changes still make sense) I'm going to check if something can still be used/useful as a start

@rpgeeganage
Copy link
Contributor

@kandros ,
Thanks a lot. I would like to carry on working this.

@rpgeeganage
Copy link
Contributor

@kandros ,
Can you please give me a branch or commit which I can take a look at?
(it will be really useful).
Thanks a lot.

@flozender
Copy link
Contributor

@victorphoenix3 and I would love to take this up!

@SimenB
Copy link
Member

SimenB commented Nov 2, 2020

Wonderful! I'm fine with just adding this to jest-circus, btw - I'll land the PR making it default this week. I won't say no to a PR supporting jasmine of course, but it's not needed unless you want to 👍

@SimenB
Copy link
Member

SimenB commented Nov 5, 2020

It has been pointed out to me that this is probably solved in #9496. We are missing succesful assertions, but we don't currently track those at all. I think it's natural to include those in the report data when we _do_track them though, so I'll close this.

Shout at me if anybody disagree! 🙂

@SimenB SimenB closed this as completed Nov 5, 2020
@jeysal jeysal removed this from the High priority future milestone Feb 28, 2021
@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 May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants