Refactor value matchers and postFix dependent syntax#56
Conversation
jozic
left a comment
jozic
left a comment
There was a problem hiding this comment.
looks good
left some minor comments
| when(aMock.bar) thenReturn "mocked!" <=> aMock.bar shouldReturn "mocked!" | ||
| when(aMock.bar) thenReturn "mocked!" thenReturn "mocked again!" <=> aMock.bar shouldReturn "mocked!" andThen "mocked again!" | ||
| when(aMock.bar) thenCallRealMethod() <=> aMock.bar shouldCallRealMethod | ||
| when(aMock.bar) thenCallRealMethod() <=> aMock.bar shouldCall realMethod |
There was a problem hiding this comment.
are there any other things you can type after shouldCall?
If not, then I'm not sure it's worth splitting it, just my 2c
There was a problem hiding this comment.
It was also postfix notation and had a similar compiler error if it had a follow up line
There was a problem hiding this comment.
what about shouldReturn whateverRealMethdoReturns (not really sure what to put as the shouldReturn parameter, but it would make it more consistent if we can find something that reads nicely
There was a problem hiding this comment.
i think shouldReturn whateverRealMethdoReturns looks too long, so shouldCall realMethod is probably better
another way looking at it can be aMock.bar should not be mocked
There was a problem hiding this comment.
should not be would clash with ScalaTest, so it's a no go
I wasn't being literal with whateverRealMethdoReturns :P just looking for suggestions
| verify(aMock, times(2)).bar <=> aMock.bar wasCalled twice | ||
| verify(aMock, times(6)).bar <=> aMock.bar wasCalled sixTimes | ||
| verifyNoMoreInteractions(aMock) <=> aMock was never called again | ||
| verifyNoMoreInteractions(aMock) <=> aMock wasNever calledAgain |
There was a problem hiding this comment.
alternatively this can be
aMock wasCalled neverAgain
it looks more consistent with wasCalled twice and so on
| verify(aMock).bar(any) <=> aMock.bar(*) was called | ||
| verify(aMock, only).bar <=> aMock.bar wasCalled onlyHere | ||
| verify(aMock, never).bar <=> aMock.bar was never called | ||
| verify(aMock, never).bar <=> aMock.bar wasNever called |
There was a problem hiding this comment.
alternatively aMock.bar wasCalled zeroTimes or aMock.bar wasCalled never
it's more consistent with the rest of the api, though I admit wasNever Called (or was neverCalled) reads better
There was a problem hiding this comment.
yeah, I'll take a second look
There was a problem hiding this comment.
What about wasNot called and wasNot calledAgain
I know still doesn't uses the wasCalled but reads much better and it allows you to copy/paste and just add 'Not' (I've found myself doing that many times, so I think it's a popular use case`
also using Not instead of Never seems nicer, the reason I didn't do it before was that not was a keyword and it clashed with the same defined by ScalaTest, but if I use it as part of the method name then that's not an issue anymore
There was a problem hiding this comment.
naming things is hard, and there always will be who thinks it can have a better name, so i suggest juts pick something that compiles in all circumstances and be happy :)
i personally think that consistency is more important than following English, easy to remember => quick to use => more productive
but again, this is just me
There was a problem hiding this comment.
I see your point, but one of the main purposes of the DSL is to get rid of awkward reading sentences as well, and the current form is consistent with what it was there before (in both version 1.0.0 and 0.x.x) so I'm gonna keep it like this, history will judge me XD
Thanks a lot for your help!
2 participants
Refactor eqToVal matcher (Fixes #55)
Replace all syntax that requires postFix notation (Fixex #54) (@jozic please take a look)