petergtz / pegomock

Pegomock is a powerful, yet simple mocking framework for the Go programming language
Apache License 2.0
252 stars 28 forks source link

Adding NotEqualsMatcher and generated matcher factory #113

Closed yhrn closed 3 years ago

yhrn commented 3 years ago

This also adds:

See #110

yhrn commented 3 years ago

And would you mind adding a couple of tests validating that both the NotEq... matchers and the XThat... matchers actually do what they're supposed to do? Those can go into dsl_test.go.

Will do

And I was wondering, do you intend to add the pegomegaMatcher matcher as well in a separate PR, or was your plan to just make that possible, but not add it to the library?

My initial intention was not to add that into the library, just to make such a matcher straightforward to use without additional boilerplate for each argument type. I meant it as an example, it didn't really feel like library quality with how the String() method works. But if you would like it in the library then I don't mind adding it. The other consideration here is that it takes a dependency on Gomega but I guess that could be avoided by defining an equivalent interface in Pegomock.

Another thing that I just realized is that I should probably add XThat matcher factories for the "basic" types in matcher.go. But thinking about that made me realize that this could make the other factories kind of obsolete if the exiting built in matchers had differently named constructors. For example all the EqX() factories could be replaced with XThat(IsEqualTo()). What do you think about this direction? Obviously removing existing factories is a breaking change so we wouldn't do that but they could be deprecated.

petergtz commented 3 years ago

it didn't really feel like library quality with how the String() method works.

TBH, I haven't had a detailed look at how this would come out as an actual message in pegomock. Need to look into this a bit closer.

But if you would like it in the library then I don't mind adding it.

I like Gomega, so why not make interoperability with it easy. Feel free to do that in a separate PR if you want to get this one out first. Whatever works better for you.

The other consideration here is that it takes a dependency on Gomega but I guess that could be avoided by defining an equivalent interface in Pegomock.

Yes, would definitely define an equivalent interface in Pegomock to avoid the dependency.

For example all the EqX() factories could be replaced with XThat(IsEqualTo()). What do you think about this direction?

I considered this briefly too when you originally proposed the XThat..., and it's certainly more modular. I'm concerned a bit about the simplicity that the EqXs have and which then would go away. Also, XThat(IsEqualTo(...)) is certainly not as succinct anymore, even when you shorten it to XThat(Equals(...)) or XThat(IsEq(...)) or similar.

Oh, and there's one more thing :-). How about documenting the new features in the README? This could also go into a separate PR if you prefer. But it would be a shame if these cool new things stay hidden for most.

yhrn commented 3 years ago

Sounds reasonable, I'll add the tests and XThat matcher factories in matcher.go as well as update the README a bit. I'll try to get to that in a day or so. And then we can discuss the PegomegaMatcher in a separate PR.

petergtz commented 3 years ago

Super cool! Thank you so much, @yhrn.