search

brendon / acts_as_list

An ActiveRecord plugin for managing lists.
http://brendon.github.io/acts_as_list/
MIT License
2.05k stars 356 forks source link

Broken tests related to time comparison #238

Closed brendon closed 7 years ago

brendon commented 7 years ago

We're getting failures like this:

TouchTest#test_moving_item_to_top_touches_all_other_items [/home/travis/build/swanandp/acts_as_list/test/test_list.rb:723]:
No visible difference in the Time#inspect output.
You should look at the implementation of #== on Time or its members.

I implemented timecop to try and alleviate the occasional failure of tests due to millisecond creep. I know there is a way to build in allowance for the time to change slightly (milliseconds) in the test comparison but would rather not since we shouldn't have to.

Would one of you be able to take a quick look if you've had experience with these kinds of tests before? I've tried everything I can think of to make sure that the times are the same.

fabn commented 7 years ago

Are you able to reproduce the failures locally? I tried but wasn't able to do that. Ideally, as you told these issues are solved using Timecop and/or assert_in_delta but timecop alone should be enough.

It would be interesting to see time differences in failures using Time.now.to_f to make the comparison, and also to put a sleep to see if for some reason timecop is not working.

In this moment I cannot try it because I just upgraded my laptop to OsX Sierra and I'm having some troubles when building native extension. As soon as I fix these issues I'll try to reproduce the issue locally.

brendon commented 7 years ago

Thanks @fabn, I can't reproduce locally either. Might be something to do with Ubuntu/Linux itself. I tried adding the sleep and the tests still pass locally so I think timecop is working. I like your idea of converting them to floats (or to_i) first.

brendon commented 7 years ago

Fixed this in #240 and #241. The tests are green on master again :)