engine: s/TestMVCCRead/TestMVCCGet/ and test Go version of MVCCGet

These tests were missed by e35ea07, probably because of their name. Release note: None
nvanbenschoten · Sep 10, 2019 · ee4de67 · ee4de67
1 parent 179f29b
commit ee4de67
Show file tree

Hide file tree

Showing 2 changed files with 95 additions and 65 deletions.
diff --git a/pkg/storage/engine/mvcc.go b/pkg/storage/engine/mvcc.go
@@ -869,7 +869,7 @@ func mvccGetInternal(
 		// by this transaction or not an intent at all (so there's no
 		// conflict). Note that when reading the own intent, the timestamp
 		// specified is irrelevant; we always want to see the intent (see
-		// TestMVCCReadWithPushedTimestamp).
+		// TestMVCCGetWithPushedTimestamp).
 		seekKey.Timestamp = metaTimestamp
 
 		// Check for case where we're reading our own txn's intent

diff --git a/pkg/storage/engine/mvcc_test.go b/pkg/storage/engine/mvcc_test.go
@@ -3066,64 +3066,82 @@ func TestMVCCWriteWithDiffTimestampsAndEpochs(t *testing.T) {
 	}
 }
 
-// TestMVCCReadWithDiffEpochs writes a value first using epoch 1, then
+// TestMVCCGetWithDiffEpochs writes a value first using epoch 1, then
 // reads using epoch 2 to verify that values written during different
 // transaction epochs are not visible.
