-
-
Notifications
You must be signed in to change notification settings - Fork 136
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
withSomeParams modifier for mockGetRequest #227
Conversation
Hmm .. interesting .. and useful .. me like |
This is great! It might be nice to |
Yeah, was thinking about this. Can do it along with 'mutual reset' feature. |
maybe something like: withParams(blah).matchSome()
withParams(blah).matchExact() but by default it would be matchExact |
Not sure.. this looks longer. It would make sense if you wanted to test with exact/partial match with the same params object, but it doesn't look like a common use-case. |
Actually, it might be good idea to just use |
I just don't like too many things like withSomeParams, withExactParams , withMyPetPonyParams .. etc .. withParams(blah); // which means exact match ( by default )
or
withParams().matchSome() // if that is what you mean this way you can do: mock.matchSome() and it seems easier to learn this new syntax since you dont have to rewrite your old code |
Well, to keep less methods, this might be an argument for
|
I see what you mean that about the match(payload) but maybe since there is no payload in mockGet's then it is obvious what you are matching. |
I like that one withParams({}, exact) // exact defaults to true Only reason I throw out other examples is I not sure about use case. I thought you wanted to switch and change the mock from exact to some match .. so I was confused there .. usually you want one or the other I thought |
Are there real parameters with Yes, the initial idea was to just setup mock to match by exact or some params and that's it, but later I really liked the idea that you can use the same mock with new settings as your test goes on. And it makes perfect sense, especially if that mock-stack is something that could affect performance (which we're still investigating). Also I think that it might be good idea to set default exact to |
I hope so ( about params with update ) with ember-data .. I will let you know .. because I trying to pull that out of my hat today. Interesting about sending queryParams that are undefined ( does not ember usually not send param if it is undefined .. ). You might have different use case for that. maybe so about withParams({}, exact) // where you like exact to be false by default .. but right now it is true by default .. so as long as it does not break all existing tests .. I don't mind too much. Also, the match(attrs) in mockUpdate / mockCreate is exact match .. there is not the idea of partial match .. is there? I am mistaken .. where you get the idea that was partial matching going on there ? |
Ummm... maybe I was wrong, but it's just naturally assumed. Let's say, you have a form, but model also has some attributes that not in the form but have defaults (for example). So you'd expect that those attributes get to payload as well, but you're testing just one field in the form.
|
It is exact because you only provide the attrs that you want to exactly match. Oh .. I see what you mean now. So yes, your right, it is partial match. Because you match on what you want to match, though there could be more attrs in the payload .. duuuuuuuhhhhh |
Yeah, right... So I think that match-some-approach is more general for testing. That's why I propose to align modifiers of get-mocks and post-mocks. This could be just more straight-forward for beginners. |
right .. I got you now. Though one more note. There is still a difference between: so that is what was confusing me As long as you can lay out a nice plan though, that makes sense I am up for it. |
Hmm... Right now it's very similar. All params supplied to I have another idea... what if we rename public methods to |
In my opinion, using |
Boy this brings up the idea from a long time ago ( see open issues about with vs withParams ) where someone suggested doing with({params: {}} with({attributes: attrs}) so I like the suggestion, and am just slightly suggesting something else though I am just saying this to stir up ideas because I like matchAttributes or matchParams too. |
Actually, this is pretty good idea. Besides clarity, generalizing |
ok . sure thing .. I like to sit and ponder on things too just to let the particles of ideas settle down a bit |
This PR introduces new functionality to match by some params. Here's the excerpt from documentation additions:
Probably both
withParams
andwithSomeParams
calls might need to clear opposite object to be able to switch between behaviors in flight.kudos to @ryedeer 👍 👍 👍