Extend failure messages (issue 97)

[tonatiuh]

This extends the failure message for the matchers be_enqueued", "have_enqueued" and "have_scheduled", so that when the test fails they will show details not only about the expected queue's job but also about existent queue's job.

Here are some examples:

be_queued:

before { Resque.enqueue(Person, 'foo', '123') }
subject { Person }

it { should be_queued('foo', 'bar') }

New failure message:

Failure/Error: it { should be_queued('foo', 'bar') }
       expected that Person would be queued with [foo, bar], but got Person with [foo, 123] instead

Previous failure message:

Failure/Error: it { should be_queued('foo', 'bar') }
       expected that Person would be queued with [foo, bar]

have_queued:

before { Resque.enqueue(Person, first_name, '123') }
subject { Person }

it { should have_queued('foo', 'bar') }

New failure message:

Failure/Error: it { should have_queued('foo', 'bar') }
       expected that Person would have [foo, bar] queued, but got Person with [Les, 123] instead

Previous failure message:

Failure/Error: it { should have_queued('foo', 'bar') }
       expected that Person would have [foo, bar] queued

have_scheduled (with at chain):

let(:scheduled_at) { Time.now }
before { Resque.enqueue_at(scheduled_at, Person, 'foo', '123') }

it "returns true if the arguments are found in the queue" do
  Person.should have_scheduled('foo', 'bar').at(scheduled_at + 2 * 60)
end

New failure message:

Failure/Error: Person.should have_scheduled('foo', 'bar').at(scheduled_at + 2 * 60)
       expected that Person would have [foo, bar] scheduled at 2015-04-19 09:35:40 -0500, but got Person with [foo, 123] scheduled at 2015-04-19 09:33:40 -0500 instead

Previous failure message:

Failure/Error: Person.should have_scheduled('foo', 'bar').at(scheduled_at + 2 * 60)
       expected that Person would have [foo, bar] scheduled at 2015-04-19 09:35:40 -0500

have_scheduled (with in chain):

let(:interval) { 1 * 60 }
before(:each) { Resque.enqueue_in(interval, Person, first_name, '123') }

it "returns true if arguments and interval matches positive expectation" do
  Person.should have_scheduled('foo', 'bar').in(2 * 60)
end

New failure message:

Failure/Error: Person.should have_scheduled('foo', 'bar').in(2 * 60)
       expected that Person would have [foo, bar] scheduled in 120 seconds, but got Person with [Les, 123] scheduled in 60 seconds instead

Previous failure message:

Failure/Error: Person.should have_scheduled('foo', 'bar').in(2 * 60)
       expected that Person would have [foo, bar] scheduled in 120 seconds

[tonatiuh]

@leshill could you check this PR and let me know what do you think?

[leshill]

@tonatiuh Looking…

[leshill]

Not happy with the spacing here, leave it up to the caller.

[tonatiuh]

Thanks, I'll attend this today.

[tonatiuh]

The issues I see if I don't add the blank space are that:

1 - There would be some inconsistency in the failure_message method for the have_scheduled matcher, given that only this method uses the format of ["expected that #{actual} would not have [#{expected_args.join(', ')}] scheduled", @time_info].join(' '), all the others follow the "expected that #{actual} would be queued with [#{expected_args.join(', ')}]#{@times_info}" format.
2 - The last line of the failure_message will have to be updated to ["expected that #{actual} would not have [#{expected_args.join(', ')}] scheduled", "#{@time_info},", actual_queue_message].join(' ') so that there is a comma before showing the existent enqueued job in the failure message, and given that there may be times when the @time_info is empty then an extra blank space would appear in those cases.

What do you think?
(Please let me know if I didn't communicate well any of those points.)

[leshill]

As an example, addressing point 2:

Line 117 could be:

"but got #{actual} with [#{actual_queue_args_str}]#{@times_info} instead"

Line 248 could be:

`["expected that #{actual} would not have [#{expected_args.join(', ')}] scheduled", "#{@time_info}", actual_queue_message].join(', ')

The ", " is inserted by the caller as needed. Each part of the message is just a building block and not specifying how it is joined to the rest of the message.

Does that make sense?

[tonatiuh]

Thanks. I think I see what you say, however, since a comma is added by each element in the array then the resulting failure text goes like the following:

expected that Person would have [a, b] scheduled, at 2015-06-08 17:41:44 -0500, but got Person with [Les, Hill] scheduled at 2015-06-08 17:41:44 -0500 instead

(Please check that there is an extra comma after "scheduled").
I don't really like having an extra comma there and I think point 1 is valid. but you tell me, I can leave it with the extra comma if that works for you.

[leshill]

LOL, yea I did not completely rewrite it, so that will need to be addressed too.

Go ahead with my latest suggestion (extra comma) and I will fix that when I merge the PR. Thanks for the doing the PR.

[tonatiuh]

Thanks.

I just realized that when there is no @time_info two commas will appear as in:

expected that Person would have [Les, asdf] scheduled, , but got Person with [Les, Hill] scheduled instead

This goes for scenario in https://github.com/leshill/resque_spec/blob/master/spec/resque_spec/matchers_spec.rb#L357 for instance. I think you can 1) merge this as it is (since it's working and is consistent, in my opinion) or 2) I leave it is and you do the small modifications you see are needed when merging (having an extra blank space works better for me than having one extra comma).

What do you think?

[tonatiuh]

Let me know in case you need anything else from my side : )