Merge #151746

151746: kvserver: change all instances of `forTesting` suffixes to `Testing` prefixes in `kvserver/replica.go` and related files r=dodeca12 a=dodeca12

Addresses TODO message by `@wenyihu6`  to do as titled.

Renaming functions to use the `Testing` prefix is more inline with the codebase standard as the vast majority of such related functions use the prefix instead of the suffix.

Changed all such suffixes in `kvserver/replica.go` and also resolved the changes across dependant test files.

Fixes: #144119
Release note: None

Co-authored-by: Swapneeth Gorantla <[email protected]>

Author: craig[bot]

pkg/ccl/changefeedccl/protected_timestamps_test.go

@@ -637,7 +637,7 @@ func TestPTSRecordProtectsTargetsAndSystemTables(t *testing.T) {
 			t,
 			spanconfigptsreader.TestingRefreshPTSState(ctx, ptsReader, asOf),
 		)
-		require.NoError(t, repl.ReadProtectedTimestampsForTesting(ctx))
+		require.NoError(t, repl.TestingReadProtectedTimestamps(ctx))
 	}
 	gcTestTableRange := func(tableName, databaseName string) {
 		row := sqlDB.QueryRow(t, fmt.Sprintf("SELECT range_id FROM [SHOW RANGES FROM TABLE %s.%s]", tableName, databaseName))

pkg/crosscluster/physical/stream_ingestion_manager_test.go

@@ -195,7 +195,7 @@ func TestRevertTenantToTimestampPTS(t *testing.T) {
 			t,
 			spanconfigptsreader.TestingRefreshPTSState(ctx, ptsReader, asOf),
 		)
-		require.NoError(t, repl.ReadProtectedTimestampsForTesting(ctx))
+		require.NoError(t, repl.TestingReadProtectedTimestamps(ctx))
 
 		t.Logf("enqueuing range %d for mvccGC", rangeID)
 		systemSQL.Exec(t, `SELECT crdb_internal.kv_enqueue_replica($1, 'mvccGC', true)`, rangeID)

pkg/kv/kvclient/kvcoord/dist_sender_circuit_breaker_test.go

@@ -103,7 +103,7 @@ func TestDistSenderReplicaStall(t *testing.T) {
 		// Deadlock n3.
 		repl3, err := tc.GetFirstStoreFromServer(t, 2).GetReplica(desc.RangeID)
 		require.NoError(t, err)
-		mu := repl3.GetMutexForTesting()
+		mu := repl3.TestingGetMutex()
 		mu.Lock()
 		defer mu.Unlock()
 		t.Log("deadlocked n3")
@@ -211,7 +211,7 @@ func TestDistSenderCircuitBreakerModes(t *testing.T) {
 				// Deadlock either liveness or the scratch range.
 				repl, err := tc.GetFirstStoreFromServer(t, 2).GetReplica(desc.RangeID)
 				require.NoError(t, err)
-				mu := repl.GetMutexForTesting()
+				mu := repl.TestingGetMutex()
 				mu.Lock()
 				defer mu.Unlock()
 				t.Logf("deadlocked range on n3 - %v", desc)

pkg/kv/kvclient/kvtenant/tenant_kv_test.go

@@ -70,7 +70,7 @@ func TestTenantRangeQPSStat(t *testing.T) {
 	require.NoError(t, err)
 	// NB: We call directly into the load tracking struct, in order to avoid
 	// flakes due to timing differences affecting the result
-	loadStats := repl.GetLoadStatsForTesting()
+	loadStats := repl.TestingGetLoadStats()
 	qpsBefore := loadStats.TestingGetSum(load.Queries)
 	for i := 0; i < 110; i++ {
 		r.Exec(t, `SELECT k FROM foo.qps_test`)

pkg/kv/kvclient/rangefeed/rangefeed_external_test.go

@@ -1009,7 +1009,7 @@ func TestUnrecoverableErrors(t *testing.T) {
 			if conf, err := repl.LoadSpanConfig(ctx); err != nil || conf.GCPolicy.IgnoreStrictEnforcement {
 				return errors.New("waiting for span config to apply")
 			}
-			require.NoError(t, repl.ReadProtectedTimestampsForTesting(ctx))
+			require.NoError(t, repl.TestingReadProtectedTimestamps(ctx))
 			return nil
 		})
 

pkg/kv/kvserver/client_mvcc_gc_test.go

@@ -73,7 +73,7 @@ func TestMVCCGCCorrectStats(t *testing.T) {
 	ms.ValBytes = 32 * (1 << 20) // 16mb
 	ms.GCBytesAge = 48 * (1 << 20) * 100 * int64(time.Hour.Seconds())
 
-	repl.SetMVCCStatsForTesting(&ms)
+	repl.TestingSetMVCCStats(&ms)
 	require.NoError(t, store.ManualMVCCGC(repl))
 
 	// Verify that the mvcc gc queue restored the stats.
@@ -201,7 +201,7 @@ SELECT count(*)
 		if len(cfg.GCPolicy.ProtectionPolicies) == 0 {
 			return errors.New("waiting for span config to apply")
 		}
-		require.NoError(t, repl.ReadProtectedTimestampsForTesting(ctx))
+		require.NoError(t, repl.TestingReadProtectedTimestamps(ctx))
 		return nil
 	})
 

pkg/kv/kvserver/client_replica_test.go

@@ -3133,15 +3133,15 @@ func TestLeaderlessWatcherErrorRefreshedOnUnavailabilityTransition(t *testing.T)
 	// The leaderlessWatcher starts off as available.
 	require.False(t, repl.LeaderlessWatcher.IsUnavailable())
 	// Let it know it's leaderless.
-	repl.RefreshLeaderlessWatcherUnavailableStateForTesting(ctx, raft.None, manual.Now(), st)
+	repl.TestingRefreshLeaderlessWatcherUnavailableState(ctx, raft.None, manual.Now(), st)
 	// Even though the replica is leaderless, enough time hasn't passed for it to
 	// be considered unavailable.
 	require.False(t, repl.LeaderlessWatcher.IsUnavailable())
 	// The error should be nil as we're not considered leaderless at this point.
 	require.NoError(t, repl.LeaderlessWatcher.Err())
 	// Let enough time pass.
 	manual.Increment(10 * time.Second.Nanoseconds())
-	repl.RefreshLeaderlessWatcherUnavailableStateForTesting(ctx, raft.None, manual.Now(), st)
+	repl.TestingRefreshLeaderlessWatcherUnavailableState(ctx, raft.None, manual.Now(), st)
 	// Now the replica is considered unavailable.
 	require.True(t, repl.LeaderlessWatcher.IsUnavailable())
 	require.Error(t, repl.LeaderlessWatcher.Err())
@@ -3151,14 +3151,14 @@ func TestLeaderlessWatcherErrorRefreshedOnUnavailabilityTransition(t *testing.T)
 
 	// Next up, let the replica know there's a leader. This should make it
 	// available again.
-	repl.RefreshLeaderlessWatcherUnavailableStateForTesting(ctx, 1, manual.Now(), st)
+	repl.TestingRefreshLeaderlessWatcherUnavailableState(ctx, 1, manual.Now(), st)
 	require.False(t, repl.LeaderlessWatcher.IsUnavailable())
 	// Change the range descriptor. Mark it leaderless and let enough time pass
 	// for it to be considered unavailable again.
 	tc.AddVotersOrFatal(t, key, tc.Targets(2)...)
-	repl.RefreshLeaderlessWatcherUnavailableStateForTesting(ctx, raft.None, manual.Now(), st)
+	repl.TestingRefreshLeaderlessWatcherUnavailableState(ctx, raft.None, manual.Now(), st)
 	manual.Increment(10 * time.Second.Nanoseconds())
-	repl.RefreshLeaderlessWatcherUnavailableStateForTesting(ctx, raft.None, manual.Now(), st)
+	repl.TestingRefreshLeaderlessWatcherUnavailableState(ctx, raft.None, manual.Now(), st)
 	// The replica should now be considered unavailable again.
 	require.True(t, repl.LeaderlessWatcher.IsUnavailable())
 	require.Error(t, repl.LeaderlessWatcher.Err())
@@ -4707,7 +4707,7 @@ func TestStrictGCEnforcement(t *testing.T) {
 					t,
 					spanconfigptsreader.TestingRefreshPTSState(ctx, ptsReader, l.Start.ToTimestamp().Next()),
 				)
-				require.NoError(t, r.ReadProtectedTimestampsForTesting(ctx))
+				require.NoError(t, r.TestingReadProtectedTimestamps(ctx))
 			}
 		}
 		refreshTo = func(t *testing.T, asOf hlc.Timestamp) {
@@ -4718,7 +4718,7 @@ func TestStrictGCEnforcement(t *testing.T) {
 					t,
 					spanconfigptsreader.TestingRefreshPTSState(ctx, ptsReader, asOf),
 				)
-				require.NoError(t, r.ReadProtectedTimestampsForTesting(ctx))
+				require.NoError(t, r.TestingReadProtectedTimestamps(ctx))
 			}
 		}
 		// waitForProtectionAndReadProtectedTimestamps waits until the
