Skip to content

Commit ee4e53b

Browse files
craig[bot]shubhamdhama
craig[bot]
andcommitted
Merge #143115
143115: sql: remove `createTestServerParams` r=yuzefovich a=shubhamdhama The only remaining use of `createTestServerParams` was in `TestIndexMutationKVOps`, but that test is really something the SQL queries team better suited to handle. So, I swapped it out for `createTestServerParamsAllowTenants` and override `DefaultTestTenant` with an appropriate one (tracked in #143114). Now that's taken care of, `createTestServerParams` is fully gone—one less way for test authors to accidentally skip running tests with tenants. Next step: we should rename `createTestServerParamsAllowTenants` back to `createTestServerParams` to keep things tidy. Closes: #140446 Epic: CRDB-48564 Release note: none Co-authored-by: Shubham Dhama <[email protected]>
2 parents 4d166e0 + 3a789bf commit ee4e53b

File tree

2 files changed

+4
-14
lines changed

2 files changed

+4
-14
lines changed

pkg/sql/index_mutation_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"strings"
1414
"testing"
1515

16+
"github.com/cockroachdb/cockroach/pkg/base"
1617
"github.com/cockroachdb/cockroach/pkg/jobs"
1718
"github.com/cockroachdb/cockroach/pkg/sql"
1819
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -35,7 +36,8 @@ func TestIndexMutationKVOps(t *testing.T) {
3536
defer log.Scope(t).Close(t)
3637
ctx := context.Background()
3738

38-
params, _ := createTestServerParams()
39+
params, _ := createTestServerParamsAllowTenants()
40+
params.DefaultTestTenant = base.TestDoesNotWorkWithSecondaryTenantsButWeDontKnowWhyYet(143114)
3941
// Decrease the adopt loop interval so that retries happen quickly.
4042
params.Knobs.JobsTestingKnobs = jobs.NewTestingKnobsWithShortIntervals()
4143
params.Knobs.SQLEvalContext = &eval.TestingKnobs{

pkg/sql/server_params_test.go

+1-13
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
// enables some EndTxn sanity checking and installs a flexible
1818
// TestingEvalFilter.
1919
// TODO(andrei): this function is not used consistently by SQL tests.
20+
// TODO(ssd,shubham): Rename this to `createTestServerParams`
2021
func createTestServerParamsAllowTenants() (base.TestServerArgs, *tests.CommandFilters) {
2122
var cmdFilters tests.CommandFilters
2223
params := base.TestServerArgs{}
@@ -28,16 +29,3 @@ func createTestServerParamsAllowTenants() (base.TestServerArgs, *tests.CommandFi
2829
}
2930
return params, &cmdFilters
3031
}
31-
32-
// createTestServerParams creates a set of params suitable for SQL
33-
// tests with randomized tenant testing disabled. New tests should
34-
// prefer createTestServerParamsAllowTenants(). See
35-
// createTestServerParamsAllowTenants for additional details.
36-
//
37-
// TODO(ssd): Rename this and createTestServerParamsAllowTenants once
38-
// disabling tenant testing is less common than enabling it.
39-
func createTestServerParams() (base.TestServerArgs, *tests.CommandFilters) {
40-
params, cmdFilters := createTestServerParamsAllowTenants()
41-
params.DefaultTestTenant = base.TODOTestTenantDisabled
42-
return params, cmdFilters
43-
}

0 commit comments

Comments
 (0)