Fix have_enqueued_sidekiq_job matchers 'at' and 'in' evaluators

wspurgin / rspec-sidekiq

RSpec for Sidekiq

https://github.com/wspurgin/rspec-sidekiq

Other

655 stars 132 forks source link

Fix have_enqueued_sidekiq_job matchers 'at' and 'in' evaluators #150

Closed col closed 4 years ago

col commented 6 years ago

Doing string comparisons here is problematic plus it's confusing as the output displayed when the matcher fails displays the time as a unix epoch float.

This changes the at matcher to use .to_f so that the comparison matches the users expectations.

I decided to make the in matcher use to_i as it would be impossible to compare (with millisecond precision) the exact same time based on something like

expect(Job).to have_enqueued_sidekiq_job().in(5.seconds)

There are several open issues related to this: #133 #138 #124

There is also an existing pull request but I don't agree with this implementation: #146

coveralls commented 6 years ago

Coverage increased (+3.2%) to 97.778% when pulling d95c0f859de29446431abd88dd3617d360bc2541 on col:develop into 807cb1314bb284fb7ea64dc08cbab0896b16f74f on philostler:develop.

coveralls commented 6 years ago

Coverage decreased (-17.5%) to 79.501% when pulling 4818ae807b446f8349483cdb7369e3107c3e13fd on col:develop into 12135312a7868353a51586bf97489dd2c3f0846d on philostler:develop.

geeosh commented 6 years ago

@col I had the same issues on my CI -- I posted another PR (#153) which is essentially a dupe, but it can be merged as yours has merge conflicts with develop. I can close mine if you'd like to solve your merge conflicts.

trevorrjohn commented 6 years ago

What is the status of this? I will fix the merge conflicts but I haven't seen any movement here.

col commented 5 years ago

Just stumbled back across this. I've now resolved the conflicts and would still really like to see this get merged and released. 🙏

wingrunr21 commented 5 years ago

@packrat386 thoughts here?

denys281 commented 4 years ago

Ping! cc @wpolicarpo

packrat386 commented 4 years ago

Sorry this took to so long to get to. #153 has been pulled into the latest release (3.1.0), which should address this issue. Gonna close this PR for now, but feel free to re-open if you're still seeing problems.