You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "christophd (via GitHub)" <gi...@apache.org> on 2023/05/05 13:53:43 UTC

[GitHub] [camel-k] christophd opened a new pull request, #4330: fix(e2e): Improve max build limit E2E test

christophd opened a new pull request, #4330:
URL: https://github.com/apache/camel-k/pull/4330

   - Improve assertions on max parallel running builds
   - Make sure to properly dump test namespaces on failure


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez commented on a diff in pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4330:
URL: https://github.com/apache/camel-k/pull/4330#discussion_r1186168101


##########
e2e/builder/build_test.go:
##########
@@ -57,76 +58,86 @@ func TestKitMaxBuildLimit(t *testing.T) {
 		buildB := "integration-b"
 		buildC := "integration-c"
 
-		ns1 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns1)
-		defer DeleteNamespace(t, ns1)
-
-		pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl1.Spec)
-		pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl1); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		ns2 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns2)
-		defer DeleteNamespace(t, ns2)
-
-		pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl2.Spec)
-		pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl2); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=A",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=B",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=C",
-			},
-		}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
-
-		// verify that buildC is allowed to build as soon as buildA has finished
-		Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutShort).Should(Equal(v1.BuildPhaseRunning))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseBuildRunning))
-
-		// verify that all builds are successful
-		Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
+		WithNewTestNamespace(t, func(ns1 string) {
+			WithNewTestNamespace(t, func(ns2 string) {
+				pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl1.Spec)
+				pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl1); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl2.Spec)
+				pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl2); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=A",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=B",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=C",
+					},
+				}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
+
+				var notExceedsMaxBuildLimit = func(runningBuilds int) bool {
+					return runningBuilds <= 2
+				}
+
+				limit := 0
+				for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning {

Review Comment:
   For the sake of test this is not a problem as, in case it hangs indefinitely, there is a timeout on the whole suite of tests.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez commented on a diff in pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez commented on code in PR #4330:
URL: https://github.com/apache/camel-k/pull/4330#discussion_r1186139328


##########
e2e/builder/build_test.go:
##########
@@ -57,76 +58,86 @@ func TestKitMaxBuildLimit(t *testing.T) {
 		buildB := "integration-b"
 		buildC := "integration-c"
 
-		ns1 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns1)
-		defer DeleteNamespace(t, ns1)
-
-		pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl1.Spec)
-		pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl1); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		ns2 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns2)
-		defer DeleteNamespace(t, ns2)
-
-		pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl2.Spec)
-		pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl2); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=A",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=B",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=C",
-			},
-		}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
-
-		// verify that buildC is allowed to build as soon as buildA has finished
-		Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutShort).Should(Equal(v1.BuildPhaseRunning))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseBuildRunning))
-
-		// verify that all builds are successful
-		Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
+		WithNewTestNamespace(t, func(ns1 string) {
+			WithNewTestNamespace(t, func(ns2 string) {
+				pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl1.Spec)
+				pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl1); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl2.Spec)
+				pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl2); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=A",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=B",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=C",
+					},
+				}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
+
+				var notExceedsMaxBuildLimit = func(runningBuilds int) bool {
+					return runningBuilds <= 2
+				}
+
+				limit := 0
+				for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning {

Review Comment:
   I think you need to check all BuildPhases in order to constantly check while any build is still running. I think you can indefinetely loop while any build is running and verify that the number is always lower than the limit. I think this could be a better approach then the `Consistently` usage which seem more appropriate when you want to test something in a period of time (which in our case we do not have).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on a diff in pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #4330:
URL: https://github.com/apache/camel-k/pull/4330#discussion_r1186168775


##########
e2e/builder/build_test.go:
##########
@@ -57,76 +58,86 @@ func TestKitMaxBuildLimit(t *testing.T) {
 		buildB := "integration-b"
 		buildC := "integration-c"
 
-		ns1 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns1)
-		defer DeleteNamespace(t, ns1)
-
-		pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl1.Spec)
-		pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl1); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		ns2 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns2)
-		defer DeleteNamespace(t, ns2)
-
-		pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl2.Spec)
-		pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl2); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=A",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=B",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=C",
-			},
-		}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
-
-		// verify that buildC is allowed to build as soon as buildA has finished
-		Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutShort).Should(Equal(v1.BuildPhaseRunning))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseBuildRunning))
-
-		// verify that all builds are successful
-		Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
+		WithNewTestNamespace(t, func(ns1 string) {
+			WithNewTestNamespace(t, func(ns2 string) {
+				pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl1.Spec)
+				pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl1); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl2.Spec)
+				pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl2); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=A",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=B",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=C",
+					},
+				}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
+
+				var notExceedsMaxBuildLimit = func(runningBuilds int) bool {
+					return runningBuilds <= 2
+				}
+
+				limit := 0
+				for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning {

Review Comment:
   the good thing about consistently is that acts like a loop but it has a polling interval and a fixed duration so we do not end up in an indefinite loop. If you loop yourself you would need to break the loop at some point and you would need to implement a polling interval to not stress the server with so many API requests



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on a diff in pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #4330:
URL: https://github.com/apache/camel-k/pull/4330#discussion_r1186172248


##########
e2e/builder/build_test.go:
##########
@@ -57,76 +58,86 @@ func TestKitMaxBuildLimit(t *testing.T) {
 		buildB := "integration-b"
 		buildC := "integration-c"
 
-		ns1 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns1)
-		defer DeleteNamespace(t, ns1)
-
-		pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl1.Spec)
-		pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl1); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		ns2 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns2)
-		defer DeleteNamespace(t, ns2)
-
-		pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl2.Spec)
-		pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl2); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=A",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=B",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=C",
-			},
-		}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
-
-		// verify that buildC is allowed to build as soon as buildA has finished
-		Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutShort).Should(Equal(v1.BuildPhaseRunning))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseBuildRunning))
-
-		// verify that all builds are successful
-		Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
+		WithNewTestNamespace(t, func(ns1 string) {
+			WithNewTestNamespace(t, func(ns2 string) {
+				pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl1.Spec)
+				pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl1); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl2.Spec)
+				pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl2); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=A",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=B",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=C",
+					},
+				}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
+
+				var notExceedsMaxBuildLimit = func(runningBuilds int) bool {
+					return runningBuilds <= 2
+				}
+
+				limit := 0
+				for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning {

Review Comment:
   I doubt that error messages and failure analysis is clear when running into the test suite timeout. I'd avoid to use this option



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] christophd commented on a diff in pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "christophd (via GitHub)" <gi...@apache.org>.
christophd commented on code in PR #4330:
URL: https://github.com/apache/camel-k/pull/4330#discussion_r1186163659


