Fix flaky event recording config-policy-controller E2E test #174

yiraeChristineKim · 2023-11-06T19:29:52Z

Ref: https://issues.redhat.com/browse/ACM-7938

dhaiducek · 2023-11-07T14:50:31Z

test/e2e/case15_event_format_test.go

+		Eventually(func() []v1.Event {
+			return utils.GetMatchingEvents(clientManaged, testNamespace,
+				case15AlwaysCompliantName, "", "Policy status is NonCompliant", defaultTimeoutSeconds)
+		}, defaultTimeoutSeconds, 1).Should(BeEmpty())


(Optional) Do we want this to be Consistently()? If not, I don't think it needs to be wrapped in Eventually() since it's checking whether it's empty.

yiraeChristineKim · 2023-11-09T14:47:55Z

test/e2e/case13_templatization_test.go

@@ -482,7 +482,7 @@ var _ = Describe("Test templatization", Ordered, func() {
 					case13PruneTmpErr+"-configmap", "default", true, defaultTimeoutSeconds)

 				return configmap
-			}, defaultTimeoutSeconds, 1).ShouldNot(BeNil())


consistently timeout

What do you mean?

I think this change is good - we don't need to check that the configmap is still there for 60 (or more) seconds, 30 should be fine. But I don't understand why it would be timing out right now?

It doesn't any issue but I tried to reduce e2e test time. I think 60 secs is meaningless. Just suggestion. It took over 20mins before but now less than 15 mins

test/e2e/case13_templatization_test.go

yiraeChristineKim · 2023-11-09T14:48:11Z

test/e2e/case20_delete_objects_test.go

@@ -219,7 +219,7 @@ var _ = Describe("Test Object deletion", Ordered, func() {
 					case20PodName, "default", true, defaultTimeoutSeconds)

 				return pod
-			}, defaultTimeoutSeconds, 1).Should(Not(BeNil()))
+			}, 30, 1).Should(Not(BeNil()))


consistently timeout

10-12 mins now

yiraeChristineKim · 2023-11-09T14:50:47Z

test/e2e/e2e_suite_test.go

