GNOME Bugzilla – Bug 749080
gdatetime test: fails if close to rollover between seconds
Last modified: 2015-09-14 11:41:31 UTC
Created attachment 303041 [details] [review] gdatetime test: don't assume that time stands still If we call time(NULL), then do something (however trivial), then call g_date_time_new_now_utc(), they do not necessarily share a seconds value. Let's say the gmtime call takes 2ms. time(NULL) could return xx:xx:23 when the time is actually xx:xx:23.999999, resulting in the g_date_time_new_now_utc() happening at xx:xx:24.000001. This is unlikely, but did happen to me in a parallel build: GLib:ERROR:.../glib/tests/gdatetime.c:674:test_GDateTime_now_utc: assertion failed (tm.tm_sec == g_date_time_get_second (dt)): (23 == 24) A similar argument applies to the rollover from xx:23:59.999999 to xx:24:00, so comparing seconds with a 1s "fuzz" or a >= comparison is not sufficient; and so on into higher-order fields. I haven't seen the other tests that use _now() fail in the same way, but they could.
Review of attachment 303041 [details] [review]: The loops could do with a brief comment stating they’re there to handle the rare case of rollover in the number of seconds.
Created attachment 303087 [details] [review] [v2] gdatetime test: don't assume that time stands still [same long commit message as before] --- v2: * add a comment before each loop * replace an earlier attempt at this, with comment "XXX we need some fuzzyness here", with a check for strict equality; the earlier version would be OK at the boundary between seconds, but would fail at the boundary between minutes
For what it's worth, this is how I fixed the same(?) issue in GstDatetime back in the day: http://cgit.freedesktop.org/gstreamer/gstreamer/commit/tests/check/gst/gstdatetime.c?id=64effe78e76ec434606962a83a250c3b1ab5d612 (don't judge me)
(In reply to Tim-Philipp Müller from comment #3) > For what it's worth, this is how I fixed the same(?) issue in GstDatetime > back in the day That has the same issue at the rollover between minutes (or higher-order time periods), afaics, so it makes the issue 60x less likely but doesn't actually eliminate it.
Review of attachment 303087 [details] [review]: Great!
Does anyone object to this patch, or shall I commit it as reviewed by Philip?
*** Bug 725668 has been marked as a duplicate of this bug. ***
This went in.