"time =~ other_time" matcher #167

Closed
wants to merge 3 commits into
from

Projects

None yet

5 participants

@andriusch

Need for this matcher is best shown with example code:

Timecop.freeze(Time.now) # or Time.stub(:now).and_return(Time.now)
user = User.create! # user for example ActiveRecord object
user.reload.created_at.should == Time.now # fails since time field loses milliseconds after reload

So the only other options would be to use be_within (which is more for floats and if I remember correctly doesn't work that well with time objects) or convert times to integers (which then loses readability if test fails).

@travisbot

This pull request fails (merged d08fc7e into d427bac).

@travisbot

This pull request fails (merged 9fa2bf5 into d427bac).

@dchelimsky
Member

Hey @andriusch. This is a very thorough pull request, which I appreciate and hope to see more from you in the future. I am, however, against merging it for 3 reasons:

  • =~ already means matches for regexes (per Ruby) and array contents (per RSpec), and this would add a subtly different meaning: approximately.
  • we're generally moving away from operator matchers to matchers that can be used in both actual.should matcher(expected) or expect(actual).to matcher(expected).
  • the problem this solves is already solved by TimeCop, but the example you cite above uses it incorrectly (see https://github.com/jtrupiano/timecop)

I'm going to leave this open for others to comment on, but that's my feedback.

@travisbot

This pull request passes (merged 5e7ca88 into d427bac).

@andriusch

Hey,
Actually the problem I'm trying to solve here is not solved by Timecop, well at least not in a way I'd like it to be. Basically when you freeze time using Timecop to Time.now (whatever it currently is) Timecop also saves fractions of a second (which is how it should work of course). The problem is that you can't save fractions of a second to database (none that I know at least) so when Time objects are loaded from database they don't have fractions of the second in them and thus comparing to frozen Time.now fails (and on older RSpec it would show awkward message about objects being not equal, while their String representations are equal, I think failure message was changed to not be that weird in newer versions there's still no actual nice way to compare those time objects). Not sure if this explains anything, but maybe expanded code example will:

Timecop.freeze(Time.now)
Time.now.to_f # => 1345894130.7751524
user = User.create!
user.reload
user.created_at.to_f # => 1345894130.0 note the difference between Time.now.to_f
user.created_at.should == Time.now # fails even though it might look like it should pass

So the fundamental problem is that databases don't support fractions of a second, I see three ways of fixing it:

  • Fix all databases to support fractions of a second (yeah...)
  • Change Timecop to ignore fractions of a second when freezing, however I think it would be flawed solution as there may be times where they matter.
  • Add matcher to RSpec to match similar Time objects.

As for name: it doesn't have to use operator matcher, I'd be ok with something like be_similar_time(time) or be_approximate_time(time).

@dchelimsky
Member

The examples on https://github.com/jtrupiano/timecop all show assigning time before freezing, or use a block format rather than assigning during the freeze. I agree that still doesn't solve the problem w/ databases, but it does solve the problem when all the references to Time.now are in the same process.

I think seeing be_similar_time fail would lead to confusion over the meaning of similar (same goes for approximate, etc). We already have more explicit tools you can use (rspec and ruby):

expect(user.created).to be_within(0.9).of(Time.now)
expect(user.created.to_i).to eq(Time.now.to_i)

Both of these examples put the problem front and center and are explicit about what constitutes similarity.

More thoughts?

@andriusch

Well all Time.now references are the same without block form or assigning before freezing too, this is not a problem.

expect(user.created.to_i).to eq(Time.now.to_i)

Problem with this is that if test fails it's really not clear what time values are.

expect(user.created).to be_within(0.9).of(Time.now)

I remember trying this several versions ago and it converted to floats, however seems it's fixed now and the message is much clearer so seems this is good enough. So I'm closing this.

On an unrelated note I've also wanted to extract and add another matcher set for comparing hashes and jsons that I've used in several projects. One of the matchers checks if hash is part of another hash (very useful when writing request specs for JSON APIs) another one checks if hashes are equal. Both of them output developer friendly messages, something like "expected key.subkey[2].subsubkey to be 10 but it was 2". As I said even though they're used for comparing hashes I'm mostly using them for comparing JSONs so not sure if this would be something you would be interested to merge to RSpec or I shouldn't waste my time extracting them?

@andriusch andriusch closed this Aug 25, 2012
@myronmarston
Member

FWIW, 1.9 added Time#round so you can do:

expect(user.created.round).to eq(Time.now.round)
@alindeman
Collaborator

@myronmarston, there might still have an edge case if the user were created right at the boundary of a second and the assertion ran in the "next" second, right?

I'd probably use be_within(1.second).of(Time.now)

@andriusch

@alindeman it's not even an edge case, user.created_at will always have 0 for fraction part after reload from database and Time.now will have fraction part between 0 and 0.(9) so statistically test will fail 1 out of 2 times. I'll stick with be_within too, nice touch on 1.second :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment