Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

msbutler
Copy link
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
Member

This change is Reviewable

@adityamaru
Copy link
Contributor

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
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, rhu713 and adityamaru 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
Contributor

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
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
Contributor

(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