-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
logictest: add test for mixed-version configs #114309
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @celiala and @RaduBerinde)
pkg/sql/logictest/logictestbase/logictestbase_test.go
line 19 at r1 (raw file):
) func TestLogicTestConfigs(t *testing.T) {
Do we maybe want to name this test something like TestLogicTestConfigsHasMixedVersionConfigs
? Or alternatively put this into a subtest with t.Run("verify there is a mixed-version config for each supported release", ...)
?
pkg/sql/logictest/logictestbase/logictestbase_test.go
line 24 at r1 (raw file):
found := false for _, c := range LogicTestConfigs { if c.BootstrapVersion == v {
It might be useful to also check that c.DisableUpgrade == true
.
This commit adds a test that verifies that for each supported previous release we have a logictest config that bootstraps the cluster at that version. Informs: cockroachdb#112629 Release note: None
e070cea
to
123a2dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTR!
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @andyyang890, @celiala, and @rafiss)
pkg/sql/logictest/logictestbase/logictestbase_test.go
line 19 at r1 (raw file):
Previously, andyyang890 (Andy Yang) wrote…
Do we maybe want to name this test something like
TestLogicTestConfigsHasMixedVersionConfigs
? Or alternatively put this into a subtest witht.Run("verify there is a mixed-version config for each supported release", ...)
?
Good point, renamed to TestLogicTestMixedVersionConfigs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @celiala and @rafiss)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's a great check to add, thanks
TFTRs! bors r+ |
Build succeeded: |
This commit adds a test that verifies that for each supported previous release we have a logictest config that bootstraps the cluster at that version.
Informs: #112629
Release note: None