Skip to content

backupccl: move datadriven infra to different package for IMPORT#85637

Closed
msbutler wants to merge 1 commit intocockroachdb:masterfrom
msbutler:butler-data-driver
Closed

backupccl: move datadriven infra to different package for IMPORT#85637
msbutler wants to merge 1 commit intocockroachdb:masterfrom
msbutler:butler-data-driver

Conversation

@msbutler
Copy link
Copy Markdown
Collaborator

@msbutler msbutler commented Aug 4, 2022

This refactor moves the backup ccl infra to a new package in order to develop
data driven IMPORT tests.

Release note: none

@msbutler msbutler self-assigned this Aug 4, 2022
@cockroach-teamcity
Copy link
Copy Markdown
Member

cockroach-teamcity commented Aug 4, 2022

This change is Reviewable

@adityamaru
Copy link
Copy Markdown
Contributor

adityamaru commented Aug 4, 2022

nit: should we carve out pkg/ccl/testutilsccl/bulk/datadriven.go instead of bulkdatadriven. This way in the future if we want to put non-DD stuff we have a place.

@msbutler
Copy link
Copy Markdown
Collaborator Author

msbutler commented Aug 5, 2022

nit: should we carve out pkg/ccl/testutilsccl/bulk/datadriven.go instead of bulkdatadriven

What about bulktestutils instead of bulk as the package name? I'd like the package name to be a bit more descriptive than just bulk

@msbutler msbutler force-pushed the butler-data-driver branch from 589ba87 to 1419f0b Compare August 5, 2022 14:37
@msbutler msbutler marked this pull request as ready for review August 5, 2022 14:37
@msbutler msbutler requested review from a team, adityamaru and rhu713 and removed request for rhu713 August 5, 2022 14:37
@msbutler msbutler force-pushed the butler-data-driver branch from 1419f0b to 145a235 Compare August 5, 2022 14:39
@adityamaru
Copy link
Copy Markdown
Contributor

adityamaru commented Aug 5, 2022

What about bulktestutils instead of bulk as the package name? I'd like the package name to be a bit more descriptive than just bulk

We're already in testutilsccl so seems a little redundant but I don't feel that strongly.

@msbutler msbutler force-pushed the butler-data-driver branch 2 times, most recently from 40a5887 to 22230c2 Compare August 5, 2022 16:55
@msbutler msbutler force-pushed the butler-data-driver branch 3 times, most recently from df59503 to 82dfb68 Compare August 5, 2022 21:42
@msbutler msbutler mentioned this pull request Aug 5, 2022
Closed
@msbutler
backupccl: move datadriven and test setup infra to different package …
43c2b3f 
…for IMPORT

This refactor moves the backupccl data driven test infra as well as a few test
setup functions to the new pkg/ccl/testutilccl/bulktestutils package to allow
IMPORT tests to reuse this infrastructure.

A future patch should change all the test setup call sites in backup tests to
call the helper funcitons in this new bulktestutils package instead.

Release note: none
@msbutler msbutler force-pushed the butler-data-driver branch from 82dfb68 to 43c2b3f Compare August 6, 2022 18:16
@msbutler
Copy link
Copy Markdown
Collaborator Author

msbutler commented Aug 6, 2022 •
edited
Loading

All tests in the logictestccl package are failing on:

panic: /go/src/github.com/cockroachdb/cockroach/artifacts/go-build434372895/b1502/rttanalysisccl.test flag redefined: rewrite

These mysterious test failures are persistent and can be replicated locally via
./dev test pkg/ccl/logictestccl/tests/local

@msbutler msbutler mentioned this pull request Aug 9, 2022
Merged
@adityamaru
Copy link
Copy Markdown
Contributor

adityamaru commented Apr 3, 2023

(trimming down my for review list)

@adityamaru adityamaru removed their request for review April 3, 2023 12:45
@msbutler msbutler closed this Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@msbutler msbutler

Labels

T-disaster-recovery

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@msbutler @cockroach-teamcity @adityamaru