tracer TestWithHeaderTags: Clear header tags to work with -count=2 #2262
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This test currently fails when run with -count greater than 2 because it does not clear all the global state. Add a test to ensure there are no header tags set at the end. Clear all headers tags after each test. This should ensure other tests aren't accidentally depending on this. It will help get the tests to pass with -count=10, which can be useful for tracking down flaky tests.
BenchmarksBenchmark execution time: 2023-10-12 14:10:54 Comparing candidate commit 9695a11 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 40 metrics, 1 unstable metrics. |
felixge
approved these changes
Oct 12, 2023
ddtrace/tracer/option_test.go
Outdated
| t.Run("default-off-at-end", func(t *testing.T) { | ||
| defer globalconfig.ClearHeaderTags() | ||
| newConfig() | ||
| assert.Equal(t, 0, globalconfig.HeaderTagsLen()) |
There was a problem hiding this comment.
Wouldn't it be better to move this assertion into the main test rather than a subtest? This way it also runs when the go test -run flag is used.
There was a problem hiding this comment.
Great suggestion, thank you! Done.
ajgajg1134
approved these changes
Oct 12, 2023
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
This test currently fails when run with -count greater than 2 because it does not clear all the global state. Add a test to ensure there are no header tags set at the end. Clear all headers tags after each test.
This should ensure other tests aren't accidentally depending on this. It will help get the tests to pass with -count=10, which can be useful for tracking down flaky tests.
Motivation
I'm trying to track down race bugs, so I tried to run the tests with
-count=10, and they fail.Reviewer's Checklist
For Datadog employees:
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!