jobsccl: remove some uses of TODOTestTenantDisabled, simplify test code#106928
jobsccl: remove some uses of TODOTestTenantDisabled, simplify test code#106928craig[bot] merged 5 commits intocockroachdb:masterfrom
Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewed 2 of 2 files at r1, 1 of 1 files at r2, 4 of 4 files at r3, 6 of 6 files at r4, 1 of 1 files at r5, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @ericharmeling, @herkolategan, @knz, @rhu713, @srosenberg, and @stevendanna)
pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go line 43 at r3 (raw file):
// TestSystemConfigWatcher is a test which exercises the end-to-end integration // of the systemconfigwatcher. It exists in this subpackage so that it can be // run to exercise secondary tenants, which are ccl-only.
Should we refactor this testing helper to be a proper test itself, which enables the ccl features and runs both system and secondary tenants?
pkg/testutils/serverutils/test_tenant_shim.go line 117 at r4 (raw file):
// InternalExecutor returns a *sql.InternalExecutor as an interface{} (which // also implements insql.InternalExecutor if the test cannot depend on sql).
nit: s/insql/isql/.
pkg/server/admin_test.go line 145 at r4 (raw file):
s, _, _ := serverutils.StartServer(t, base.TestServerArgs{ // Disable the default test tenant for now as this tests fails with // it enabled. Tracked with #81590.
Seems like the last commit closes #81590.
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/systemconfigwatcher/systemconfigwatchertest/test_system_config_watcher.go line 43 at r3 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Should we refactor this testing helper to be a proper test itself, which enables the ccl features and runs both system and secondary tenants?
Good idea, filed #107058 to track.
pkg/testutils/serverutils/test_tenant_shim.go line 117 at r4 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit:
s/insql/isql/.
Fixed.
pkg/server/admin_test.go line 145 at r4 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
Seems like the last commit closes #81590.
Sadly it does not -- this only covers the debug interface, but all the SQL introspection APIs (get db details etc) are not done yet.
|
TFYR bors r=yuzefovich |
|
Build failed: |
|
bors r=yuzefovich |
|
bors r- |
|
Canceled. |
Release note: None
|
bors r=yuzefovich |
New sub-packages: - `storage_api`: unit tests for API endpoints specific to the KV/storage layer. - `application_api`: unit tests for API endpoints valid for application servers, including for secondary tenants. - `privchecker`: SQL authorization interface for API handlers. (previously: `adminPrivilegeChecker`) - `authserver`: HTTP/RPC authentication code. (previously: `authenticationServer`) - `srverrors`: error objects suitable for return from API handlers. - `srvtestutils`: common helpers for test code. Release note: None
Release note: None
This also removes `TODOTestTenantDisabled` since the tests properly exercise secondary tenants already. Release note: None
|
Canceled. |
|
bors r=yuzefovich |
|
Timed out. |
See internal discussion: https://cockroachlabs.slack.com/archives/CJ0H8Q97C/p1689769837489049 Release note: None
|
bors r=yuzefovich |
|
Build succeeded: |
First commits from #107135.
Epic: CRDB-18499
Informs #76378.