@@ -106,7 +106,7 @@ var _ = BeforeSuite(func() {
 	clientManagedDynamic = NewKubeClientDynamic("", kubeconfigManaged, "")
 	defaultImageRegistry = "quay.io/open-cluster-management"
 	testNamespace = "managed"
-	defaultTimeoutSeconds = 60
+	defaultTimeoutSeconds = 100


Increase this timeout because minimum version is slow

@yiraeChristineKim is it only specific tests that fail without this increased default timeout?

Now 60 sec works fine

JustinKuli · 2023-11-10T15:04:09Z

test/e2e/case13_templatization_test.go

@@ -482,7 +482,7 @@ var _ = Describe("Test templatization", Ordered, func() {
 					case13PruneTmpErr+"-configmap", "default", true, defaultTimeoutSeconds)

 				return configmap
-			}, defaultTimeoutSeconds, 1).ShouldNot(BeNil())


What do you mean?

I think this change is good - we don't need to check that the configmap is still there for 60 (or more) seconds, 30 should be fine. But I don't understand why it would be timing out right now?

test/e2e/case20_delete_objects_test.go

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

yiraeChristineKim · 2023-11-13T17:08:23Z

test/e2e/e2e_suite_test.go

-	gvrPolicy             schema.GroupVersionResource
+	testNamespace               string
+	defaultTimeoutSeconds       int
+	defaultConsistentlyDuration int


I added this only

yiraeChristineKim · 2023-11-13T17:08:43Z

test/e2e/case5_multi_test.go

@@ -21,7 +21,7 @@ const (
 	case5ComboYaml               string = "../resources/case5_multi/case5_multi_combo.yaml"
 )

-var _ = Describe("Test multiple obj template handling", func() {
+var _ = Describe("Test multiple obj template handling", Ordered, func() {


This caused minimum version issue

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

openshift-ci · 2023-11-13T17:53:28Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: JustinKuli, 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:

~~OWNERS~~ [JustinKuli,yiraeChristineKim]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mprahl · 2023-11-14T19:03:31Z

test/e2e/case23_invalid_field_test.go

@@ -73,7 +73,7 @@ var _ = Describe("Test an objectDefinition with an invalid field", Ordered, func
 			}

 			return startTime.After(compPlcEvents[len(compPlcEvents)-1].LastTimestamp.Time)
-		}, defaultTimeoutSeconds, 1).Should(BeTrue())
+		}, 30, 1).Should(BeTrue())


@yiraeChristineKim did you forget to update this value and the other value in this file to use the new global variable?

I think I missed these 2 Thanks Let me open pr

openshift-ci bot added do-not-merge/work-in-progress dco-signoff: yes approved labels Nov 6, 2023

yiraeChristineKim force-pushed the ACM-7938 branch 2 times, most recently from f806ab7 to 2d6a84e Compare November 6, 2023 20:08

dhaiducek reviewed Nov 7, 2023

View reviewed changes

yiraeChristineKim force-pushed the ACM-7938 branch 4 times, most recently from e508299 to 32b3fa7 Compare November 9, 2023 14:07

yiraeChristineKim commented Nov 9, 2023

View reviewed changes

test/e2e/case13_templatization_test.go Show resolved Hide resolved

yiraeChristineKim commented Nov 9, 2023

View reviewed changes

yiraeChristineKim requested review from mprahl, JustinKuli and dhaiducek November 9, 2023 14:50

yiraeChristineKim commented Nov 9, 2023

View reviewed changes

yiraeChristineKim marked this pull request as ready for review November 9, 2023 18:38

openshift-ci bot removed the do-not-merge/work-in-progress label Nov 9, 2023

JustinKuli reviewed Nov 10, 2023

View reviewed changes

Fix flaky event recording config-policy-controller E2E test

1de000e

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

yiraeChristineKim force-pushed the ACM-7938 branch 2 times, most recently from 38ddc71 to 17a00f0 Compare November 13, 2023 16:26

yiraeChristineKim commented Nov 13, 2023

View reviewed changes

yiraeChristineKim requested a review from JustinKuli November 13, 2023 17:32

yiraeChristineKim force-pushed the ACM-7938 branch from 17a00f0 to 435a1b7 Compare November 13, 2023 17:42

Reduce consistently timeout

492f449

Signed-off-by: Yi Rae Kim <yikim@redhat.com>

yiraeChristineKim force-pushed the ACM-7938 branch from 435a1b7 to 492f449 Compare November 13, 2023 17:42

JustinKuli approved these changes Nov 13, 2023

View reviewed changes

openshift-ci bot assigned JustinKuli Nov 13, 2023

openshift-ci bot added the lgtm label Nov 13, 2023

openshift-merge-bot bot merged commit 30d1181 into open-cluster-management-io:main Nov 13, 2023

magic-mirror-bot bot mentioned this pull request Nov 13, 2023

🤖 Sync from open-cluster-management-io/config-policy-controller: #174 stolostron/config-policy-controller#635

Merged

mprahl reviewed Nov 14, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix flaky event recording config-policy-controller E2E test #174

Fix flaky event recording config-policy-controller E2E test #174

yiraeChristineKim commented Nov 6, 2023 •

edited

Loading

dhaiducek Nov 7, 2023

yiraeChristineKim Nov 9, 2023

JustinKuli Nov 10, 2023

yiraeChristineKim Nov 10, 2023

yiraeChristineKim Nov 9, 2023

yiraeChristineKim Nov 13, 2023

yiraeChristineKim Nov 9, 2023

mprahl Nov 10, 2023

yiraeChristineKim Nov 13, 2023

JustinKuli Nov 10, 2023

yiraeChristineKim Nov 13, 2023

yiraeChristineKim Nov 13, 2023

openshift-ci bot commented Nov 13, 2023

mprahl Nov 14, 2023

yiraeChristineKim Nov 14, 2023

Fix flaky event recording config-policy-controller E2E test #174

Fix flaky event recording config-policy-controller E2E test #174

Conversation

yiraeChristineKim commented Nov 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openshift-ci bot commented Nov 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yiraeChristineKim commented Nov 6, 2023 •

edited

Loading