On Thu, Aug 2, 2018 at 11:58 PM, Michael Gross <mich...@tutti.ch> wrote: > Thanks for this. > > > What it boils down to for me is having a better reasoning for this > suggestion from CodeReviewComments: > > > “The implementing package should return concrete (usually pointer or struct) > types” > > > And that’s the reasoning: > > “that way, new methods can be added to implementations without requiring > extensive refactoring” > > > Even if you would return an interface rather than a concrete type when > adding new methods to it you do not have to refactor any client packages. So > why is it that “extensive refactoring” is required in this case?
If you return an interface type other packages may choose to implement that interface type for their own tests. Then if you add a new method to the interface, you will break that code. Ian > On Tuesday, July 31, 2018 at 7:15:53 PM UTC+2, Jake Montgomery wrote: >> >> I'm sure everyone has their own opinion, but here are some ideas. First >> your final question: >> >>> >>> If so, what are the downsides of having z returning an interface instead >>> of a concrete type >> >> >> I think the link you provided covers that: >> >> "The implementing package should return concrete (usually pointer or >> struct) types: that way, new methods can be added to implementations without >> requiring extensive refactoring." >> >> I would stick with that advice pretty much across the board. I would also >> resist the urge to define a single interface for a,b and c to use. >> >>> Code duplication because the interface ZClient is implemented in all the >>> depending packages with the test fixtures for it. >>> >>> It is very easy to end up with the same mocking code in different >>> packages. >> >> >> Small amounts of code duplication are ok, and even part of go philosophy. >> I know that runs counter to the philosophy of many other languages. So if >> the overhead of mocking is small, or the needs of a, b and c vary, then I >> would stick with the separate implementations of the mock code/ >> >>> Maintainability >>> >>> When refactoring package z, client side testing code has to be adjusted >>> in all the depending packages. >> >> >> >> This is a bit of a red herring. If z is refactored internally, then the >> public API will not change, so there should be no need to adjust the a,b and >> c tests or their mocks. If, OTOH, you are refactoring the public interface >> if z, then you will, presumably also need to adjust the a, b and c code. So >> a little adjusting of their tests is just part of that. Really, breaking >> changes to the public API of a package should be avoided anyway. So by >> coding to make that easier you are really coding for failure. (This talk by >> Rich Hickey about breaking changes is not about go, but is worth watch.) >> >> >> So. If a,b and c are public libraries, I would try to stick with the >> separate mocking, as you describe. (Public here means truly public, or >> widely used within a large organization.) If, on the other hand, a, b and c >> are all packages that serve a single final executable, or possibly even a >> small suite of closely related executables, all maintained by the same >> development group, then you might take a more pragmatic approach, while >> still maintaining good practices. Given my comments above I would still >> default to keeping it all separate until it is clear that it is creating a >> real burden. >> >> So, if the mocking requires non-trivial amounts of code, and the >> conditions above are met, you could consider creating a separate package >> that contains the z-mock helpers. That package can be imported by a, b and >> c. If you do that, try to code the mock package using good library package >> design practices. And I would have the z-mock functions return concrete >> types, as described in >> https://github.com/golang/go/wiki/CodeReviewComments#interfaces. As long as >> it implements the same functions as z, then it will also fulfill the >> interfaces that a,b and c take. >> >> Good luck. >> >> On Monday, July 30, 2018 at 3:51:25 AM UTC-4, Michael Gross wrote: >>> >>> Hello list, >>> >>> >>> I have a question about testing packages in conjunction with the general >>> recommendations from >>> >>> https://github.com/golang/go/wiki/CodeReviewComments >>> >>> >>> Suppose we have packages a,b and c which depend on a package z. Say z >>> implements all the necessary unit and integration tests it needs to test >>> itself. >>> >>> Farther more it has a factory 'func New() z.Client' returning a client >>> which all the depending packages use. >>> >>> >>> Following these recommendations >>> >>> https://github.com/golang/go/wiki/CodeReviewComments#interfaces >>> >>> >>> z.Client is implemented as a struct >>> >>> Say z.Client has one method which all the depending packages use >>> >>> >>> func Request(o Opts) map[string]interface{} >>> >>> >>> In order to test a, b and c a mock implementation of z.Client is needed. >>> >>> >>> Still following the CodeReviewComments recommendations we implement in >>> each of the depending packages a,b and c this interface >>> >>> >>> type ZClient interface { >>> >>> Request(o Opts) map[string]interface{} >>> >>> } >>> >>> >>> and code against it. >>> >>> Now when writing the tests for the packages a,b and c since the returned >>> map of the request call is general we use fixtures >>> >>> implementing the required cases. >>> >>> >>> stuff like >>> >>> r := c.EXPECTRequest(gmock.Any()).Return(map[string]interface{}{ >>> >>> `special_flag_a` : true, >>> >>> `treat_this_as_an_error` : `timeout`, >>> >>> }) >>> >>> >>> IMO there are several down sides to this approach >>> >>> >>> Code duplication because the interface ZClient is implemented in all the >>> depending packages with the test fixtures for it. >>> >>> It is very easy to end up with the same mocking code in different >>> packages. >>> >>> >>> Maintainability >>> >>> When refactoring package z, client side testing code has to be adjusted >>> in all the depending packages. >>> >>> >>> So is this the suggested way of implementing this? >>> >>> >>> If so, what are the downsides of having z returning an interface instead >>> of a concrete type and have the package also implement a public mock >>> >>> with corresponding fixtures? >>> >>> > -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