-func TestMVCCReadWithDiffEpochs(t *testing.T) {
+func TestMVCCGetWithDiffEpochs(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	engine := createTestEngine()
-	defer engine.Close()
 
-	// Write initial value without a txn.
-	if err := MVCCPut(context.Background(), engine, nil, testKey1, hlc.Timestamp{Logical: 1}, value1, nil); err != nil {
-		t.Fatal(err)
-	}
-	// Now write using txn1, epoch 1.
-	if err := MVCCPut(context.Background(), engine, nil, testKey1, hlc.Timestamp{WallTime: 1}, value2, txn1); err != nil {
-		t.Fatal(err)
-	}
-	// Try reading using different txns & epochs.
-	testCases := []struct {
-		txn      *roachpb.Transaction
-		expValue *roachpb.Value
-		expErr   bool
-	}{
-		// No transaction; should see error.
-		{nil, nil, true},
-		// Txn1, epoch 1; should see new value2.
-		{txn1, &value2, false},
-		// Txn1, epoch 2; should see original value1.
-		{txn1e2, &value1, false},
-		// Txn2; should see error.
-		{txn2, nil, true},
-	}
-	for i, test := range testCases {
-		value, _, err := MVCCGet(context.Background(), engine, testKey1, hlc.Timestamp{WallTime: 2}, true, test.txn)
-		if test.expErr {
-			if err == nil {
-				t.Errorf("test %d: unexpected success", i)
-			} else if _, ok := err.(*roachpb.WriteIntentError); !ok {
-				t.Errorf("test %d: expected write intent error; got %v", i, err)
+	for _, impl := range mvccGetImpls {
+		t.Run(impl.name, func(t *testing.T) {
+			mvccGet := impl.fn
+
+			ctx := context.Background()
+			engine := createTestEngine()
+			defer engine.Close()
+
+			// Write initial value without a txn.
+			if err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{Logical: 1}, value1, nil); err != nil {
+				t.Fatal(err)
 			}
-		} else if err != nil || value == nil || !bytes.Equal(test.expValue.RawBytes, value.RawBytes) {
-			t.Errorf("test %d: expected value %q, err nil; got %+v, %v", i, test.expValue.RawBytes, value, err)
-		}
+			// Now write using txn1, epoch 1.
+			if err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{WallTime: 1}, value2, txn1); err != nil {
+				t.Fatal(err)
+			}
+			// Try reading using different txns & epochs.
+			testCases := []struct {
+				txn      *roachpb.Transaction
+				expValue *roachpb.Value
+				expErr   bool
+			}{
+				// No transaction; should see error.
+				{nil, nil, true},
+				// Txn1, epoch 1; should see new value2.
+				{txn1, &value2, false},
+				// Txn1, epoch 2; should see original value1.
+				{txn1e2, &value1, false},
+				// Txn2; should see error.
+				{txn2, nil, true},
+			}
+			for i, test := range testCases {
+				t.Run(strconv.Itoa(i), func(t *testing.T) {
+					value, _, err := mvccGet(ctx, engine, testKey1, hlc.Timestamp{WallTime: 2}, true, test.txn)
+					if test.expErr {
+						if err == nil {
+							t.Errorf("test %d: unexpected success", i)
+						} else if _, ok := err.(*roachpb.WriteIntentError); !ok {
+							t.Errorf("test %d: expected write intent error; got %v", i, err)
+						}
+					} else if err != nil || value == nil || !bytes.Equal(test.expValue.RawBytes, value.RawBytes) {
+						t.Errorf("test %d: expected value %q, err nil; got %+v, %v", i, test.expValue.RawBytes, value, err)
+					}
+				})
+			}
+		})
 	}
 }
 
-// TestMVCCReadWithOldEpoch writes a value first using epoch 2, then
+// TestMVCCGetWithOldEpoch writes a value first using epoch 2, then
 // reads using epoch 1 to verify that the read will fail.
-func TestMVCCReadWithOldEpoch(t *testing.T) {
+func TestMVCCGetWithOldEpoch(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	engine := createTestEngine()
-	defer engine.Close()
 
-	if err := MVCCPut(context.Background(), engine, nil, testKey1, hlc.Timestamp{WallTime: 1}, value2, txn1e2); err != nil {
-		t.Fatal(err)
-	}
-	_, _, err := MVCCGet(context.Background(), engine, testKey1, hlc.Timestamp{WallTime: 2}, true, txn1)
-	if err == nil {
-		t.Fatalf("unexpected success of get")
+	for _, impl := range mvccGetImpls {
+		t.Run(impl.name, func(t *testing.T) {
+			mvccGet := impl.fn
+
+			ctx := context.Background()
+			engine := createTestEngine()
+			defer engine.Close()
+
+			if err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{WallTime: 1}, value2, txn1e2); err != nil {
+				t.Fatal(err)
+			}
+			_, _, err := mvccGet(ctx, engine, testKey1, hlc.Timestamp{WallTime: 2}, true, txn1)
+			if err == nil {
+				t.Fatalf("unexpected success of get")
+			}
+		})
 	}
 }
 
@@ -3174,30 +3192,42 @@ func TestMVCCWriteWithSequenceAndBatchIndex(t *testing.T) {
 	}
 }
 
-// TestMVCCReadWithPushedTimestamp verifies that a read for a value
+// TestMVCCGetWithPushedTimestamp verifies that a read for a value
 // written by the transaction, but then subsequently pushed, can still
 // be read by the txn at the later timestamp, even if an earlier
 // timestamp is specified. This happens when a txn's intents are
 // resolved by other actors; the intents shouldn't become invisible
 // to pushed txn.
-func TestMVCCReadWithPushedTimestamp(t *testing.T) {
+func TestMVCCGetWithPushedTimestamp(t *testing.T) {
 	defer leaktest.AfterTest(t)()
-	engine := createTestEngine()
-	defer engine.Close()
 
-	// Start with epoch 1.
-	if err := MVCCPut(context.Background(), engine, nil, testKey1, hlc.Timestamp{Logical: 1}, value1, txn1); err != nil {
-		t.Fatal(err)
-	}
-	// Resolve the intent, pushing its timestamp forward.
-	txn := makeTxn(*txn1, hlc.Timestamp{WallTime: 1})
-	if err := MVCCResolveWriteIntent(context.Background(), engine, nil, roachpb.Intent{Span: roachpb.Span{Key: testKey1}, Status: txn.Status, Txn: txn.TxnMeta}); err != nil {
-		t.Fatal(err)
-	}
-	// Attempt to read using naive txn's previous timestamp.
-	value, _, err := MVCCGet(context.Background(), engine, testKey1, hlc.Timestamp{Logical: 1}, true, txn1)
-	if err != nil || value == nil || !bytes.Equal(value.RawBytes, value1.RawBytes) {
-		t.Errorf("expected value %q, err nil; got %+v, %v", value1.RawBytes, value, err)
+	for _, impl := range mvccGetImpls {
+		t.Run(impl.name, func(t *testing.T) {
+			mvccGet := impl.fn
+
+			ctx := context.Background()
+			engine := createTestEngine()
+			defer engine.Close()
+
+			// Start with epoch 1.
+			if err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{Logical: 1}, value1, txn1); err != nil {
+				t.Fatal(err)
+			}
+			// Resolve the intent, pushing its timestamp forward.
+			txn := makeTxn(*txn1, hlc.Timestamp{WallTime: 1})
+			if err := MVCCResolveWriteIntent(ctx, engine, nil, roachpb.Intent{
+				Span:   roachpb.Span{Key: testKey1},
+				Status: txn.Status,
+				Txn:    txn.TxnMeta,
+			}); err != nil {
+				t.Fatal(err)
+			}
+			// Attempt to read using naive txn's previous timestamp.
+			value, _, err := mvccGet(ctx, engine, testKey1, hlc.Timestamp{Logical: 1}, true, txn1)
+			if err != nil || value == nil || !bytes.Equal(value.RawBytes, value1.RawBytes) {
+				t.Errorf("expected value %q, err nil; got %+v, %v", value1.RawBytes, value, err)
+			}
+		})
 	}
 }