Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multitenant: increase tenant testing coverage of unit tests #138912

Closed
1 task done
shubhamdhama opened this issue Jan 13, 2025 · 2 comments
Closed
1 task done

multitenant: increase tenant testing coverage of unit tests #138912

shubhamdhama opened this issue Jan 13, 2025 · 2 comments
Labels
C-investigation Further steps needed to qualify. C-label will change. T-db-server target-release-25.2.0

Comments

@shubhamdhama
Copy link
Contributor

shubhamdhama commented Jan 13, 2025

The idea is to continue from where we left off in #76378

See the comments below for the current state of this issue,

Some sub-issues logged so far:

Jira issue: CRDB-46420

@shubhamdhama shubhamdhama added C-investigation Further steps needed to qualify. C-label will change. T-db-server labels Jan 13, 2025
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Jan 13, 2025
Since cockroachdb#75449 is fixed we can enable tenant testing here.

Informs: cockroachdb#138912
Epic: CRDB-38970
Release note: None
@shubhamdhama shubhamdhama self-assigned this Jan 13, 2025
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Jan 20, 2025
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Jan 21, 2025
Informs: cockroachdb#138912
Epic: CRDB-38970
Release note: None
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Jan 21, 2025
This change is mostly straightforward:
* Removed remaining usages of `TODOTestTenantDisabled`, enabling tenant
  testing for those tests.
* Made some improvements to a couple of tests.
* Created a new function (hopefully temporary)
  `TestSkippedForExternalModeDueToPerformance` to exclude external mode for
  `TestAdminAPIEvents` due to poor performance.

Informs: cockroachdb#138912
Epic: CRDB-38970
Release note: None
craig bot pushed a commit that referenced this issue Jan 21, 2025
139227: demo: remove `TODOTestTenantDisabled` usage r=cthumuluru-crdb a=shubhamdhama

Since the `demo` is controlling the tenants explicitly, `TestControlsTenantsExplicitly` makes more sense here.

Informs: #138912
Epic: CRDB-38970
Release note: None

Co-authored-by: Shubham Dhama <[email protected]>
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Jan 27, 2025
This change is part of our effort to eliminate the remaining usages of
`TODOTestTenantDisabled` from tests across various features. It removes
its occurrence in the backup tests.

Informs: cockroachdb#138912

Epic: CRDB-38970

Release note: None
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Feb 7, 2025
shubhamdhama added a commit to shubhamdhama/cockroach that referenced this issue Feb 13, 2025
Certain tests need to determine the deployment mode of the test
server—whether it's single-tenant, shared-process, or external-process.
While this can be queried via SQL, a dedicated helper method improves
usability.

This commit adds `DeploymentMode` to `ApplicationLayerInterface` to provide
a simpler way to retrieve this information.

Informs: cockroachdb#138912
Epic: CRDB-38970
Release note: None
craig bot pushed a commit that referenced this issue Feb 13, 2025
140662: testutils: add `DeploymentMode` method to `ApplicationLayerInterface` r=stevendanna,herkolategan a=shubhamdhama

Certain tests need to determine the deployment mode of the test
server—whether it's single-tenant, shared-process, or external-process.
While this can be queried via SQL, a dedicated helper method improves
usability.

This commit adds `DeploymentMode` to `ApplicationLayerInterface` to provide
a simpler way to retrieve this information.

Informs: #138912
Epic: CRDB-38970
Release note: None

Co-authored-by: Shubham Dhama <[email protected]>
@shubhamdhama shubhamdhama changed the title multitenant: review/remove usages of TODOTestTenantDisabled multitenant: increase tenant testing coverage of unit tests Feb 17, 2025
@shubhamdhama
Copy link
Contributor Author

Current status

Package Total Done Status Notes
pkg/backup 18 0
pkg/ccl/changefeedccl 10 0
pkg/ccl/importerccl 2 0
pkg/ccl/kvccl 2 0
pkg/ccl/multiregionccl 5 0
pkg/ccl/partitionccl 6 0
pkg/ccl/serverccl 1 0
pkg/ccl/workloadccl 2 0
pkg/cli/democluster 3 3
pkg/server 15 0
pkg/sql/importer 13 0
pkg/sql/schemachanger 1 0
pkg/sql #140446 177 114 ongoing Usages of createTestServerParams
Total 255 117

craig bot pushed a commit that referenced this issue Mar 14, 2025
139436: server: enable tenant testing for server/admin APIs r=cthumuluru-crdb a=shubhamdhama

`` server: copy `StubTimeNow` server testing knobs to tenants ``

`` server: enable tenant testing for server/admin APIs ``

This change is mostly straightforward:
* Removed remaining usages of `TODOTestTenantDisabled`, enabling tenant
  testing for those tests.
* Made some improvements to a couple of tests.
* Created a new function (hopefully temporary)
  `TestSkippedForExternalModeDueToPerformance` to exclude external mode for
  `TestAdminAPIEvents` due to poor performance.

Informs: #138912
Epic: CRDB-38970
Release note: None

Co-authored-by: Shubham Dhama <[email protected]>
@shubhamdhama shubhamdhama removed their assignment Mar 15, 2025
@rimadeodhar
Copy link
Collaborator

Closing as all the sub issues are tracked under different epics related to MT testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-investigation Further steps needed to qualify. C-label will change. T-db-server target-release-25.2.0
Projects
None yet
Development

No branches or pull requests

2 participants