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

#28472 fix flaky tests in libbeat fmtstr to use time.UTC instead of time.Local #28473

Conversation

hinchliff
Copy link
Contributor

@hinchliff hinchliff commented Oct 15, 2021

What does this PR do?

There are four unit tests that seem to be about re-formatting a timestamp in UTC. However, the unit tests generate the expected time using time.Local, which produces inconsistent results based on the local timezone used by e.g. the developer. If my understanding of the purpose of the test cases is correct, then using time.UTC should resolve any potential inconsistencies.

Why is it important?

Tests should work everywhere.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

  • is the PR author correct in their understanding of the purpose of the Tests being modified

How to test this PR locally

Ci/CD tests should pass as normal. You should be able to run the same tests locally on a box that does not use UTC, and have them pass.

Related issues

Use cases

  • N/A

Screenshots

  • N/A

Logs

See ticket #28472

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 15, 2021
@cla-checker-service
Copy link

cla-checker-service bot commented Oct 15, 2021

💚 CLA has been signed

@mergify
Copy link
Contributor

mergify bot commented Oct 15, 2021

This pull request does not have a backport label. Could you fix it @hinchliff? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v./d./d./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 15, 2021
@elasticmachine
Copy link
Collaborator

elasticmachine commented Oct 15, 2021

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Duration: 119 min 44 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

@hinchliff
Copy link
Contributor Author

I should be covered under my employer's CCLA. There is a CCLA with Optiv Security that includes me.

@gtback
Copy link
Member

gtback commented Oct 20, 2021

cla/check

@andresrc andresrc added the Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team label Oct 21, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 21, 2021
@kvch
Copy link
Contributor

kvch commented Oct 27, 2021

jenkins run tests

@kvch kvch added backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Nov 30, 2021
@kvch kvch merged commit db2903b into elastic:master Nov 30, 2021
mergify bot pushed a commit that referenced this pull request Nov 30, 2021
mergify bot pushed a commit that referenced this pull request Nov 30, 2021
@hinchliff hinchliff deleted the 28472-libbeat-fmtstr-test-use-time-utc-not-time-local branch November 30, 2021 17:45
elasticmachine pushed a commit to nxei/beats that referenced this pull request Dec 1, 2021
* upstream/master:
  [libbeat] Fix add_labels flattening of arrays values (elastic#29211)
  Change elastic-agent pprof default to false (elastic#29155)
  elastic#28472 fix flaky tests in libbeat fmtstr to use time.UTC instead of time.Local (elastic#28473)
  Adopt `parsers` in Filebeat's journald input (elastic#29070)
  [Elastic Agent] Add process error handling guidelines (elastic#29152)
  winlogbeat/sys/winevent: use reflect IsZero method (elastic#29190)
  Remove Journalbeat (elastic#29131)
  Add note that there is no warranty or support for generator code (elastic#28797)
  packetbeat: preparation for npcap addition (elastic#29017)
  Use the generic helper for opening file to read in filestream (elastic#29180)
  Workflow for macos (elastic#29174)
  Fix `decode_json_fields` processor to always add error key (elastic#29107)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v7.16.0 Automated backport with mergify backport-v8.0.0 Automated backport with mergify Team:Elastic-Agent-Data-Plane Label for the Agent Data Plane team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libbeat fmtstr tests fail outside of UTC
6 participants