-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[chore] Try fixing flaky SharedInstance e2e test #10929
[chore] Try fixing flaky SharedInstance e2e test #10929
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10929 +/- ##
=======================================
Coverage 91.62% 91.62%
=======================================
Files 406 406
Lines 19046 19046
=======================================
Hits 17450 17450
Misses 1237 1237
Partials 359 359 ☔ View full report in Codecov by Sentry. |
@@ -102,31 +104,34 @@ func Test_ComponentStatusReporting_SharedInstance(t *testing.T) { | |||
err = s.Shutdown(context.Background()) | |||
require.NoError(t, err) | |||
|
|||
assert.Equal(t, 5, len(eventsReceived)) | |||
require.Equal(t, 2, len(eventsReceived)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the value changed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To simplify the test I added a check in the test extension's ComponentStatusChanged
function to only record events for the component the test cares about. This changed the number or recorded sources from 5 to 2.
} | ||
} | ||
t.Logf("events received: %v", eventStr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just confirming that the two log statements were left intentionally
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ya I added these so that in the future, if a flaky test comes back, it will be a little easier to debug.
Fixes #10927.
go test status_test.go --count 100
passed.