-
Notifications
You must be signed in to change notification settings - Fork 72
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
test: SANDBOX-624: TestForceMetricsSynchronization: EnsureMUR #971
test: SANDBOX-624: TestForceMetricsSynchronization: EnsureMUR #971
Conversation
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.
Looks good 👍 ! Thanks for improving this 🙏
JFYI - there is also this helper:
userSignup, err = hostAwait.WaitForUserSignup(t, userSignup.Name,
wait.UntilUserSignupHasCompliantUsername())
require.NoError(t, err)
which waits for the UserSignup to have the CompliantUsername field set, if that is what we are looking for.
I see the e2e job failing but it doesn't seem related to your change , maybe some other flakyness :)
|
Quality Gate passedIssues Measures |
@mfrancisc, thank you so much for the tip!! Can we wait for the CompliantUsername in the Execute function? |
Goinggg to take a look 🏎️ |
@mfrancisc, based on this search query, it only happened once for now. I will keep an eye on it. Thanks!! |
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.
Thanks for addressing the comment!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alexeykazakov, mfrancisc, rsoaresd The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
d039910
into
codeready-toolchain:master
Maybe there are some cases where we don't want to "wait" until the CompliantUserName is populated , like when we create multiple signups in parallel, thus worth looking deeper at the code for the usage of |
Description
Sporadically, we are hitting this flaky test:
As we can see on the logs, "the UserSignup multiple-signup-testuser-1 doesn't have CompliantUsername set", so at this time, it does not have the MUR, and that's why it failed.
When the test
TestForceMetricsSynchronization
fails, when it tries to get the metricsandbox_master_user_records
, the second UserSignup does not yet have the CompliantUsername set probably because it does not have enough time to generate it between the time that the UserSignup is created and the time the test access the metrics.Since we are not ensuring MUR in the function
CreateMultipleSignup
, we are not ensuring that the UserSignup has already generated the CompliantUsername (CompliantUsername is needed for the MUR) before continuing the test flow.For now, this is just a theory that I hope it will solve it :D
Issue ticket number and link
SANDBOX-624