infinitered / cdq

Core Data Query for RubyMotion
MIT License
172 stars 35 forks source link

datetime fields don't store times with odd seconds #86

Closed johngallagher closed 9 years ago

johngallagher commented 9 years ago

Schema:

schema "001" do
  entity "Entry" do
    datetime :startedAt
    datetime :finishedAt

    integer64 :duration
  end
end

Test:

it 'stores startedAt correctly' do
  Entry.create(startedAt: Time.new(2014, 6, 2, 0, 0, 0)).startedAt.should == Time.new(2014, 6, 2, 0, 0, 0)
  Entry.create(startedAt: Time.new(2014, 6, 2, 0, 0, 1)).startedAt.should == Time.new(2014, 6, 2, 0, 0, 1)
end

rake spec gives:

- stores startedAt correctly [FAILED - 2014-06-02 00:00:00 +0100.==(2014-06-02 00:00:01 +0100) failed]

For some reason cdq is rounding the time with 1 second down to 0 seconds...

I've created a repo with a failing test case to describe the issue here:

https://github.com/johngallagher/failing_cdq_test

Any help as to why this is happening would be great! :)

johngallagher commented 9 years ago

Also, I'm running on 10.9.5, with Ruby Motion 3.7, sqlite3 3.7.13

johngallagher commented 9 years ago

Interestingly, when I clone down the cdq repo onto my machine and run all the tests they all pass.

Including, of course, the lines in your tests where you do pretty much exactly what I do in mine - add a second onto the time then check it...

https://github.com/infinitered/cdq/blob/f0b11fc760d25da03ded43b43430dc1812a2bf63/spec/timestamp_spec.rb#L30-L41

kemiller commented 9 years ago

Does this happen if you use an NSDate?

johngallagher commented 9 years ago

Sadly, yes. Sorry - I should have said in the first place I tested it with Time, Date, NSDate and it fails with them all.

I added another test to my test suite:

https://github.com/johngallagher/failing_cdq_test/blob/master/spec/main_spec.rb#L27-L35

Result:

Application 'failing_cdq_test'
  - accurately sets startedAt for even seconds
  - accurately sets startedAt for odd seconds [FAILED - 2014-06-02 00:00:00 +0100.==(2014-06-02 00:00:01 +0100) failed]
  - accurately sets startedAt for even seconds for NSDates
  - accurately sets startedAt for odd seconds for NSDates [FAILED - 2014-06-02 00:00:00 +0100.==(2014-06-02 00:00:01 +0100) failed]
johngallagher commented 9 years ago

Oh! Just had a thought.

I do get the warning:

/Users/johngallagher/.gem/ruby/2.2.0/gems/activesupport-3.2.21/lib/active_support/values/time_zone.rb:270: warning: circular argument reference - now

every time I run tests... could this be something to do with the issue?

I updated the code to remove this warning, but still the tests failed. Which indicates this has nothing to do with the price of fish.

kemiller commented 9 years ago

Curious that you're getting an error in active support. That should only be active while parsing the schema files, which happens in regular ruby, not rubymotion.

I poked around on the console a bit, and discovered that this will set the date correctly:

Time.new(2014, 6, 2, 0, 0, 1.01)

To make this even odder, if you just print out the object details, internally it has the right date. It's only when it gets spit back out that it's wrong.

There's some kind of weird data type conversion issue going on. But the code we are dealing with there is pure Core Data, CDQ isn't really trapping any of this stuff (try by just setting the property). My best guess is that something is going wrong in the RM code that magically makes all NSDates into ruby Times.

johngallagher commented 9 years ago

Aha! Interesting. Yeah, I even looked through your code for NSDates, Time etc and there's nothing - as you say, it looks like an issue with the internals of RubyMotion.

It does seem very odd that you write a test for updated_at and created_at and it converts and reads back out perfectly, then in my code I'm essentially doing something very similar and it does that funny rounding thing.

I'll post it on the RubyMotion forum and close this for now. Thanks for your help! :)

johngallagher commented 9 years ago

Oh and the error in ActiveSupport isn't anything to do with anything. It's a known issue and I can easily fix the warning by tweaking the code a little and it has zero effect.