[GitHub] [camel-k] tadayosi commented on a diff in pull request #3716: e2e: test fixes and bug fixes from test runs

[GitBox <gi...@apache.org>]

tadayosi commented on code in PR #3716:
URL: https://github.com/apache/camel-k/pull/3716#discussion_r988553887


##########
e2e/namespace/install/cli/run_test.go:
##########
@@ -36,117 +37,161 @@ import (
 	v1 "github.com/apache/camel-k/pkg/apis/camel/v1"
 )
 
-func TestKamelCLIRun(t *testing.T) {
+var sampleJar = "https://raw.githubusercontent.com/apache/camel-k/main/e2e/global/common/traits/files/jvm/sample-1.0.jar"
+
+func operatorID(ns string) string {
+	return fmt.Sprintf("camel-k-%s", ns)
+}
+
+func installWithID(ns string) {
+	Expect(KamelInstallWithID(operatorID(ns), ns).Execute()).To(Succeed())
+	Eventually(OperatorPod(ns)).ShouldNot(BeNil())
+	Eventually(PlatformPhase(ns), TestTimeoutLong).Should(Equal(v1.IntegrationPlatformPhaseReady))
+}
+
+func TestKamelCLIRunGitHubExampleJava(t *testing.T) {

Review Comment:
   I am not convinced yet. From the point of view of test independence, it is obviously good to have separate test functions, but this creates a namespace each time, so there are time efficiency problems such as the unavailability of downloaded jar caches and built kits. And we are already suffering from the long execution time of E2E tests. As a result of that trade-off, we tried to keep the tests together in E2E as much as possible (refer to the discussion #3298 if you are not aware of it). It is not consistent with the rest if we separate this test only here.
   
   > In the same way as TestRunConfig, the single test with run functions requires the clearing of the namespace at the end of each run - avoiding test contamination.
   
   In the light of the above discussion, I don't see it particularly as a problem.
   
   If we had to separate the tests to solve the instability issue, then that would be a clear reason to separate them, but as far as I remember, this test hasn't been particularly unstable except this new one "Run with http dependency". Why can't we just fix the problematic test instead?  Or has the `TestKamelCLIRun()` test been flaky on OCP?



-- 
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