Export MockBroker in a separate package

[eapache]

Per #307 (comment) and #307 (comment) this functionality is useful to other people, it just shouldn't live as part of Sarama's API.

I know @wvanbergen had plans to add a bunch of functionality to it, so splitting it out will also make that easier.

Currently the only road-block is it's use of the private encoder type, but that can just be replaced with []byte and the tests can encode the packets ahead of time. This will actually make it more flexible too, by permitting us to test what happens when the broker gives us back garbage packets.

@yagnik this is a nice not-entirely-trivial but danger-free ticket if you're looking to work on your go.

cc @Shopify/kafka @schleppy

[eapache]

Related to the last point (DRY up the tests) of #271 since as-is removing the use of encoder will add a bit more duplication to the test suite.

[yagnik]

I can try to take a stab at this over the weekend

[eapache]

Current open sarama tickets/PRs dealing with the mockbroker that will have to be migrated in some form:

• Improve expectation API to allow for testing of more intricate scenarios #298
• Add examples to godocs. #323

[wvanbergen]

I finally have an idea for MockBroker and MockClient.

1. We implement in a separate package that avoids circular dependencies e.g. sarama/internal/mocks
2. We expose them to the outside world by simply importing them into sarama/mocks.

package mocks

import internal_mocks "sarama/internal/mocks"
type Consumer internal_mocks.Consumer

Would this work?

[eapache]

I believe that would work, 👍 clever

[sybrandy]

Do you plan on having the MockBroker and MockClient available? I have the 1.0.0 release and I don't see them in there. This is important to me as I have code that I want to test to ensure that I'm properly setting up connections, checking broker health, etc.

[sybrandy]

FYI: I'm writing tests right now for what I need to test my interfaces and I'm following the pattern described here: Creating Fakes in Go With Channels. So far, it looks very clean and very easy to integrate into tests. I just wanted to pass this along as I think this pattern would make for very nice mocks without too much effort.

[eapache]

These are planned, but we keep running into technical roadblocks (mostly around circular imports, which go doesn't allow). We don't want to export these types in the main sarama package obviously, but they depend on types from the main sarama package, which means we can't actually use them in tests by default without causing a circular import. It's a bit of a puzzle.

[wvanbergen]

@sybrandy Can you elaborate a bit on what your application does?

If it is a consumer or producer, I would suggest you use the mock producer or mock consumer interface to test your application, and assume that sarama implements those interfaces correctly.

If you don't want to assume sarama's correctness, the right place to add tests is sarama itself, where you have access to the mock broker. We definitely are interested in more tests around failovers and error handling, so we'd really appreciate any contributions there. I'll gladly help you get started with this if you are interested.

[sybrandy]

@wvanbergen It is both a producer/consumer, so I do use both. However, I wanted to test logic that I have that requests topic and partition information. I currently cannot do that with sarama, so I faked it myself (See my previous comment). The fact that Client is an interface helped a lot.

What I'm really looking for is a way to simulate interacting with a broker. Whether that be via a mock broker or a mock Client/Broker, I don't care. I ended up with the latter approach and it worked great. What I have fits my needs right now, but someone else may need this as well.

[wvanbergen]

We definitely need to add a mock Client implementation next to the producer and consumer. Any chance that your mock implementation can be contributed?

[wvanbergen]

Also, we should probably add a Topic() and Partitions(topic) method to Consumer, so you don't have to instantiate Client to get this info (see #366)

[sybrandy]

Unfortunately, no, not at this time. If I get a chance, I'll try to make one. It wasn't hard to do. It's more just finding some free time.

[wvanbergen]

yeah, same here :)

[sybrandy]

As for adding topic/partition to the consumer, will they still be part of the client? Since I instantiate both in my code, I'd rather instantiate just the client once, then create the producer and consumers from that one client. (This reads from multiple topics. I know, probably an odd use case.)

[wvanbergen]

@sybrandy it would just delegate it to the internal Client instance. If you instantiated your Consumer using NewConsumerFromClient, that would be the same client.

[eapache]

Superseded by #570.