gambol99 / go-marathon

A GO API library for working with Marathon
Apache License 2.0
199 stars 128 forks source link

Why is `Marathon` an interface? #106

Closed mattes closed 8 years ago

mattes commented 8 years ago

I don't think an interface is really needed here and it doesn't feel like DRY. I mean how often are we planning to implement the Marathon interface? :-)

https://github.com/gambol99/go-marathon/blob/8ce3f764250b2de3f2c627d12ca7dd21bd5e7f93/client.go#L36

eicca commented 8 years ago

I think it's the same purpose as here: https://github.com/kubernetes/kubernetes/blob/master/pkg/client/unversioned/client.go#L35 it will allow easier mock testing, so user can inject another mocked marathon client instead of creating a wrapper. In this case it'd be nice to provide this mock implementation.

timoreimann commented 8 years ago

I was also thinking that the purpose is testing-related.

The other alternative I heard of is to use more broad interfaces, like Readers and Writers. I've never tried that approach for something other than very small interfaces, however.

mattes commented 8 years ago

I like that mocking idea. Now that I start thinking about this, wouldn't it be nice, if go-marathon offered a marathon mock implementation?

func NewMockClient(config Config) (Marathon, error)

timoreimann commented 8 years ago

I was thinking the same already but would opt for a fake instead. Unlike a mock where you need to specify behavior (which is an implementation detail), a fake exposes the same API but comes with a simplified implementation. In our case, we'd probably want an in-memory backend instead of a full-blown cluster. See also this Google TotT blog post on the subject (and this and this related post, respectively).

To be fair, we might need mockish properties to test certain scenarios (like failure behaviors). Those could come in addition to a regular fake, however.

eicca commented 8 years ago

wouldn't it be nice, if go-marathon offered a marathon mock implementation?

I was just thinking about it and I didn't come up with an idea how to do it in a simple and flexible way. User needs a way to precisely specify what needs to be returned from a specific marathon call. Maybe I need to check kubernetes code more.

Another idea could be is to provide a fake marathon server (we already use something similar in our tests). In this case it would be crystal clear how to use it + it will be used in our tests as well, so we can re-use code in our tests and expose it for users. In case we'll choose this approach we can get rid of the interface.

timoreimann commented 8 years ago

I'd rename the issue to something like Build Marathon API test double at this stage.

mattes commented 8 years ago

Closing in favor of #108