serverutils: improve the initialization of TestServer#107808
serverutils: improve the initialization of TestServer#107808craig[bot] merged 6 commits intocockroachdb:masterfrom
Conversation
|
This change is |
yuzefovich
left a comment
There was a problem hiding this comment.
Nice find!
Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @healthy-pod and @knz)
-- commits line 11 at r1:
nit: s/second/third/.
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: s/second/third/.
Fixed.
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/testutils/lint/lint_test.go line 595 at r5 (raw file):
"grep", "-nE", `, _, _ := serverutils\.StartServer\(`,
nit: perhaps make colon optional, i.e. , _, _ :?= serverutils\.StartServer\(.
19634af to
dff547e
Compare
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/testutils/lint/lint_test.go line 595 at r5 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
nit: perhaps make colon optional, i.e.
, _, _ :?= serverutils\.StartServer\(.
Done.
ee85a2f to
7ea9228
Compare
Release note: None
|
TFYRs! bors r=stevendanna,yuzefovich |
|
Thank you for updating your pull request. Before a member of our team reviews your PR, I have some potential action items for you:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
Canceled. |
|
bors r=stevendanna,yuzefovich |
|
bors r- There's a merge conflict with #107757 which is in the bors queue already. |
|
Canceled. |
Release note: None
The primary purpose of this patch is to tease out a `StartServerOnly()` function that doesn't return a `*gosql.DB` and `*kv.DB`, to improve readability in those tests that don't need them. In doing so however, this patch also fixes two bugs: - the advertised behavior of `StartServer()` was to return `.ApplicationLayer().DB()` in the third return value. However, the code was effectively returning `.SystemLayer().DB()` instead. This patch fixes it. - in case a binary version override was set in the parameters, the expectation is to upgrade both the storage layer and the application layer to the specified version. The code was only upgrading the application layer. This patch fixes it. Release note: None
In those tests that don't need a client connection, this simplification removes the overhead of setting up the TLS certs on disk. Release note: None
Release note: None
Release note: None
|
bors r=stevendanna,yuzefovich |
|
Build succeeded: |
The primary purpose of this patch is to tease out a
StartServerOnly()function that doesn't return a*gosql.DBand*kv.DB, to improve readability in those tests that don't need them.In doing so however, this patch also fixes two bugs:
the advertised behavior of
StartServer()was to return.ApplicationLayer().DB()in the second return value. However, the code was effectively returning.SystemLayer().DB()instead. This patch fixes it.in case a binary version override was set in the parameters, the expectation is to upgrade both the storage layer and the application layer to the specified version. The code was only upgrading the application layer. This patch fixes it.
Release note: None
Epic: CRDB-28893