betfair / cougar

Cougar is a framework for making building network exposed service interfaces easy.
http://betfair.github.io/cougar
Apache License 2.0
27 stars 18 forks source link

Tests failing #77

Closed andredasilvapinto closed 10 years ago

andredasilvapinto commented 10 years ago

I'm getting these test errors while building the root project:

TextEventLogHandlerTest.testConcreteNoFlush:70->CougarUtilTestCase.validateFileContents:88 expected:<...OGGABLE,1970-01-01 0[0:00:00.000,,1970-01-01 00]:00:00.000> but was:<...OGGABLE,1970-01-01 0[1:00:00.000,,1970-01-01 01]:00:00.000> TextEventLogHandlerTest.testConcrete:61->CougarUtilTestCase.validateFileContents:88 expected:<...OGGABLE,1970-01-01 0[0:00:00.000,,1970-01-01 00]:00:00.000,Y,N> but was:<...OGGABLE,1970-01-01 0[1:00:00.000,,1970-01-01 01]:00:00.000,Y,N> TextEventLogHandlerTest.testConcreteWithList:88->CougarUtilTestCase.validateFileContents:88 expected:<...oo,bar,[1970-01-01 0[0:00:00.000,1970-01-01 00:00:00.001,1970-01-01 00]:00:00.002]> but was:<...oo,bar,[1970-01-01 0[1:00:00.000,1970-01-01 01:00:00.001,1970-01-01 01]:00:00.002]>

I'm in GMT+1 timezone.

andredasilvapinto commented 10 years ago

Running the tests individually doesn't produce the error above. It only occurs when running them through Maven's build procedure.

andredasilvapinto commented 10 years ago

The problem is that CougarUtilTestCase is overriding the default TimeZone for each test but the EventLogRecord getDateFormatter() does lazy loading of the DateFormat object, so after the first call to that method the object that is returned comes from the cache. If during the first call the timezone was still other than UTC the retrieved object will be in a different timezone than the one specified in the test.

If you really want to always use UTC then you can do so by setting the SimpleDateFormat timezone manually like this:

    private DateFormat getDateFormatter() {
        if (dateFormatter.get() == null) {
            SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
            dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
            dateFormatter.set(dateFormat);
        }
        return dateFormatter.get();
    }

The other option would be to not enforce the test Timezone in this case (I don't think setting the JVM default Timezone in tests is the best approach for most cases, normally is enough to mock or force the timezone on the test objects only) and generate the expected string dynamically, so both of them would follow the default JVM one.

eswdd commented 10 years ago

Cougar is built with an assumption of UTC as the time zone. Appreciate this causes probs for the tests, would be more than happy to accept a pull request to fix them..

Alternatively pass -Duser.timezone=UTC when you build.

Simon

On 9 Jul 2014, at 18:06, André Pinto notifications@github.com wrote:

The problem is that CougarUtilTestCase is overriding the default TimeZone for each test but the EventLogRecord getDateFormatter() does lazy loading of the DateFormat object, so after the first call to that method the object that is returned comes from the cache. If during the first call the timezone was still other than UTC the retrieved object will be in a different timezone than the one specified in the test.

If you really want to always use UTC then you can do so by setting the SimpleDateFormat timezone manually like this:

private DateFormat getDateFormatter() {
    if (dateFormatter.get() == null) {
        SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
        dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
        dateFormatter.set(dateFormat);
    }
    return dateFormatter.get();
}

The other option would be to not enforce the test Timezone in this case, so both of them would follow the default JVM one.

— Reply to this email directly or view it on GitHub.

eswdd commented 10 years ago

Although I do find it odd that my system tz is BST and I never have a problem with the tests in this regard.

On 10 Jul 2014, at 06:39, Simon Matic Langford eswddd@gmail.com wrote:

Cougar is built with an assumption of UTC as the time zone. Appreciate this causes probs for the tests, would be more than happy to accept a pull request to fix them..

Alternatively pass -Duser.timezone=UTC when you build.

Simon

On 9 Jul 2014, at 18:06, André Pinto notifications@github.com wrote:

The problem is that CougarUtilTestCase is overriding the default TimeZone for each test but the EventLogRecord getDateFormatter() does lazy loading of the DateFormat object, so after the first call to that method the object that is returned comes from the cache. If during the first call the timezone was still other than UTC the retrieved object will be in a different timezone than the one specified in the test.

If you really want to always use UTC then you can do so by setting the SimpleDateFormat timezone manually like this:

private DateFormat getDateFormatter() {
    if (dateFormatter.get() == null) {
        SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS");
        dateFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
        dateFormatter.set(dateFormat);
    }
    return dateFormatter.get();
}

The other option would be to not enforce the test Timezone in this case, so both of them would follow the default JVM one.

— Reply to this email directly or view it on GitHub.