arempe93 / bunny-mock

A mock client for RabbitMQ modeled after the Bunny client in ruby
http://www.rubydoc.info/github/arempe93/bunny-mock
MIT License
44 stars 37 forks source link

BunnyMock::Queue#pop api does not match Bunny::Queue#pop #15

Open podung opened 8 years ago

podung commented 8 years ago

I see that BunnyMock::Queue#pop returns a hash with :message and :options keys:

xchg.publish("Hello, everybody!", :routing_key => 'test1')

message = q.pop
message[:message]    # => 'Hello, everybody!'
message[:options]    # => { routing_key: 'test1' }

However Bunny::Queue#pop returns a 3 element array:

xchg.publish("Hello, everybody!", :routing_key => 'test1')

delivery_info, properties, payload = q.pop

I assume that this was to make popping messages off of queues in specs a lot cleaner. However, it makes BunnyMock only really able to help test the publishing of messages. Any code that attempts to use Bunny to read messages from queues would not be able to use BunnyMock.new as a replacement for Bunny.new.

Ideally the APIs would match, however changing it would break anyone using the current behavior. I propose creating some sort of config option (e.g. BunnyMock.use_bunny_queue_pop_api = true) that could default to the current behavior. The README could clearly show how to use the two options.

Thoughts? Let me know if I'm misunderstanding something.

I'll put together a PR if you're open to the idea.

arempe93 commented 8 years ago

Yeah, this is leftover from when this was a personal use thing before I made it into a gem - because I didn't like the 3 element array style...probably not a good idea in hind sight lol. The apis should definitely match - so it can be used as a drop in replacement.

I think your idea is a good one, but we should add a deprecation notice to it - ie. if they keep using the current behavior they will know it will be phased out in a later version and be only available as the Bunny api behavior.

arempe93 commented 8 years ago

@podung - see 1d2e57c

I took your idea and implemented it - along with adding a ton more stuff to match Bunny such as GetResponse and MessageProperties implementations and passing options and blocks to Queue#pop

podung commented 8 years ago

Awesome. I haven't had a chance to look at this thoroughly yet, but thanks for being open to moving in this direction.