PR is submitted. Thank you so much Pascal for helping me get it done.
PS: maybe you guys could add a link to https://help.github.com/articles/fork-a-repo/ on http://www.groovy-lang.org/contribute.html#code for total github newbies like me. 2015-06-10 12:10 GMT-03:00 Fabio de Matos Quaresma Gonçalves < [email protected]>: > Thank you so much Pascal, I'm always overwhelmed by those layers of > administrative security, I guess you know what I mean. > > I'm about to submit the PR, I have a couple of questions first: > > > - should I send it based on master or GROOVY_2_4_X branch? > - how do I reference it? is the following commit message enought? > "GROOVY-7462: Dates factory fix when initializing miliseconds field" > > > BTW I was having problems building the whole thing, I'm not sure exactly > where it failed and can't afford putting more time on this right now. > Although I could run ./gradlew :groovy-json:test with 0 errors (2 new > unit tests included) , I'd be more comfortable with someone else running > the full build+test suite for me. > > > > > > 2015-06-09 13:43 GMT-03:00 Pascal Schumacher <[email protected]>: > >> Hi Fabio, >> >> I created https://issues.apache.org/jira/browse/GROOVY-7462 for this. >> >> It would be nice if you could reference it from your pull request. >> >> Thanks and kind regards, >> Pascal >> >> Am 09.06.2015 um 18:35 schrieb Fabio de Matos Quaresma Gonçalves: >> >> Thanks, >> >> I'll take my chances building gradle, so I can try the patch and >> writing a unit test for that. >> >> BTW, do you guys need a JIRA issue to go along with the pull request? >> I'd stay away from it unless you get very sad ;) I've used JIRA before but >> I don't have an apache account. >> >> 2015-06-09 13:26 GMT-03:00 Pascal Schumacher <[email protected]>: >> >>> Hi Fabio, >>> >>> thanks for reporting this. :) >>> >>> You should be able to create a issue for the groovy project here (as >>> long as you have a Apache JIRA account): >>> >>> https://issues.apache.org/jira/secure/CreateIssue!default.jspa >>> >>> You should be able to submit a pull request here: >>> >>> https://github.com/apache/incubator-groovy >>> >>> Thanks and kind regards, >>> Pascal >>> >>> >>> Am 09.06.2015 um 17:17 schrieb Fabio de Matos Quaresma Gonçalves: >>> >>>> I found a bug on Dates class, on how it instantiates objects without >>>> explicitly specifying the miliseconds parameter. By omission, it ends up >>>> using the value provided by Calendar.getInstance(), which is copied over >>>> from System.currentTimeMillis(). This is a problem when comparing dates, >>>> even if I don't care about such high resolution. >>>> >>>> >>>> I reproduced it using groovy console v 2.4.3 on JVM 1.8. I also cloned >>>> the git repo and I'm sure the bug is still unsolved. >>>> >>>> Unfortunately this code is tricky to run, I had it returning the same >>>> amount of miliseconds if the Thread.sleep line is removed. Even with a >>>> println in between constructors didn't guarantee the expected result. >>>> >>>>>>>>>>>>>>> >>>> import groovy.json.internal.Dates >>>> import groovy.transform.TypeChecked >>>> Date d1 = >>>> Dates.toDate(TimeZone.getTimeZone("GMT"),2015,06,07,23,55,59) >>>> Thread.sleep(1) // lets get some time between calling >>>> constructors >>>> Date d2 = >>>> Dates.toDate(TimeZone.getTimeZone("GMT"),2015,06,07,23,55,59) >>>> >>>> >>>> println Date.getMillisOf(d1) + "!=" + Date.getMillisOf(d2) >>>> >>>> assert d1 == d2 >>>> <<<<<<<<<<<<<<< >>>> >>>> The problem is in the following code (suggested fix is commented within >>>> snippet): >>>> >>>>>>>>>>>>>>> >>>> private static Date internalDate(TimeZone tz, int year, int month, >>>> int day, int hour, int minute, int second) { >>>> Calendar calendar = Calendar.getInstance(); >>>> >>>> calendar.set(Calendar.YEAR, year); >>>> calendar.set(Calendar.MONTH, month - 1); >>>> calendar.set(Calendar.DAY_OF_MONTH, day); >>>> calendar.set(Calendar.HOUR_OF_DAY, hour); >>>> calendar.set(Calendar.MINUTE, minute); >>>> calendar.set(Calendar.SECOND, second); >>>> calendar.setTimeZone(tz); >>>> >>>> // missed setting ms to some deterministic value >>>> calendar.set(Calendar.MILLISECOND, 0); >>>> >>>> return calendar.getTime(); >>>> } >>>> <<<<<<<<<<<<<<< >>>> >>>> An alternative way to see the bug and fix it: >>>> >>>> >>>>>>>>>>>>>>> >>>> public static Date toDate(TimeZone tz, int year, int month, int day, >>>> int hour, int minute, int second) { >>>> return internalDate(tz, year, month, day, hour, minute, >>>> second); // this is bugged >>>> //return internalDate(tz, year, month, day, hour, minute, >>>> second,0); //this will work >>>> } >>>> <<<<<<<<<<<<<<< >>>> >>>> I'd gladly submit a jira issue or a pull request, but apparently I need >>>> permission for any of those. >>>> >>>> I'd like to stay unsubscribed to the emailing list if possible, so >>>> please CC me in the reply. >>>> >>>> >>>> -- >>>> Fabio de Matos Gonçalves >>>> >>> >>> >> >> >> -- >> Fabio de Matos Gonçalves >> >> >> > > > -- > Fabio de Matos Gonçalves > -- Fabio de Matos Gonçalves
