testserver: do not verify enterprise license #108762
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
knz
requested review from
rachitgsrivastava and
DarrylWong
and removed request for
a team
|
This change is |
This was referenced Aug 15, 2023
stevendanna
reviewed
Aug 15, 2023
This comment was marked as outdated.
This comment was marked as outdated.
|
I have added the list to the issue #76378. |
|
Just wanted to point out that the actual number of tests that require fixing is larger than what the screenshot shows. This is because some of the failures result in crashes, so the whole package counts as a single failure (although there might be more lurking). |
|
yes, that is true, but at least the list of packages is complete (and it was longer than in the screenshot, the screenshot was taken when the CI run wasn't complete yet) |
knz
force-pushed
the
20230815-disable-enterprise-check
branch
from
b634f0a to
29c40a7
Compare
knz
force-pushed
the
20230815-disable-enterprise-check
branch
2 times, most recently
from
425822f to
f1edfee
Compare
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
This will make it possible to merge the change that enables tenant selection everywhere before we investigate the test failures in detail. Release note: None
Some of the tests are properly fixes, others have skips associated with them. Release note: None
Some packages contain many tests that fail when run with a virtual cluster active. Until we have time to investigate them in details, opt them out. Release note: None
knz
force-pushed
the
20230815-disable-enterprise-check
branch
2 times, most recently
from
f0576b3 to
d0512f9
Compare
Some packages contain many tests that fail when run with a virtual cluster active. Until we have time to investigate them in details, opt them out. Release note: None
Prior to this patch, for test tenant servers to be automatically used two conditions needed to hold: - the CCL package(s) needed to be imported. - an enterprise license needed to be set (usually via `ccl.TestingEnableEnterprise()`) While we want these conditions to hold to start separate-process SQL servers in production, they do not need to be verified in unit tests. NB: as a result of merging this PR, numerous (recently introduced) calls to `ccl.TestingEnableEnterprise()` in various `main_test.go` files are now unnecessary. Cleaning them up can be pushed to a later phase. Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
Release note: None
knz
force-pushed
the
20230815-disable-enterprise-check
branch
from
d0512f9 to
11be180
Compare
herkolategan
added a commit
to herkolategan/cockroach
that referenced
this pull request
Oct 26, 2023
Previously a CCL license was required for the default test tenant to be started. This requirement no longer holds true, hence the comment has been updated. See also: cockroachdb#108762 Epic: CRDB-31933 Release note: None
herkolategan
added a commit
to herkolategan/cockroach
that referenced
this pull request
Oct 31, 2023
Previously a CCL license was required for the default test tenant to be started. This requirement no longer holds true, hence the comment has been updated. See also: cockroachdb#108762 Epic: CRDB-31933 Release note: None
herkolategan
added a commit
to herkolategan/cockroach
that referenced
this pull request
Nov 2, 2023
Previously a CCL license was required for the default test tenant to be started. This requirement no longer holds true, hence the comment has been updated. See also: cockroachdb#108762 Epic: CRDB-31933 Release note: None
herkolategan
added a commit
to herkolategan/cockroach
that referenced
this pull request
Nov 2, 2023
Previously a CCL license was required for the default test tenant to be started. This requirement no longer holds true, hence the comment has been updated. See also: cockroachdb#108762 Epic: CRDB-31933 Release note: None
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prior to this patch, for test tenant servers to be automatically used
two conditions needed to hold:
ccl.TestingEnableEnterprise())While we want these conditions to hold to start separate-process SQL
servers in production, they do not need to be verified in unit tests.
NB: as a result of merging this PR, a lot of (recently introduced)
calls to
ccl.TestingEnableEnterprise()in variousmain_test.gofiles are now unnecessary. Cleaning them up can be pushed to a later
phase.
Release note: None
Epic: CRDB-18499
Fixes #104500.
Fixes #108761.
Informs #76378