From d4a7f0b3e1b34e0bfafcdc0dbcc3c441695f5f7e Mon Sep 17 00:00:00 2001 From: Monis Khan Date: Thu, 26 Aug 2021 14:31:50 -0400 Subject: [PATCH] test/integration: mark certain tests as disruptive This prevents them from running with any other test, including other parallel tests. Signed-off-by: Monis Khan --- .../concierge_api_serving_certs_test.go | 3 +- test/integration/main_test.go | 41 +++++++++++++++---- test/integration/supervisor_discovery_test.go | 9 ++-- 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/test/integration/concierge_api_serving_certs_test.go b/test/integration/concierge_api_serving_certs_test.go index 86267a39..67d06296 100644 --- a/test/integration/concierge_api_serving_certs_test.go +++ b/test/integration/concierge_api_serving_certs_test.go @@ -17,7 +17,8 @@ import ( "go.pinniped.dev/test/testlib" ) -func TestAPIServingCertificateAutoCreationAndRotation(t *testing.T) { +// Never run this test in parallel since breaking discovery is disruptive, see main_test.go. +func TestAPIServingCertificateAutoCreationAndRotation_Disruptive(t *testing.T) { env := testlib.IntegrationEnv(t) defaultServingCertResourceName := env.ConciergeAppName + "-api-tls-serving-certificate" diff --git a/test/integration/main_test.go b/test/integration/main_test.go index b3b670ae..6147da4c 100644 --- a/test/integration/main_test.go +++ b/test/integration/main_test.go @@ -29,7 +29,7 @@ func splitIntegrationTestsIntoBuckets(m *testing.M) { return } - var serialTests, parallelTests, finalTests []testing.InternalTest + var serialTests, parallelTests, disruptiveTests, finalTests []testing.InternalTest for _, test := range tests { test := test @@ -40,9 +40,17 @@ func splitIntegrationTestsIntoBuckets(m *testing.M) { // top level tests that want the standard Go behavior of only running // parallel tests with other parallel tests should use the regular // t.Parallel() approach. this has no effect on any subtest. - if strings.HasSuffix(test.Name, "_Parallel") { + switch { + case strings.HasSuffix(test.Name, "_Parallel"): parallelTests = append(parallelTests, test) - } else { + + // top level integration tests the end with the string _Disruptive + // are indicating that they are never safe to run with any other + // test because they break the underlying cluster in some way. + case strings.HasSuffix(test.Name, "_Disruptive"): + disruptiveTests = append(disruptiveTests, test) + + default: serialTests = append(serialTests, test) } } @@ -51,7 +59,7 @@ func splitIntegrationTestsIntoBuckets(m *testing.M) { Name: "TestIntegrationSerial", F: func(t *testing.T) { _ = testlib.IntegrationEnv(t) // make sure these tests do not run during unit tests - t.Parallel() // outer test runs in parallel always + t.Parallel() // outer test always runs in parallel for this bucket for _, test := range serialTests { test := test @@ -66,7 +74,7 @@ func splitIntegrationTestsIntoBuckets(m *testing.M) { Name: "TestIntegrationParallel", F: func(t *testing.T) { _ = testlib.IntegrationEnv(t) // make sure these tests do not run during unit tests - t.Parallel() // outer test runs in parallel always + t.Parallel() // outer test always runs in parallel for this bucket for _, test := range parallelTests { test := test @@ -79,13 +87,32 @@ func splitIntegrationTestsIntoBuckets(m *testing.M) { }, } - if len(serialTests) > 0 { - finalTests = append(finalTests, serialTest) + disruptiveTest := testing.InternalTest{ + Name: "TestIntegrationDisruptive", + F: func(t *testing.T) { + _ = testlib.IntegrationEnv(t) // make sure these tests do not run during unit tests + // outer test never runs in parallel for this bucket + + for _, test := range disruptiveTests { + test := test + t.Run(test.Name, func(t *testing.T) { + test.F(t) // inner disruptive tests do not run in parallel + }) + } + }, } if len(parallelTests) > 0 { finalTests = append(finalTests, parallelTest) } + if len(serialTests) > 0 { + finalTests = append(finalTests, serialTest) + } + + if len(disruptiveTests) > 0 { + finalTests = append(finalTests, disruptiveTest) + } + *testsPointer = finalTests } diff --git a/test/integration/supervisor_discovery_test.go b/test/integration/supervisor_discovery_test.go index 8bc48a5f..08f499bf 100644 --- a/test/integration/supervisor_discovery_test.go +++ b/test/integration/supervisor_discovery_test.go @@ -39,7 +39,8 @@ import ( // // Testing talking to the supervisor's port 8443 where the supervisor is terminating TLS itself is // handled by the others tests in this file. -func TestSupervisorOIDCDiscovery(t *testing.T) { +// Never run this test in parallel since deleting all federation domains is disruptive, see main_test.go. +func TestSupervisorOIDCDiscovery_Disruptive(t *testing.T) { env := testlib.IntegrationEnv(t) client := testlib.NewSupervisorClientset(t) @@ -143,7 +144,8 @@ func TestSupervisorOIDCDiscovery(t *testing.T) { } } -func TestSupervisorTLSTerminationWithSNI(t *testing.T) { +// Never run this test in parallel since deleting all federation domains is disruptive, see main_test.go. +func TestSupervisorTLSTerminationWithSNI_Disruptive(t *testing.T) { env := testlib.IntegrationEnv(t) pinnipedClient := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t) @@ -214,7 +216,8 @@ func TestSupervisorTLSTerminationWithSNI(t *testing.T) { }) } -func TestSupervisorTLSTerminationWithDefaultCerts(t *testing.T) { +// Never run this test in parallel since deleting all federation domains is disruptive, see main_test.go. +func TestSupervisorTLSTerminationWithDefaultCerts_Disruptive(t *testing.T) { env := testlib.IntegrationEnv(t) pinnipedClient := testlib.NewSupervisorClientset(t) kubeClient := testlib.NewKubernetesClientset(t)