@@ -4735,7 +4735,7 @@ func TestStrictGCEnforcement(t *testing.T) {
 				ptutil.TestingWaitForProtectedTimestampToExistOnSpans(ctx, t, tc.Server(i),
 					ptsReader, protectionTimestamp,
 					[]roachpb.Span{span})
-				require.NoError(t, r.ReadProtectedTimestampsForTesting(ctx))
+				require.NoError(t, r.TestingReadProtectedTimestamps(ctx))
 			}
 		}
 		insqlDB = tc.Server(0).InternalDB().(isql.DB)

pkg/kv/kvserver/replica.go

@@ -1721,9 +1721,9 @@ func (r *Replica) GetMVCCStats() enginepb.MVCCStats {
 	return *r.shMu.state.Stats
 }
 
-// SetMVCCStatsForTesting updates the MVCC stats on the repl object only, it does
+// TestingSetMVCCStats updates the MVCC stats on the repl object only, it does
 // not affect the on disk state and is only safe to use for testing purposes.
-func (r *Replica) SetMVCCStatsForTesting(stats *enginepb.MVCCStats) {
+func (r *Replica) TestingSetMVCCStats(stats *enginepb.MVCCStats) {
 	r.raftMu.Lock()
 	defer r.raftMu.Unlock()
 	r.mu.Lock()
@@ -2749,9 +2749,9 @@ func (r *Replica) GetLeaseHistory() []roachpb.Lease {
 	return r.leaseHistory.get()
 }
 
-// EnableLeaseHistoryForTesting turns on the lease history for testing purposes.
+// TestingEnableLeaseHistory turns on the lease history for testing purposes.
 // Returns a function to return it to its original state that can be deferred.
-func EnableLeaseHistoryForTesting(maxEntries int) func() {
+func TestingEnableLeaseHistory(maxEntries int) func() {
 	originalValue := leaseHistoryMaxEntries
 	leaseHistoryMaxEntries = maxEntries
 	return func() {
@@ -2848,21 +2848,21 @@ func (r *Replica) measureNanosRunning(start time.Duration, f func(float64)) {
 	f(float64(dur))
 }
 
-// GetLoadStatsForTesting is for use only by tests to read the Replicas' load
+// TestingGetLoadStats is for use only by tests to read the Replicas' load
 // tracker state.
-func (r *Replica) GetLoadStatsForTesting() *load.ReplicaLoad {
+func (r *Replica) TestingGetLoadStats() *load.ReplicaLoad {
 	return r.loadStats
 }
 
-// HasOutstandingLearnerSnapshotInFlightForTesting is for use only by tests to
+// TestingHasOutstandingLearnerSnapshotInFlight is for use only by tests to
 // gather whether there are in-flight snapshots to learner replcas.
-func (r *Replica) HasOutstandingLearnerSnapshotInFlightForTesting() bool {
+func (r *Replica) TestingHasOutstandingLearnerSnapshotInFlight() bool {
 	return r.errOnOutstandingLearnerSnapshotInflight() != nil
 }
 
-// ReadProtectedTimestampsForTesting is for use only by tests to read and update
+// TestingReadProtectedTimestamps is for use only by tests to read and update
 // the Replicas' cached protected timestamp state.
-func (r *Replica) ReadProtectedTimestampsForTesting(ctx context.Context) (err error) {
+func (r *Replica) TestingReadProtectedTimestamps(ctx context.Context) (err error) {
 	var ts cachedProtectedTimestampState
 	defer r.maybeUpdateCachedProtectedTS(&ts)
 	r.mu.RLock()
@@ -2871,29 +2871,26 @@ func (r *Replica) ReadProtectedTimestampsForTesting(ctx context.Context) (err er
 	return err
 }
 
-// GetMutexForTesting returns the replica's mutex, for use in tests.
-func (r *Replica) GetMutexForTesting() *ReplicaMutex {
+// TestingGetMutex returns the replica's mutex, for use in tests.
+func (r *Replica) TestingGetMutex() *ReplicaMutex {
 	return &r.mu.ReplicaMutex
 }
 
-// TODO(wenyihu6): rename the *ForTesting functions to be Testing* (see
-// #144119 for more details).
-
-// SetCachedClosedTimestampPolicyForTesting sets the closed timestamp policy on r
+// TestingSetCachedClosedTimestampPolicy sets the closed timestamp policy on r
 // to be the given policy. It is a test-only helper method.
-func (r *Replica) SetCachedClosedTimestampPolicyForTesting(policy ctpb.RangeClosedTimestampPolicy) {
+func (r *Replica) TestingSetCachedClosedTimestampPolicy(policy ctpb.RangeClosedTimestampPolicy) {
 	r.cachedClosedTimestampPolicy.Store(&policy)
 }
 
-// GetCachedClosedTimestampPolicyForTesting returns the closed timestamp policy on r.
+// TestingGetCachedClosedTimestampPolicy returns the closed timestamp policy on r.
 // It is a test-only helper method.
-func (r *Replica) GetCachedClosedTimestampPolicyForTesting() ctpb.RangeClosedTimestampPolicy {
+func (r *Replica) TestingGetCachedClosedTimestampPolicy() ctpb.RangeClosedTimestampPolicy {
 	return *r.cachedClosedTimestampPolicy.Load()
 }
 
-// RefreshLeaderlessWatcherUnavailableStateForTesting refreshes the replica's
+// TestingRefreshLeaderlessWatcherUnavailableState refreshes the replica's
 // leaderlessWatcher's unavailable state. Intended for tests.
-func (r *Replica) RefreshLeaderlessWatcherUnavailableStateForTesting(
+func (r *Replica) TestingRefreshLeaderlessWatcherUnavailableState(
 	ctx context.Context, postTickLead raftpb.PeerID, nowPhysicalTime time.Time, st *cluster.Settings,
 ) {
 	r.mu.Lock()

pkg/kv/kvserver/replica_closedts_test.go

@@ -1112,7 +1112,7 @@ func TestClosedTimestampPolicyRefreshIntervalOnLeaseTransfers(t *testing.T) {
 	})
 
 	// Force repl2 policy to be LAG_BY_CLUSTER_SETTING.
-	repl2.SetCachedClosedTimestampPolicyForTesting(ctpb.LAG_BY_CLUSTER_SETTING)
+	repl2.TestingSetCachedClosedTimestampPolicy(ctpb.LAG_BY_CLUSTER_SETTING)
 	require.Equal(t, roachpb.LAG_BY_CLUSTER_SETTING, repl2.GetRangeInfo(ctx).ClosedTimestampPolicy)
 
 	// Ensure that transferring the lease to repl2 does trigger a lease refresh.
@@ -1227,7 +1227,7 @@ func TestRefreshPolicyWithVariousLatencies(t *testing.T) {
 			repl.RefreshPolicy(tc.latencies)
 
 			// Verify the policy is set correctly.
-			actualPolicy := repl.GetCachedClosedTimestampPolicyForTesting()
+			actualPolicy := repl.TestingGetCachedClosedTimestampPolicy()
 			require.Equal(t, tc.expectedPolicy, actualPolicy, "expected policy %v, got %v", tc.expectedPolicy, actualPolicy)
 		})
 	}

pkg/kv/kvserver/replica_learner_test.go

@@ -1741,7 +1741,7 @@ func TestLearnerOrJointConfigAdminRelocateRange(t *testing.T) {
 		desc := tc.LookupRangeOrFatal(t, scratchStartKey)
 		repl, err := tc.GetFirstStoreFromServer(t, 0).GetReplica(desc.RangeID)
 		require.NoError(t, err)
-		if repl.HasOutstandingLearnerSnapshotInFlightForTesting() {
+		if repl.TestingHasOutstandingLearnerSnapshotInFlight() {
 			return errors.Errorf("outstanding learner snapshot in flight %s", desc)
 		}
 		return nil