roachtest: port import/mixed-versions to the new framework#113546
roachtest: port import/mixed-versions to the new framework#113546craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
c620278 to
df12260
Compare
|
Got a successful run from the gceworker, so it's RFAL. |
DrewKimball
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @renatolabs)
renatolabs
left a comment
There was a problem hiding this comment.
Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status:
pkg/cmd/roachtest/tests/mixed_version_import.go line 44 at r1 (raw file):
func runImportMixedVersions(ctx context.Context, t test.Test, c cluster.Cluster, warehouses int) { crdbNodes := c.All() c.Put(ctx, t.Cockroach(), "./cockroach", crdbNodes)
You can skip this step and use tpccImportCmdWithCockroachBinary(test.DefaultCockroachPath) down below to avoid uploading a new binary to the cluster.
pkg/cmd/roachtest/tests/mixed_version_import.go line 48 at r1 (raw file):
// NB: We rely on the testing framework to choose a random predecessor to // upgrade from. mvt := mixedversion.NewTest(ctx, t, t.L(), c, crdbNodes, mixedversion.AlwaysUseFixtures)
Is there a specific reason to always use fixtures? The test doesn't seem to rely on them.
pkg/cmd/roachtest/tests/mixed_version_import.go line 52 at r1 (raw file):
node := h.RandomNode(r, crdbNodes) l.Printf("dropping tpcc db if exists on node %d", node) _, err := h.Connect(node).ExecContext(ctx, "DROP DATABASE IF EXISTS tpcc CASCADE;")
We can replace the RandomNode and log call with h.Exec().
There are some minor differences: - we no longer install fixtures (which weren't used anyway) - we'll probably run the IMPORT multiple times throughout the test run. Release note: None
df12260 to
1e5f61e
Compare
yuzefovich
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/mixed_version_import.go line 44 at r1 (raw file):
Previously, renatolabs (Renato Costa) wrote…
You can skip this step and use
tpccImportCmdWithCockroachBinary(test.DefaultCockroachPath)down below to avoid uploading a new binary to the cluster.
Nice, thanks, done.
pkg/cmd/roachtest/tests/mixed_version_import.go line 48 at r1 (raw file):
Previously, renatolabs (Renato Costa) wrote…
Is there a specific reason to always use fixtures? The test doesn't seem to rely on them.
The original test used fixtures, so I kept that. I also don't see the reason for using them, removed.
pkg/cmd/roachtest/tests/mixed_version_import.go line 52 at r1 (raw file):
Previously, renatolabs (Renato Costa) wrote…
We can replace the
RandomNodeand log call withh.Exec().
Done.
renatolabs
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/cmd/roachtest/tests/mixed_version_import.go line 48 at r1 (raw file):
Previously, yuzefovich (Yahor Yuzefovich) wrote…
The original test used fixtures, so I kept that. I also don't see the reason for using them, removed.
Yeah, nothing wrong with using them; by default the test will now probabilistically run with them, which makes more sense for tests that don't explicitly rely on the contents of the fixtures.
|
Got a passing run on the gceworker with latest push too. TFTRs! bors r+ |
|
Build succeeded: |
|
blathers backport 23.1 |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 1e5f61e to blathers/backport-release-23.1-113546: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1 failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Fixes: #110529.
Release note: None