Fix flaky propagator test (case19_rep_policy_placement_test) #222
Conversation
yiraeChristineKim
force-pushed
the
ACM-11256
branch
2 times, most recently
from
0937dbe to
d0eb2af
Compare
yiraeChristineKim
changed the title
|
Or I can reset Reconciler before each |
|
/hold for discussion |
| )[0] | ||
| afterTotal, err = strconv.Atoi(afterString) | ||
| Expect(err).ShouldNot(HaveOccurred()) | ||
| Expect(afterTotal - beforeTotal).Should(Equal(2)) |
There was a problem hiding this comment.
This is the one I was thinking could alternatively just be:
Expect(afterTotal - beforeTotal).Should(Equal(Or(2,3)))
There was a problem hiding this comment.
It should be 2. because managed1, managed2 added. This happens only "minimum"
| Expect(err).ShouldNot(HaveOccurred()) | ||
| Expect(afterTotal - beforeTotal).Should(Equal(2)) | ||
| }) | ||
| It("should reconcile twice when managed1 and managed2 are created by placementRule", func() { |
There was a problem hiding this comment.
I don't have a strong opinion, but if we keep the test should it just be this test node that gets removed?
There was a problem hiding this comment.
You mean just remove the test after line 166?
There was a problem hiding this comment.
As an alternative, yes, but @mprahl indicated the tests don't have a lot of value, which I'm fine with, so removing it is also fine with me.
Remove case 19 that is checking reconcile numbers because this test is flaky, and hard to reset the metrics, we already verified our reconcile work properly Signed-off-by: yiraeChristineKim <[email protected]>
yiraeChristineKim
force-pushed
the
ACM-11256
branch
from
d0eb2af to
f1ca87c
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mprahl, yiraeChristineKim 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 |
|
/unhold |
openshift-merge-bot
bot
merged commit bceafae
into
open-cluster-management-io:main
No description provided.