server, ccl: adjust more tests to work with test tenant#107470
server, ccl: adjust more tests to work with test tenant#107470craig[bot] merged 4 commits intocockroachdb:masterfrom
Conversation
|
This change is |
af8058b to
6a79d39
Compare
knz
left a comment
There was a problem hiding this comment.
LGTM with a request for clarification.
Reviewed 3 of 3 files at r1, 9 of 9 files at r2, 22 of 22 files at r3, 13 of 13 files at r4, all commit messages.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @stevendanna and @yuzefovich)
-- commits line 32 at r4:
nit: you can mention that the long-term fix will be to move the tests to at _test package.
pkg/sql/sqlstats/test_utils.go line 68 at r2 (raw file):
// Note: SQL Stats’s read path uses follower read // // (AS OF SYSTEM TIME follower_read_timestamp()) to ensure that contention
nit: something is off with the comment formatting.
pkg/server/server_test.go line 778 at r2 (raw file):
respString := string(respBytes) // Since server test package links ccl, build.GetInfo() says that we
I'm confused - why is this true?
This particular file (server_test.go) has package server at the top. It's not server_test. How come this logic is necessary?
yuzefovich
left a comment
There was a problem hiding this comment.
TFTR!
bors r+
Reviewable status:
pkg/server/server_test.go line 778 at r2 (raw file):
Previously, knz (Raphael 'kena' Poss) wrote…
I'm confused - why is this true?
This particular file (server_test.go) has
package serverat the top. It's notserver_test. How come this logic is necessary?
Discussed this offline.
|
Build failed (retrying...): |
|
Any chance this could be making |
|
Yes, this PR is the culprit, thanks. bors r- |
|
Canceled. |
Release note: None
Now that `ccl` package is linked into server tests, this commit needed to adjust `TestServeIndexHTML` a little (even though that test is in `server` and not `server_test`). Additionally, this commit moves the creation of SQLStats testing knobs into `sqlstats` package as well as removes it from one place where it's unnecessary. Release note: None
These calls are redundant. Also adjust the tests a bit to work with the test tenant. Release note: None
This commit adjusts tests in these package to run with the test tenant. Note that the test tenant won't still run in `cli` because `ccl` isn't linked (I briefly tried, and I think there was an import cycle - the longer term fix is to move tests into `cli_test` package, but it's left for later). Release note: None
|
bors r+ |
|
Build succeeded: |
Informs: #76378.
Epic: CRDB-18499.