From ac342a2e4ea090c81ddedc7c7e150fbfe4b6a3ad Mon Sep 17 00:00:00 2001 From: Felipe Gasper Date: Mon, 25 Nov 2024 14:48:28 -0500 Subject: [PATCH] Make CheckRunner methods check to see if the verifier failed. This prevents the test from hanging if the verifier fails for an unforeseen reason. --- internal/verifier/change_stream_test.go | 6 ++--- internal/verifier/check_runner.go | 21 +++++++++++++---- internal/verifier/migration_verifier_test.go | 24 ++++++++++---------- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/internal/verifier/change_stream_test.go b/internal/verifier/change_stream_test.go index 71fc326f..741c1738 100644 --- a/internal/verifier/change_stream_test.go +++ b/internal/verifier/change_stream_test.go @@ -280,7 +280,7 @@ func (suite *IntegrationTestSuite) TestCursorKilledResilience() { verifierRunner := RunVerifierCheck(suite.Context(), suite.T(), verifier) // wait for generation 0 to end - verifierRunner.AwaitGenerationEnd() + suite.Require().NoError(verifierRunner.AwaitGenerationEnd()) const mvName = "Migration Verifier" @@ -374,7 +374,7 @@ func (suite *IntegrationTestSuite) testInsertsBeforeWritesOff(docsCount int) { verifierRunner := RunVerifierCheck(suite.Context(), suite.T(), verifier) // wait for generation 0 to end - verifierRunner.AwaitGenerationEnd() + suite.Require().NoError(verifierRunner.AwaitGenerationEnd()) docs := lo.RepeatBy(docsCount, func(_ int) bson.D { return bson.D{} }) _, err := coll.InsertMany( @@ -424,7 +424,7 @@ func (suite *IntegrationTestSuite) TestCreateForbidden() { verifierRunner := RunVerifierCheck(suite.Context(), suite.T(), verifier) // wait for generation 0 to end - verifierRunner.AwaitGenerationEnd() + suite.Require().NoError(verifierRunner.AwaitGenerationEnd()) db := suite.srcMongoClient.Database(suite.DBNameForTest()) coll := db.Collection("mycoll") diff --git a/internal/verifier/check_runner.go b/internal/verifier/check_runner.go index fe48fd26..ba86d071 100644 --- a/internal/verifier/check_runner.go +++ b/internal/verifier/check_runner.go @@ -3,6 +3,8 @@ package verifier import ( "context" "testing" + + "github.com/pkg/errors" ) type CheckRunner struct { @@ -30,14 +32,25 @@ func RunVerifierCheck(ctx context.Context, t *testing.T, verifier *Verifier) *Ch } // AwaitGenerationEnd blocks until the check’s current generation ends. -func (cr *CheckRunner) AwaitGenerationEnd() { - <-cr.generationDoneChan +func (cr *CheckRunner) AwaitGenerationEnd() error { + select { + case <-cr.generationDoneChan: + return nil + case err := <-cr.checkDoneChan: + return errors.Wrap(err, "verifier failed while test awaited generation completion") + } } // StartNextGeneration blocks until it can tell the check to start // the next generation. -func (cr *CheckRunner) StartNextGeneration() { - cr.doNextGenerationChan <- struct{}{} +func (cr *CheckRunner) StartNextGeneration() error { + select { + case cr.doNextGenerationChan <- struct{}{}: + return nil + case err := <-cr.checkDoneChan: + return errors.Wrap(err, "verifier failed while test waited to start next generation") + } + } // Await will await generations and start new ones until the check diff --git a/internal/verifier/migration_verifier_test.go b/internal/verifier/migration_verifier_test.go index 10317f76..2c52a481 100644 --- a/internal/verifier/migration_verifier_test.go +++ b/internal/verifier/migration_verifier_test.go @@ -1245,7 +1245,7 @@ func (suite *IntegrationTestSuite) TestMetadataMismatchAndPartitioning() { suite.Require().NoError(err) runner := RunVerifierCheck(ctx, suite.T(), verifier) - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.AwaitGenerationEnd()) cursor, err := verifier.verificationTaskCollection().Find( ctx, @@ -1263,8 +1263,8 @@ func (suite *IntegrationTestSuite) TestMetadataMismatchAndPartitioning() { suite.Require().Equal(verificationTaskVerifyCollection, tasks[1].Type) suite.Require().Equal(verificationTaskMetadataMismatch, tasks[1].Status) - runner.StartNextGeneration() - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.StartNextGeneration()) + suite.Require().NoError(runner.AwaitGenerationEnd()) cursor, err = verifier.verificationTaskCollection().Find( ctx, @@ -1310,8 +1310,8 @@ func (suite *IntegrationTestSuite) TestGenerationalRechecking() { suite.T().Logf("TotalTasks is 0 (generation=%d); waiting %s then will run another generation …", verifier.generation, delay) time.Sleep(delay) - runner.StartNextGeneration() - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.StartNextGeneration()) + suite.Require().NoError(runner.AwaitGenerationEnd()) status, err = verifier.GetVerificationStatus() suite.Require().NoError(err) } @@ -1319,7 +1319,7 @@ func (suite *IntegrationTestSuite) TestGenerationalRechecking() { } // wait for one generation to finish - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.AwaitGenerationEnd()) status := waitForTasks() suite.Require().Equal(VerificationStatus{TotalTasks: 2, FailedTasks: 1, CompletedTasks: 1}, *status) @@ -1328,10 +1328,10 @@ func (suite *IntegrationTestSuite) TestGenerationalRechecking() { suite.Require().NoError(err) // tell check to start the next generation - runner.StartNextGeneration() + suite.Require().NoError(runner.StartNextGeneration()) // wait for generation to finish - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.AwaitGenerationEnd()) status = waitForTasks() // there should be no failures now, since they are are equivalent at this point in time suite.Require().Equal(VerificationStatus{TotalTasks: 1, CompletedTasks: 1}, *status) @@ -1341,10 +1341,10 @@ func (suite *IntegrationTestSuite) TestGenerationalRechecking() { suite.Require().NoError(err) // tell check to start the next generation - runner.StartNextGeneration() + suite.Require().NoError(runner.StartNextGeneration()) // wait for one generation to finish - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.AwaitGenerationEnd()) status = waitForTasks() // there should be a failure from the src insert @@ -1355,10 +1355,10 @@ func (suite *IntegrationTestSuite) TestGenerationalRechecking() { suite.Require().NoError(err) // continue - runner.StartNextGeneration() + suite.Require().NoError(runner.StartNextGeneration()) // wait for it to finish again, this should be a clean run - runner.AwaitGenerationEnd() + suite.Require().NoError(runner.AwaitGenerationEnd()) status = waitForTasks() // there should be no failures now, since they are are equivalent at this point in time