Skip to content

cli/image: make history test timezone-independent - #7296

Open
keeltrace wants to merge 2 commits into
docker:masterfrom
keeltrace:keeltrace/history-timezone-test
Open

cli/image: make history test timezone-independent#7296
keeltrace wants to merge 2 commits into
docker:masterfrom
keeltrace:keeltrace/history-timezone-test

Conversation

@keeltrace

Copy link
Copy Markdown

AI generated PR; managed by a human. Feel free to close without merging or make any and all requests and I will update the PR - keeltrace handler

What I changed

Make TestNewHistoryCommandSuccess set time.Local directly to UTC for the duration of the test and restore the previous location with t.Cleanup.

Why

The test previously used t.Setenv("TZ", "UTC"), but another test in the same package calls .Local() first. Once Go initializes time.Local, changing TZ no longer changes the already-cached location. On non-UTC hosts, the package test could therefore compare a local RFC3339 timestamp against the UTC golden file and fail.

Verification

  • Before the change, TZ=America/Los_Angeles ./scripts/with-go-mod.sh go test ./cli/command/image -count=1 fails in TestNewHistoryCommandSuccess/non-human.
  • After the change, the package passes under UTC, America/Los_Angeles, and Asia/Tokyo.
  • TZ=America/Los_Angeles ./scripts/with-go-mod.sh go test -race ./cli/command/image -count=1 passes.
  • ./scripts/with-go-mod.sh go test ./cli/... -count=1 passes.
  • ./scripts/with-go-mod.sh go test ./... -count=1 passes all normal packages; E2E packages require TEST_DOCKER_HOST and are unavailable on this host.
  • make -f docker.Makefile test-unit is unavailable on this host because Docker Buildx is not installed.

Signed-off-by: Daniel Steele <218190424+keeltrace@users.noreply.github.com>
Comment on lines -121 to -122
// Set to UTC timezone as timestamps in output are
// printed in the current timezone

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's worth keeping the comment (but can be moved to the new code).

It's unfortunate indeed that TZ is only read once for a process (with a sync.Once) so there's no clean way to override it for a test 🤔

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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

Successfully merging this pull request may close these issues.

3 participants