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

Add test for non-default time zones on Windows #897

Closed
wants to merge 2 commits into from

Conversation

jefferbrecht
Copy link
Member

Description

Add test for non-default time zones on Windows, which is currently broken. When fluent-bit is fixed upstream, merge the fix here and re-run the tests.

Related issues

b/218888265
fluent/fluent-bit#2941

How has this been tested?

Ran tests locally; will let the standard tests run in the PR.

Checklist:

  • Unit tests
    • Unit tests do not apply.
    • Unit tests have been added/modified and passed for this PR.
  • Integration tests
    • Integration tests do not apply.
    • Integration tests have been added/modified.
  • Documentation
    • This PR introduces no user visible changes.
    • This PR introduces user visible changes and the corresponding documentation change has been made.
  • Minor version bump
    • This PR introduces no new features.
    • This PR introduces new features, and there is a separate PR to bump the minor version since the last release already.
    • This PR bumps the version.

@jefferbrecht jefferbrecht force-pushed the jefferbrecht-windows-timezone branch from 36602e3 to 5fcd937 Compare October 6, 2022 21:07
@jefferbrecht jefferbrecht force-pushed the jefferbrecht-windows-timezone branch from 6e5c113 to 0fda09e Compare December 5, 2022 14:54
}
ctx, logger, vm := agents.CommonSetup(t, platform)
if _, err := gce.RunRemotely(ctx, logger.ToMainLog(), vm, "", `Set-TimeZone -Id "Eastern Standard Time"`); err != nil {
t.Fatal(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the new windows VM default time zone is just the same as Eastern Time. Does the VM created with zone us-east1 default to have Eastern Time?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the default time zone is already Eastern Time then this command is a no-op and the test should still fail as expected.

In my testing the VM time zone was always UTC regardless of which Cloud zone it was created in.

@franciscovalentecastro
Copy link
Contributor

Should we update also TestCustomLogFormat in this PR too ? Since it uses UTC to avoid hitting the timezone bug.

// When not using UTC timestamps, the parsing with "%Y-%m-%dT%H:%M:%S.%L%z" doesn't work
// correctly in windows (b/218888265).
line := fmt.Sprintf("<13>1 %s %s my_app_id - - - qqqqrrrr\n", time.Now().UTC().Format(time.RFC3339Nano), vm.Name)
if err := gce.UploadContent(ctx, logger, vm, strings.NewReader(line), logPath); err != nil {
t.Fatalf("error writing dummy log line: %v", err)
}

@jefferbrecht jefferbrecht force-pushed the jefferbrecht-windows-timezone branch from f69e68f to d078974 Compare December 6, 2022 14:32
@jefferbrecht jefferbrecht force-pushed the jefferbrecht-windows-timezone branch from d078974 to f81a91c Compare December 12, 2022 14:46
@jefferbrecht
Copy link
Member Author

Closing because this has been moved into #1078.

@jefferbrecht jefferbrecht deleted the jefferbrecht-windows-timezone branch February 17, 2023 15:34
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