sql: remove calls to CreateTestServerParams #107765
Conversation
|
This change is |
yuzefovich
force-pushed
the
tt-helper
branch
7 times, most recently
from
28eed02
to
a2e46c2
Compare
yuzefovich
requested review from
rytaft,
Santamaura,
smg260 and
renatolabs
and removed request for
a team
yuzefovich
requested review from
knz and
stevendanna
and removed request for
rytaft,
Santamaura,
smg260 and
renatolabs
There was a problem hiding this comment.
LGTM with nits.
Reviewed 5 of 5 files at r1, 72 of 72 files at r2, 8 of 8 files at r3, 45 of 46 files at r4, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @stevendanna and @yuzefovich)
pkg/sql/schemachanger/scbuild/builder_test.go line 296 at r2 (raw file):
systemDB := s.SystemLayer().SQLConn(t, "") _, err := systemDB.Exec(`ALTER TENANT ALL SET CLUSTER SETTING "spanconfig.tenant_limit" = 10000000`)
May I suggest a setting override? The ALTER TENANT ALL propagates async with a rangefeed.
pkg/sql/tests/random_schema_test.go line 46 at r2 (raw file):
systemDB := s.SystemLayer().SQLConn(t, "") _, err := systemDB.Exec(`ALTER TENANT ALL SET CLUSTER SETTING "spanconfig.tenant_limit" = 10000000`)
ditto
This commit refactors `TestValidateSystemSchemaAfterBootStrap` to be run against the test tenant. Given that the system schema is different for system and application tenants, a new `bootstrap_tenant` file is introduced. Release note: None
This commit removes all calls to `tests.CreateTestServerParams` outside of `sql` and `sql_test` tests. The following commit will move and unexport this method. Miscellaneous items: - enable ccl license in many touched packages - adjust some tests to work with test tenant - rename pkg/sql/pgrepl/pgrepl_test.go -> pkg/sql/pgrepl/main_test.go - unexport `AOSTClause` from `sqlstats.TestingKnobs` - sprinkle some log scopes. Release note: None
It doesn't seem that useful - it only sets SQLStats testing knob which I don't think any of the tests actually need. Release note: None
Now that `tests.CreateTestServerParams` is only used from within `sql` folder, this helper is now moved into the `sql` package. Also, in order to unexport the helper to prevent it from polluting other packages unnecessarily, this commit removes all callers of the helper that are in `sql` test package (as opposed to `sql_test`). Release note: None
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/sql/schemachanger/scbuild/builder_test.go line 296 at r2 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
May I suggest a setting override? The ALTER TENANT ALL propagates async with a rangefeed.
The setting is not exported, so I reverted back to the testing knob.
pkg/sql/tests/random_schema_test.go line 46 at r2 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
ditto
Done.
|
Build failed (retrying...): |
|
Build succeeded: |
This PR removes calls to
tests.CreateTestServerParamsoutside ofsql_testtests as well as does some miscellaneous cleanups along the way. The rationale for doing this is that vast majority of callers didn't actually need the setup performed by the helper but inherited the disabling of test tenant. It seems more prudent for each test to be explicit about the kind of testing knobs and settings it needs.Informs: #76378.
Epic: CRDB-18499.
Release note: None