[v15] Fix flaky opsgenie recipients test#45271
Conversation
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
EdwardDowling
added
the
no-changelog
zmb3
changed the title
|
I don't see how this changes the test, can you clarify? |
| require.ElementsMatch(t, tt.expectedRecipients, recipients) | ||
|
|
||
| got := make(map[string]string, len(recipients)) | ||
| for _, r := range recipients { | ||
| got[r.Name] = r.ID | ||
| } | ||
| want := make(map[string]string, len(tt.expectedRecipients)) | ||
| for _, r := range tt.expectedRecipients { | ||
| want[r.Name] = r.ID | ||
| } | ||
|
|
||
| require.Equal(t, want, got) |
There was a problem hiding this comment.
If this fixes the issue, it's probably because we were getting duplicates in recipients.
We should look into why we get them, instead of ignoring them by using a set.
There was a problem hiding this comment.
The issue wasn't the duplicated recipients, it was that the test assumed an order to the return values. Added the results to a map just to compare without order mattering.
There was a problem hiding this comment.
According to the docs, require.ElementsMatch does not care about the order:
https://pkg.go.dev/github.com/stretchr/testify/require#ElementsMatch
ElementsMatch asserts that the specified listA(array, slice...) is equal to specified listB(array, slice...) ignoring the order of the elements. If there are duplicate elements, the number of appearances of each of them in both lists should match.
There was a problem hiding this comment.
Ah sorry, you are right.
I am confused then by the message in the ticket then
expected: []common.Recipient{common.Recipient{Name:"foo", ID:"foo", Kind:"", Data:interface {}(nil)}, common.Recipient{Name:"bar", ID:"bar", Kind:"", Data:interface {}(nil)}} actual : []common.Recipient{common.Recipient{Name:"bar", ID:"bar", Kind:"", Data:interface {}(nil)}, common.Recipient{Name:"foo", ID:"foo", Kind:"", Data:interface {}(nil)}}
As it looks like the only difference is the order.
There was a problem hiding this comment.
🤔
That error message comes from the fact that we were using require.Equals
However, it was fixed in this PR #40777
This PR was created 4 days after the flaky test was reported (in April).
I think we can close this PR, unless I'm missing something
3 participants
Fixes flaky test mentioned in #40653