-
Notifications
You must be signed in to change notification settings - Fork 38
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
Added a verifyExpectedMocksCalled which only asserts on expectCalledWith #64
base: master
Are you sure you want to change the base?
Conversation
…ith; added a flag when.expectCallShouldAssert to opt into disabling the existing behavior of expectCalledWith
I appreciate the effort on this. :) I'm thinking it's a bit awkward to set a global I'm thinking the better way to do this is just make multiple individual assertions in your code with |
Agreed, and agreed. I don't like changing behavior with flags, nor do I like having a vastly different contract for two different calls of the same function. It should probably be a separate thing, but I just wanted to get the ball rolling. I just wanted to show some code to demonstrate:
I also agree that I could just add a I don't think it's necessarily more readable to have expectations at the bottom. I realize the common wisdom is that tests go:
...and that expectations are just another kind of assertion. But I think treating them the same way puts you in a weird spot. It tends to end up like:
I don't like the extra cognitive load of having to deal with a stripped-down version of the expectation as well as the full one. Interaction-based testing already tends toward duplicating the assumptions of the implementation code in the test code. I don't want to think about my implementation when I'm writing a test. I just wanna say what I really mean to say. The more I worry about keeping the call stack happy, the further I stray from TDD nirvana. I'd rather lean into it and just say: Interaction-based tests are fundamentally different from state/value-based testing. You should separate the two anyway, and your tests should SHOUT that they are two very different things. I phrase it as:
...and apologize for no transgression. 😆 Not everyone agrees with that, which is fine. But I think there's room for both approaches. Probably. I think. ¯\(ツ)/¯ |
Thanks for the write up and sharing your thoughts :) Let me see... 🤔
Just a quick aside, I'm not sure I'd describe this project as solely a boilerplate eliminator, it's more of a "let's add argument matching in an elegant way that feels like canonical Jest". I'm not sure what you mean by Expectation Lite? Do you feel that the setting up of the when matchers is akin to setting up expectations? I could see this... you are essentially saying I'm expecting this function to be called with these arguments, and when it is, I want to return this value. Why is the regular |
Bingo. You have to go 90% of the way there to establish a return value, matching the signature of the call you expect, so then if you want to assert that it was called it's mostly copy-paste and changing one word.
Fair enough. :)
I'd like to be able to establish a pattern in my codebase where it's very clear that: This is a class I'm going to use interaction-based testing on, so I'll set up some baseline canned responses in the I haven't tried this, but I gathered from https://github.com/timkindberg/jest-when#assert-the-args that As for I'm okay with backing off of this, btw. I just thought there was an appetite for it based on discussion in other issues/PRs. |
Ok I think the request is clear to me now... particularly the need to set up some non-asserted calls in a beforeEach that may or may not be called in every test... but then some asserted calls in particular tests. Thanks for explaining a bit more. I'm still a bit curious why |
Yeah. Asserting early means that you can't describe something like:
|
Hmm ok I think maybe if we can have an elegant name for it ... and I wonder should we just auto revert it in our own And ... should we also call the
Thoughts? |
I think we could even make lazy expect calls the default in the next major version... and then have a |
...and also added a flag
when.expectCallShouldAssert
to opt into disabling the existing behavior ofexpectCalledWith
.Not sure this is 100% in line with how you'd want to go about this, but if I'm gonna request a feature I can at least take a crack at it, right? 😉