-
Notifications
You must be signed in to change notification settings - Fork 817
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
Fix for flaky TestSDKSetAnnotation #516
Fix for flaky TestSDKSetAnnotation #516
Conversation
Build Succeeded 👏 Build Id: b0b151ed-eb8e-4aaa-8b2a-5233be24aec7 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website will exist for the next 30 days: To install this version:
|
}) | ||
|
||
assert.Nil(t, err) | ||
assert.NotEmpty(t, gs.ObjectMeta.Annotations["stable.agones.dev/sdk-timestamp"]) | ||
|
||
assert.NotEmpty(t, gs.ObjectMeta.Annotations[annotation]) |
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.
I distinctly remember seeing failures where "sdk-timestamp" was set but version was not.
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.
Can you try:
go test -count=100 -timeout 90s agones.dev/agones/test/e2e -run TestSDKSetAnnotation
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.
That would be strange, as the version annotation is set via a mutation webhook.
Ran this 50 times (in batches to fit under the timeout - also added a defer to delete the gameserver), no errors 🤷♂️
(Nice trick with -count=100
didn't know that one, will steal it 👍 )
dad7c7d
to
9d2e284
Compare
Pretty sure the new version annotation on GameServers made this test flaky. This is a fix!
9d2e284
to
d30d3b4
Compare
Build Succeeded 👏 Build Id: 5ae0ad36-29fc-408e-93dc-777031c42e13 The following development artifacts have been built, and will exist for the next 30 days:
A preview of the website will exist for the next 30 days: To install this version:
|
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.
LGTM
Pretty sure the new version annotation on GameServers made this test flaky.
This is a fix!