Hibernate ORM and Spring Data tests failing with out-by-one error when run on Mac in North American timezone

holly-cummins commented 2 years ago

Describe the bug

I can’t decide if this is a test issue, or an implementation issue, but I’m raising a defect to track the investigation (and track the disabling of the tests on Mac CIs in https://github.com/quarkusio/quarkus/pull/27156, which we will want to reverse once this is sorted out).

A number of tests which check author.dob are failing with an out-by-one error, on our Mac GitHub runner. I assume the issue is a timezone one, since taking five hours off a date (as in UTC-5) can have the effect of going back a day. I have confirmed that I can reproduce on my Mac if I set the timezone to Chicago.

2022-09-17T06:31:57.7517920Z java.lang.AssertionError: 
2022-09-17T06:31:57.7518080Z 1 expectation failed.
2022-09-17T06:31:57.7527070Z JSON path author.dob doesn't match.
2022-09-17T06:31:57.7527270Z Expected: is "1821-11-11"
2022-09-17T06:31:57.7527420Z   Actual: 1821-11-10 ```

  I can’t explain why this only affects the Mac runner, and not the other US-hosted runners. (I’ve checked, and the timestamps in the log are the same as the ubuntu runners which pass fine.)      These are the affected tests:

📦 integration-tests/hibernate-orm-rest-data-panache ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldGetAuthor line 51 - ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldGetBookHal line 82 - ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldGetBook line 70 - ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldListAuthors line 97 - ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldListBooksHal line 134 - ✖ io.quarkus.it.hibernate.orm.rest.data.panache.HibernateOrmRestDataPanacheTest.shouldListBooks line 109 -

📦 integration-tests/jackson ✖ io.quarkus.it.jackson.DateDeserializerPojoResourceTest.testSqlDate line 41 -

📦 integration-tests/spring-data-rest ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldGetAuthor line 43 - ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldGetBookHal line 67 - ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldGetBook line 55 - ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldListAuthors line 82 - ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldListBooksHal line 106 - ✖ io.quarkus.it.spring.data.rest.SpringDataRestTest.shouldListBooks line 94 -

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

To reproduce, I think you will need a Mac? But probably not specifically an M1?

Change the timezone to something West of Greenwich, like a North American or Brazilian time (hi, @gastaldi :) ).

Then

mvn verify -f integration-tests/hibernate-orm-rest-data-panache

will fail.

Output of `uname -a` or `ver`

Darwin hcummins-mac 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000 arm64

quarkus-bot[bot] commented 2 years ago

/cc @Sanne, @gastaldi, @geoand, @gsmet, @yrodiere

geoand commented 2 years ago

Although https://github.com/quarkusio/quarkus/pull/28044 does not really answer why this is happening on M1 test runners, it nonetheless does allow us to progress

holly-cummins commented 2 years ago

Super-quick fix! Can this work item also take responsibility for reversing the changes made in https://github.com/quarkusio/quarkus/pull/27156 to disable the tests, please? Otherwise it will be delaying https://github.com/quarkusio/quarkus/pull/27156 because 27156 will need to be revised to un-disable the tests and then do another CI run. (I think delaying getting M1 into the build was perhaps the opposite of the intention of the changes here.)

geoand commented 2 years ago

@holly-cummins done!

I rebased and force pushed to your branch so the PR is updated

holly-cummins commented 2 years ago

Oh, sorry, @geoand , I meant the opposite! I meant could we leave my branch alone to improve the chances of a green run on that branch, and use this work item to track re-enabling the tests, after the M1 changes have merged. The M1 PR is struggling to get in so I was hoping to minimise further changes on that branch so I don't have to keep waiting for full-build cycles.

Ah well! Hopefully tomorrow will be the day for the M1 PR (fingers crossed!)

geoand commented 2 years ago

Oh, sorry!

holly-cummins commented 2 years ago

We still see some of date-related failures on M1, so this should perhaps be re-opened. Fixes would need to be tested with https://github.com/quarkusio/quarkus/pull/28082:

📦 integration-tests/hibernate-orm-graphql-panache

✖ io.quarkus.it.hibertnate.orm.graphql.panache.HibernateOrmGraphQLPanacheTest.testEndpoint line 63 - [More details](https://github.com/quarkusio/quarkus/runs/8447795060#user-content-test-failure-io.quarkus.it.hibertnate.orm.graphql.panache.hibernateormgraphqlpanachetest-1) - [Source on GitHub](https://github.com/quarkusio/quarkus/blob/94fe4a724135efebb6130827f666e922838e8254/integration-tests/hibernate-orm-graphql-panache/src/test/java/io/quarkus/it/hibertnate/orm/graphql/panache/HibernateOrmGraphQLPanacheTest.java#L63)
📦 integration-tests/jackson

✖ io.quarkus.it.jackson.DateDeserializerPojoResourceTest.testSqlDate line 42 - [More details](https://github.com/quarkusio/quarkus/runs/8447795060#user-content-test-failure-io.quarkus.it.jackson.datedeserializerpojoresourcetest-1) - [Source on GitHub](https://github.com/quarkusio/quarkus/blob/94fe4a724135efebb6130827f666e922838e8254/integration-tests/jackson/src/test/java/io/quarkus/it/jackson/DateDeserializerPojoResourceTest.java#L42)

geoand commented 1 year ago

Is this still an issue?

holly-cummins commented 1 year ago

I suspect it is, and that we don't see it every day because the tests are disabled. I've rebased #28082 (which enables the tests and shows the problem) to force a new build so we can check.

geoand commented 1 year ago

Great, thanks

holly-cummins commented 1 year ago

Confirming this is still an issue. https://github.com/quarkusio/quarkus/runs/9674074039 shows the problem. It's been dealt with at the moment by disabling the tests, which isn't ideal for a number of reasons.

📦 integration-tests/hibernate-orm-graphql-panache

✖ io.quarkus.it.hibertnate.orm.graphql.panache.HibernateOrmGraphQLPanacheTest.testEndpoint line 63 - [More details](https://github.com/quarkusio/quarkus/runs/9674074039#user-content-test-failure-io.quarkus.it.hibertnate.orm.graphql.panache.hibernateormgraphqlpanachetest-1) - [Source on GitHub](https://github.com/quarkusio/quarkus/blob/43bba01371c1e02401c91c0833d9f69ab36a8743/integration-tests/hibernate-orm-graphql-panache/src/test/java/io/quarkus/it/hibertnate/orm/graphql/panache/HibernateOrmGraphQLPanacheTest.java#L63)
📦 integration-tests/jackson

✖ io.quarkus.it.jackson.DateDeserializerPojoResourceTest.testSqlDate line 42 - [More details](https://github.com/quarkusio/quarkus/runs/9674074039#user-content-test-failure-io.quarkus.it.jackson.datedeserializerpojoresourcetest-1) - [Source on GitHub](https://github.com/quarkusio/quarkus/blob/43bba01371c1e02401c91c0833d9f69ab36a8743/integration-tests/jackson/src/test/java/io/quarkus/it/jackson/DateDeserializerPojoResourceTest.java#L42)

To expose the problem:

Have a mac machine (not sure if M1 is needed, I assume not but haven't confirmed)
Accept https://github.com/quarkusio/quarkus/pull/28082, which re-enables the tests
Change the computer's timezone to something west of Greenwich, like California

holly-cummins commented 1 year ago

If it makes us feel any better, dates and times are hard. I mean, we knew that already, but I just saw this on the website of a major company in the finance industry:

Sanne commented 1 year ago

When I see such funny date blunders, one part of me grins, the other part wonders if it's "our" fault :)

quarkusio / quarkus