Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testDateMidnightSerWithTypeInfo test dependent on $TZ #49

Closed
tpot opened this issue Nov 6, 2014 · 10 comments
Closed

testDateMidnightSerWithTypeInfo test dependent on $TZ #49

tpot opened this issue Nov 6, 2014 · 10 comments
Milestone

Comments

@tpot
Copy link

tpot commented Nov 6, 2014

Hi there. I'm running into a problem with the test suite. It doesn't pass unless the $TZ environment variable is set to "GMT":

$ TZ=Australia/Sydney mvn test
-------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.fasterxml.jackson.datatype.joda.JodaSerializationTest
Tests run: 17, Failures: 2, Errors: 0, Skipped: 0, Time elapsed: 0.622 sec <<< FAILURE!
Running com.fasterxml.jackson.datatype.joda.JodaMapperTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.023 sec
Running com.fasterxml.jackson.datatype.joda.TestVersions
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.003 sec
Running com.fasterxml.jackson.datatype.joda.DateTimeTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.07 sec
Running com.fasterxml.jackson.datatype.joda.deser.ReadablePeriodDeserializerTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.037 sec
Running com.fasterxml.jackson.datatype.joda.deser.MiscDeserializationTest
Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.151 sec
Running com.fasterxml.jackson.datatype.joda.MixedListTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.029 sec

Results :

Failed tests:   testDateMidnightSer(com.fasterxml.jackson.datatype.joda.JodaSerializationTest): null     expected:<"2001-05-2[5]"> but was:<"2001-05-2[4]">
  testDateMidnightSerWithTypeInfo(com.fasterxml.jackson.datatype.joda.JodaSerializationTest): null expected:<...Midnight","2001-05-2[5]"]> but was:<...Midnight","2001-05-2[4]"]>

Tests run: 68, Failures: 2, Errors: 0, Skipped: 0

$ TZ=GMT mvn test
------------------------------------------------------
 T E S T S
-------------------------------------------------------
Running com.fasterxml.jackson.datatype.joda.JodaSerializationTest
Tests run: 17, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.613 sec
Running com.fasterxml.jackson.datatype.joda.JodaMapperTest
Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.018 sec
Running com.fasterxml.jackson.datatype.joda.TestVersions
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.002 sec
Running com.fasterxml.jackson.datatype.joda.DateTimeTest
Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.064 sec
Running com.fasterxml.jackson.datatype.joda.deser.ReadablePeriodDeserializerTest
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.031 sec
Running com.fasterxml.jackson.datatype.joda.deser.MiscDeserializationTest
Tests run: 35, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.147 sec
Running com.fasterxml.jackson.datatype.joda.MixedListTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.028 sec

Results :

Tests run: 68, Failures: 0, Errors: 0, Skipped: 0

Could this be a bug in the code, or just an unexpected external dependency in the test suite?

Regards,

Tim.

@cowtowncoder
Copy link
Member

Good question, I was wondering the same myself. I don't know the answer at this point.

@cowtowncoder
Copy link
Member

Hmmh. I haven't been able to reproduce the failure, even with trying to set different TZ for JVM as per:

http://stackoverflow.com/questions/2493749/how-to-set-a-jvm-timezone-properly

so I think I could use some help if anyone knows good invocation.

@ZioberMichal
Copy link
Contributor

Hi all,
actually I have noticed the same problem on my computer. I did a little test and after two hours of debugging I have noticed that it depends from chronology object which is used. Simple example:

        System.out.println("Default time zone: " + DateTimeZone.getDefault().getID());
        DateMidnight dateInUtc = new DateMidnight(2001, 1, 22, DateTimeZone.UTC);
        System.out.println("Milis for UTC: " + dateInUtc.getMillis());
        DateMidnight dateInLocal = new DateMidnight(2001, 1, 22);
        System.out.println("Milis for local: " + dateInLocal.getMillis());

Above prints:

Default time zone: Europe/Warsaw
Milis for UTC: 980121600000
Milis for local: 980118000000

When we use constructor with time zone it will be used to create chronology class - ISOChronology.getInstance(zone). Without time zone default chronology will be used: ISOChronology.getInstance().

Everything looks ok until org.joda.time.format.DateTimeFormatter#printTo(StringBuffer buf, long instant, Chronology chrono) method is invoked. To be precise below line changes time zone to UTC:

chrono = selectChronology(chrono);

In my opinion this changing of time zone causes this bug. I am not sure, this is just my assumption.

Let me know what do you think about it.

Best regards,
Michał

@cowtowncoder
Copy link
Member

@ZioberMichal sounds plausible yes. The underlying question is then whether test needs to make sure proper TZ is used, or whether DateMidnight handling (serialization and/or deserialization) is at fault here.

@ZioberMichal
Copy link
Contributor

@cowtowncoder I think that it depends from DateTimeFormatter contract. If this class work in this way when we do not pass TZ then we should set TZ and problem will be resolved. Another question is maybe DateTimeFormatter which is created by this module should be created in different way in this case. I will try to debug this when I find some time.

@cowtowncoder
Copy link
Member

@ZioberMichal Appreciate your help here.

@zkiss what do you think would make sense here?

@ZioberMichal
Copy link
Contributor

@cowtowncoder I made some other tests and I think that main problem is linked with the fact that DateMidnightSerializer is created with FormatConfig.DEFAULT_DATEONLY_FORMAT value. This in turn is created using DateTimeFormatter DEFAULT_JODA_DATEONLY_FORMAT = ISODateTimeFormat.date().withZoneUTC(). Please see below tests:
Test 1:

        DateMidnight date = new DateMidnight(2001, 1, 22);
        DateMidnight dateInUTC = new DateMidnight(2001, 1, 22, DateTimeZone.UTC);
        DateTimeFormatter DEFAULT_JODA_DATEONLY_FORMAT
                = ISODateTimeFormat.date().withZoneUTC();
        System.out.println(DEFAULT_JODA_DATEONLY_FORMAT.print(date));
        System.out.println(DEFAULT_JODA_DATEONLY_FORMAT.print(dateInUTC));

prints

2001-01-21
2001-01-22

Test 2:

        DateMidnight date = new DateMidnight(2001, 1, 22);
        DateMidnight dateInUTC = new DateMidnight(2001, 1, 22, DateTimeZone.UTC);
        DateTimeFormatter DEFAULT_JODA_DATEONLY_FORMAT
                = ISODateTimeFormat.date();
        System.out.println(DEFAULT_JODA_DATEONLY_FORMAT.print(date));
        System.out.println(DEFAULT_JODA_DATEONLY_FORMAT.print(dateInUTC));

prints

2001-01-22
2001-01-22

As you can see when formatter is created with withZoneUTC() it prints properly only these dates which were created in this timezone. Formatter with default timezone can properly print date midnights with default and set timezones.

@ZioberMichal
Copy link
Contributor

I have created small proposition how to fix this. I am not sure whether I have noticed all side effects of my changes but maybe this will help you to fix this in much better way.

@cowtowncoder cowtowncoder added this to the 2.6.0 milestone Jun 2, 2015
@cowtowncoder
Copy link
Member

Ok, let's see what happens. While I worry about compatibility, we also do want to get things improved, tests stable.

cowtowncoder added a commit that referenced this issue Jun 2, 2015
@zkiss
Copy link

zkiss commented Jun 2, 2015

@cowtowncoder @ZioberMichal sorry I've missed this, I have limited internet access at the moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants