wspurgin / rspec-sidekiq

RSpec for Sidekiq
https://github.com/wspurgin/rspec-sidekiq
Other
655 stars 132 forks source link

Any reason why `at_evaluator` using `.to_s` instead of a plain comparison? #138

Closed mvalipour closed 1 year ago

mvalipour commented 7 years ago

This is more of a question, but it's something that has come to huge confusion for me.

In here: https://github.com/philostler/rspec-sidekiq/blob/develop/lib/rspec/sidekiq/matchers/have_enqueued_job.rb#L29

def at_evaluator(value)
  return false if job['at'].blank?
  value.to_time.to_s == Time.at(job['at']).to_s
end

Is there any reason why this isn't using a simple comparison? value.to_time == Time.at(job['at']).

I tend to have a lot of issues/confusion when value is in a different time zone.

mvalipour commented 7 years ago

☝️ I'm happy to PR this if there is a consensus.

dpoetzsch commented 7 years ago

I agree, the timezone issue #124 make this really difficult to debug.

I would prefer a comparison using .to_f instead (which is probably the same as direct comparison?)

a0s commented 7 years ago

I think it should work with .utc https://github.com/philostler/rspec-sidekiq/pull/146

wspurgin commented 1 year ago

Believe this was fixed with https://github.com/wspurgin/rspec-sidekiq/commit/dcc6d037a2333a5af73ead81809b484fd0e9ac13 though it's not a direct comparison as a to_int is used to avoid fractional second mismatches.