armadaproject / armada

A multi-cluster batch queuing system for high-throughput workloads on Kubernetes.
https://armadaproject.io
Apache License 2.0
484 stars 135 forks source link

Test: timezone issue while running mage tests #3791

Open briheet opened 4 months ago

briheet commented 4 months ago

Describe the bug While checking out the issue with improving repo test coverage, i ran mage tests which gave the following results 2024-07-15-12:37:50-screenshot

groupjobs_test.go:1113: Error Trace: /home/briheet/Open/armada/internal/lookoutv2/repository/groupjobs_test.go:1113 /home/briheet/Open/armada/internal/lookoutv2/repository/groupjobs_test.go:28 /home/briheet/Open/armada/internal/common/database/db_testutil.go:59 /home/briheet/Open/armada/internal/common/database/lookout/util.go:15 /home/briheet/Open/armada/internal/lookoutv2/repository/groupjobs_test.go:24 /home/briheet/Open/armada/internal/lookoutv2/repository/groupjobs_test.go:1011 Error: Not equal: expected: []*model.JobGroup{(*model.JobGroup)(0xc00025fc00), (*model.JobGroup)(0xc00025fd40), (*model.JobGroup)(0xc00025fe00), (*model.JobGroup)(0xc00025ff40)} actual : []*model.JobGroup{(*model.JobGroup)(0xc000692020), (*model.JobGroup)(0xc000692040), (*model.JobGroup)(0xc000692140), (*model.JobGroup)(0xc0006921c0)}

                    Diff:
                    --- Expected
                    +++ Actual
                    @@ -3,3 +3,3 @@
                      Aggregates: (map[string]interface {}) (len=2) {
                    -   (string) (len=18) "lastTransitionTime": (string) (len=25) "2022-03-01T21:24:05+05:30",
                    +   (string) (len=18) "lastTransitionTime": (string) (len=20) "2022-03-01T15:54:05Z",
                       (string) (len=9) "submitted": (string) (len=20) "2022-03-01T15:24:05Z"
                    @@ -11,3 +11,3 @@
                      Aggregates: (map[string]interface {}) (len=2) {
                    -   (string) (len=18) "lastTransitionTime": (string) (len=25) "2022-03-01T20:44:05+05:30",
                    +   (string) (len=18) "lastTransitionTime": (string) (len=20) "2022-03-01T15:14:05Z",
                       (string) (len=9) "submitted": (string) (len=20) "2022-03-01T15:05:05Z"
                    @@ -19,3 +19,3 @@
                      Aggregates: (map[string]interface {}) (len=2) {
                    -   (string) (len=18) "lastTransitionTime": (string) (len=25) "2022-03-01T20:39:05+05:30",
                    +   (string) (len=18) "lastTransitionTime": (string) (len=20) "2022-03-01T15:09:05Z",
                       (string) (len=9) "submitted": (string) (len=20) "2022-03-01T15:07:05Z"
                    @@ -27,3 +27,3 @@
                      Aggregates: (map[string]interface {}) (len=2) {
                    -   (string) (len=18) "lastTransitionTime": (string) (len=25) "2022-03-01T20:34:05+05:30",
                    +   (string) (len=18) "lastTransitionTime": (string) (len=20) "2022-03-01T15:04:05Z",
                       (string) (len=9) "submitted": (string) (len=20) "2022-03-01T15:04:05Z"
        Test:           TestGroupByAnnotationWithFiltersAndAggregates

DONE 2034 tests, 3 failures in 151.295s Error: exit status 1

Files Or Markdown that can reproduce the issue mage tests

Expected behavior It should clear all the tests.

Imporvement Adding a function which converts to UTC and formats would help us. If this is ok, i could push a small pr :)

dejanzele commented 1 month ago

Hey @briheet,

Is this issue still relevant?

Did you make the tests pass on your end?

Sovietaced commented 1 month ago

I see the same issue when running tests locally.

dejanzele commented 1 month ago

We had a workaround around this which can be solved outside of changing code.

@richscott I think you had an approach for this, mind sharing it?

richscott commented 1 month ago

The workaround I used was to override the host’s time zone by doing $ env TZ=UTC mage test

richscott commented 3 weeks ago

A better, more permanent fix for this would be to change the test in internal/lookoutv2/repository/groupjobs_test.go so that the expected time.Time valuel is "cast" to UTC / GMT, maybe by using the .UTC() method on that variable. It looks like the "actual" value in the test is already always UTC, by the presence of the "Z" timezone at the end. Once this change is done, that 'TZ=UTC' workaround should not be needed, no matter what timezone the host system is in during the test.