Merge #75214

75214: roachpb: unskip TestReplicaUnavailableError r=irfansharif a=tbg This test needs to live in string_test.go for reasons that have to do with how we inject `keys.PrettyPrint` into `roachpb`. In the bazel build, this is only done for `strings_test.go` (for reasons I don't entirely understand, but I assume it is hard to override gazelle only partially, so we picked a file that would be dep'ed manually to contain the problem to this one file). Repurposing #75108 to track making this workaround more obvious. It took me a few hours to figure this all out, and it'll look similarly confusing to the next person. Release note: None Co-authored-by: Tobias Grieger <[email protected]>
cockroachdb · Jan 20, 2022 · 5359e54 · 5359e54
2 parents 96779a6 + 50af166
commit 5359e54
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 32 deletions.
diff --git a/pkg/kv/kvserver/replica_circuit_breaker_test.go b/pkg/kv/kvserver/replica_circuit_breaker_test.go
@@ -17,7 +17,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/redact"
@@ -26,7 +25,6 @@ import (
 
 func TestReplicaUnavailableError(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	skip.UnderBazelWithIssue(t, 75108, "flaky test")
 	defer log.Scope(t).Close(t)
 
 	var repls roachpb.ReplicaSet

diff --git a/pkg/roachpb/BUILD.bazel b/pkg/roachpb/BUILD.bazel
@@ -168,8 +168,6 @@ go_test(
         "//pkg/kv/kvserver/concurrency/lock",
         "//pkg/storage/enginepb",
         "//pkg/testutils/buildutil",
-        "//pkg/testutils/echotest",
-        "//pkg/testutils/skip",
         "//pkg/testutils/zerofields",
         "//pkg/util",
         "//pkg/util/bitarray",
@@ -205,14 +203,17 @@ go_test(
     name = "string_test",
     size = "small",
     srcs = ["string_test.go"],
+    data = glob(["testdata/**"]),
     deps = [
         ":with-mocks",
         "//pkg/cli/exit",
         "//pkg/keys",
         "//pkg/kv/kvserver/concurrency/lock",
         "//pkg/storage/enginepb",
+        "//pkg/testutils/echotest",
         "//pkg/util/hlc",
         "//pkg/util/uuid",
+        "@com_github_cockroachdb_errors//:errors",
         "@com_github_cockroachdb_redact//:redact",
         "@com_github_stretchr_testify//require",
     ],

diff --git a/pkg/roachpb/replica_unavailable_error_test.go b/pkg/roachpb/replica_unavailable_error_test.go
@@ -8,32 +8,9 @@
 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.
 
-package roachpb
+package roachpb_test
 
-import (
-	"context"
-	"fmt"
-	"path/filepath"
-	"testing"
-
-	"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
-	"github.com/cockroachdb/errors"
-	"github.com/cockroachdb/redact"
-)
-
-func TestReplicaUnavailableError(t *testing.T) {
-	ctx := context.Background()
-	skip.UnderBazelWithIssue(t, 75108, "flaky test")
-	var _ = (*ReplicaUnavailableError)(nil)
-	rDesc := ReplicaDescriptor{NodeID: 1, StoreID: 2, ReplicaID: 3}
-	var set ReplicaSet
-	set.AddReplica(rDesc)
-	desc := NewRangeDescriptor(123, RKeyMin, RKeyMax, set)
-
-	var err error = NewReplicaUnavailableError(desc, rDesc)
-	err = errors.DecodeError(ctx, errors.EncodeError(ctx, err))
-
-	s := fmt.Sprintf("%s\n%s", err, redact.Sprint(err))
-	echotest.Require(t, s, filepath.Join("testdata", "replica_unavailable_error.txt"))
-}
+// See TestReplicaUnavailableError in string_test.go; it needed to be moved there
+// for obscure reasons explained in:
+//
+// https://github.com/cockroachdb/cockroach/issues/75108
diff --git a/pkg/roachpb/string_test.go b/pkg/roachpb/string_test.go
@@ -11,15 +11,20 @@
 package roachpb_test
 
 import (
+	"context"
+	"fmt"
+	"path/filepath"
 	"testing"
 
 	// Hook up the pretty printer.
 	_ "github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/storage/enginepb"
+	"github.com/cockroachdb/cockroach/pkg/testutils/echotest"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/uuid"
+	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/redact"
 	"github.com/stretchr/testify/require"
 )
@@ -142,3 +147,18 @@ func TestSpansString(t *testing.T) {
 		require.Equal(t, tc.expected, tc.spans.String())
 	}
 }
+
+func TestReplicaUnavailableError(t *testing.T) {
+	ctx := context.Background()
+	var _ = (*roachpb.ReplicaUnavailableError)(nil)
+	rDesc := roachpb.ReplicaDescriptor{NodeID: 1, StoreID: 2, ReplicaID: 3}
+	var set roachpb.ReplicaSet
+	set.AddReplica(rDesc)
+	desc := roachpb.NewRangeDescriptor(123, roachpb.RKeyMin, roachpb.RKeyMax, set)
+
+	var err = roachpb.NewReplicaUnavailableError(desc, rDesc)
+	err = errors.DecodeError(ctx, errors.EncodeError(ctx, err))
+
+	s := fmt.Sprintf("%s\n%s", err, redact.Sprint(err))
+	echotest.Require(t, s, filepath.Join("testdata", "replica_unavailable_error.txt"))
+}