graze / queue

:postbox: Flexible abstraction for working with queues in PHP.
MIT License
49 stars 10 forks source link

Add support for setting DelaySeconds for a Message #35

Closed bjornpost closed 8 years ago

bjornpost commented 8 years ago

See #34

mention-bot commented 8 years ago

By analyzing the blame information on this pull request, we identified @adlawson, @sjparkinson and @h-bragg to be potential reviewers

bjornpost commented 8 years ago

Not sure on how to fix the broken tests. Directions welcome :-).

h-bragg commented 8 years ago

Within the file SqsAdapterTest.php testEnqueue method it expects some calls to metadata-get with MessageAttributes.

This method should also include mocks for calls to 'DelaySeconds'.

$this->messageA->shouldRecieve('getMetadata->get')->once()->with('DelaySeconds')->andReturn(0);

and the sendMessageBatch method should have entries for the DelaySeconds property:

['Id' => 0, 'MessageBody' => 'foo', 'MessageAttributes' => [], 'DelaySeconds' => 0],

Also there should be tests to ensure the DelaySettings functionally is actually working.

  1. If DelaySeconds is set in the message metadata we expect to see if being passed to sendMessageBatch

We will also need to consider what the default behaviour here should be and write some specific tests for those.

  1. If DelaySeconds is set in the options and not set on a Message what is the expected behaviour? DelaySeconds should either not be there or should be the same as the options setting
  2. If DelaySeconds is not set at all and not set in options? Either delay seconds should not be passed to the sendMessageBatch method or be set to 0.

Does that make sense?

If you would like me to write some of these tests let me know.

bjornpost commented 8 years ago

@h-bragg Spend a good hour or so trying to fix these tests, but Mockery keeps throwing exceptions that do not make a lot of sense to me. If you would like to add the tests: go for it. :-)

h-bragg commented 8 years ago

@bjornpost I created a new branch (and PR) #36 which the tests as I am unable to modify your branch.

The tricky mockery exceptions are due to it not showing all of the differences between expected and actual arguments for method calls it is expecting.

bjornpost commented 8 years ago

Closing this in favor of #36