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

Test: timezone issue while running mage tests #3791

Open
briheet opened this issue Jul 15, 2024 · 5 comments
Open

Test: timezone issue while running mage tests #3791

briheet opened this issue Jul 15, 2024 · 5 comments

Comments

@briheet
Copy link

briheet commented Jul 15, 2024

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
Copy link
Member

Hey @briheet,

Is this issue still relevant?

Did you make the tests pass on your end?

@Sovietaced
Copy link
Contributor

I see the same issue when running tests locally.

@dejanzele
Copy link
Member

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
Copy link
Member

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

@richscott
Copy link
Member

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.

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