##########
e2e/builder/build_test.go:
##########
@@ -57,76 +58,86 @@ func TestKitMaxBuildLimit(t *testing.T) {
 		buildB := "integration-b"
 		buildC := "integration-c"
 
-		ns1 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns1)
-		defer DeleteNamespace(t, ns1)
-
-		pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl1.Spec)
-		pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl1); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		ns2 := NewTestNamespace(false).GetName()
-		defer DumpNamespace(t, ns2)
-		defer DeleteNamespace(t, ns2)
-
-		pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
-		pl.Spec.DeepCopyInto(&pl2.Spec)
-		pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
-		pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
-		if err := TestClient().Create(TestContext, &pl2); err != nil {
-			t.Error(err)
-			t.FailNow()
-		}
-
-		Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
-
-		doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=A",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=B",
-			},
-		}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
-
-		doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
-			operatorID: fmt.Sprintf("camel-k-%s", ns),
-			dependencies: []string{
-				"camel:timer", "camel:log",
-			},
-			traits: []string{
-				"builder.properties=build-property=C",
-			},
-		}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
-
-		// verify that buildC is allowed to build as soon as buildA has finished
-		Eventually(BuildPhase(ns, buildA), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutShort).Should(Equal(v1.BuildPhaseRunning))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseBuildRunning))
-
-		// verify that all builds are successful
-		Eventually(BuildPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns1, buildB), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
-		Eventually(BuildPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.BuildPhaseSucceeded))
-		Eventually(KitPhase(ns2, buildC), TestTimeoutLong).Should(Equal(v1.IntegrationKitPhaseReady))
+		WithNewTestNamespace(t, func(ns1 string) {
+			WithNewTestNamespace(t, func(ns2 string) {
+				pl1 := v1.NewIntegrationPlatform(ns1, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl1.Spec)
+				pl1.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl1.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl1); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns1), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				pl2 := v1.NewIntegrationPlatform(ns2, fmt.Sprintf("camel-k-%s", ns))
+				pl.Spec.DeepCopyInto(&pl2.Spec)
+				pl2.Spec.Build.Maven.Settings = v1.ValueSource{}
+				pl2.SetOperatorID(fmt.Sprintf("camel-k-%s", ns))
+				if err := TestClient().Create(TestContext, &pl2); err != nil {
+					t.Error(err)
+					t.FailNow()
+				}
+
+				Eventually(PlatformPhase(ns2), TestTimeoutMedium).Should(Equal(v1.IntegrationPlatformPhaseReady))
+
+				doKitBuildInNamespace(buildA, ns, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=A",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildB, ns1, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=B",
+					},
+				}, v1.BuildPhaseRunning, v1.IntegrationKitPhaseBuildRunning)
+
+				doKitBuildInNamespace(buildC, ns2, TestTimeoutShort, kitOptions{
+					operatorID: fmt.Sprintf("camel-k-%s", ns),
+					dependencies: []string{
+						"camel:timer", "camel:log",
+					},
+					traits: []string{
+						"builder.properties=build-property=C",
+					},
+				}, v1.BuildPhaseScheduling, v1.IntegrationKitPhaseNone)
+
+				var notExceedsMaxBuildLimit = func(runningBuilds int) bool {
+					return runningBuilds <= 2
+				}
+
+				limit := 0
+				for limit < 5 && BuildPhase(ns, buildA)() == v1.BuildPhaseRunning {

Review Comment:
   Looping indefinitely on all build phases is dangerous as we have already had the situation where builds keep stuck in Running phase forever. This would mean that the test never ends and we have a hanging build. So at some point you always need to break the loop. 
   
   I'd prefer to have a defined end of the test and to make sure that the max build limit checks has been constantly verified as long as the 1st scheduled build is in running phase.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [camel-k] squakez merged pull request #4330: fix(e2e): Improve max build limit E2E test

Posted by "squakez (via GitHub)" <gi...@apache.org>.
squakez merged PR #4330:
URL: https://github.com/apache/camel-k/pull/4330


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org