Merge cockroachdb#116645

116645: testutils: Fix `ExpectErrWithTimeout` function to correctly fail the test on mismatched error message r=miretskiy a=cty123 While working on something else, I noticed the `ExpectErrWithTimeout` function always pass even when the error message mismatches. After debugging, it turned out that the error is just returned from the function instead of being thrown to fail the test. So in the current code, `ExpectErrWithTimeout` never fails a test which can potentially cause serious problems if certain regression is not captured. Epic: None Release note: None Co-authored-by: cty123 <[email protected]>
mgartner · Dec 20, 2023 · 0af1fed · 0af1fed
2 parents bab4160 + 078617a
commit 0af1fed
Showing 1 changed file with 6 additions and 1 deletion.
diff --git a/pkg/testutils/sqlutils/sql_runner.go b/pkg/testutils/sqlutils/sql_runner.go
@@ -238,13 +238,18 @@ func (sr *SQLRunner) ExpectErrWithTimeout(
 	if d == 0 {
 		d = testutils.DefaultSucceedsSoonDuration
 	}
-	_ = timeutil.RunWithTimeout(context.Background(), "expect-err", d, func(ctx context.Context) error {
+	err := timeutil.RunWithTimeout(context.Background(), "expect-err", d, func(ctx context.Context) error {
 		_, err := sr.DB.ExecContext(ctx, query, args...)
 		if !testutils.IsError(err, errRE) {
 			return errors.Newf("expected error '%s', got: %s", errRE, pgerror.FullError(err))
 		}
 		return nil
 	})
+
+	// Fail the test on unexpected error message OR execution timeout
+	if err != nil {
+		t.Fatalf("failed assert error: %s", err)
+	}
 }
 
 // Query is a wrapper around gosql.Query that kills the test on error.