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: rewrite backup tests as datadriven tests #77129

Closed
adityamaru opened this issue Feb 28, 2022 · 2 comments
Closed

backupccl: rewrite backup tests as datadriven tests #77129

adityamaru opened this issue Feb 28, 2022 · 2 comments
Assignees
@adityamaru
Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) stability-period Intended to be worked on during a stability period. Use with the Milestone field to specify version. T-disaster-recovery

Comments

@adityamaru
Copy link
Contributor

adityamaru commented Feb 28, 2022 •
edited by cockroach-jira-scripts
Loading

As of today, backup_test.go is the second-largest test file in CRDB at ~9k lines. It is becoming harder to understand the existing backup+restore test coverage, maintain older tests, and is not exactly setting a good standard for new tests. A lot of folks (myself included) default to plugging in subtests or one-line "tests" in an existing Test function when developing new functionality, instead of a dedicated test that is easily identifiable by name.

At CRDB we really like the readability and expressiveness of data-driven tests. This along with new jobs infrastructure such as jobs.debug.pausepoints has motivated us to try and rewrite as much of backup_test.go as dedicated data-driven tests. I believe this can be achieved in three general steps:

  • Refactor the data-driven framework and make it more featureful to support writing existing tests.
  • Datadrivenify existing backup/restore tests.
  • Promote a culture of first reaching for data-driven tests when testing new functionality.

Epic CRDB-7779

Jira issue: CRDB-13425
@adityamaru adityamaru added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-disaster-recovery labels Feb 28, 2022
@adityamaru adityamaru self-assigned this Feb 28, 2022
@blathers-crl
Copy link

blathers-crl bot commented Feb 28, 2022

cc @cockroachdb/bulk-io

@adityamaru adityamaru mentioned this issue Mar 5, 2022
Merged
@adityamaru adityamaru mentioned this issue Mar 16, 2022
Merged
adityamaru added a commit to adityamaru/cockroach that referenced this issue Mar 17, 2022
@adityamaru
backupccl: move jobs redaction tests to datadriven framework
57adfbb 
Informs: cockroachdb#77129

Release note: None

Release justification: non-production code changes
@adityamaru adityamaru mentioned this issue Mar 20, 2022
Closed
adityamaru added a commit to adityamaru/cockroach that referenced this issue Mar 22, 2022
@adityamaru
backupccl: datadrivenify TestBackupRestoreUserDefinedSchemas
91e0f63 
This tests previously started 8 test clusters, now it starts
just 2!

Informs: cockroachdb#77129

Release note: None
@adityamaru adityamaru mentioned this issue Mar 22, 2022
Closed
adityamaru added a commit to adityamaru/cockroach that referenced this issue Mar 24, 2022
@adityamaru
backupccl: move jobs redaction tests to datadriven framework
7c0b610 
Informs: cockroachdb#77129

Release note: None

Release justification: non-production code changes
@adityamaru adityamaru mentioned this issue Mar 24, 2022
Merged
@adityamaru adityamaru mentioned this issue Mar 24, 2022
Merged
adityamaru added a commit to adityamaru/cockroach that referenced this issue Mar 28, 2022
@adityamaru
backupccl: moved datadriven test framework to its own file
9a5de4b 
This change only moves the datadriven test and its utility methods
into its own file.

Informs: cockroachdb#77129

Release note: None
craig bot pushed a commit that referenced this issue Mar 29, 2022
@adityamaru
Merge #78428
5d67851 
78428: backupccl: moved datadriven test framework to its own file r=msbutler a=adityamaru

This change only moves the datadriven test and its utility methods
into its own file.

Informs: #77129

Release note: None

Co-authored-by: Aditya Maru <[email protected]>
adityamaru added a commit to adityamaru/cockroach that referenced this issue Mar 29, 2022
@adityamaru
backupccl: datadrivenify the restore OnFailOrCancel tests
9470cc4 
This change ports all the "cleanup on failed or canceled restore"
tests to the datadriven framework. To make this possible new instructions
have been added to the datadriven framework, please refer to the header
comment for details about each one.

Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue Apr 1, 2022
@adityamaru
backupccl: datadrivenify the restore OnFailOrCancel tests
1ba873a 
This change ports all the "cleanup on failed or canceled restore"
tests to the datadriven framework. To make this possible new instructions
have been added to the datadriven framework, please refer to the header
comment for details about each one.

Informs: cockroachdb#77129

Release note: None
craig bot pushed a commit that referenced this issue Apr 1, 2022
@adityamaru @rickystewart
Merge #78430 #79167 #79236 #79242
675eb23 
78430: backupccl: datadrivenify the restore OnFailOrCancel tests r=msbutler a=adityamaru

This change ports all the "cleanup on failed or canceled restore"
tests to the datadriven framework. To make this possible new instructions
have been added to the datadriven framework, please refer to the header
comment for details about each one.

Informs: #77129

Release note: None

79167: bazel: update arguments passed to `configure` for dev macOS arm builds r=mari-crl a=rickystewart

Release note: None

79236: dev: `lint` should honor the number of CPUs when building cockroach r=irfansharif a=rickystewart

Release note: None

79242: docs: generate swagger docs w/ bazel r=mari-crl,rail a=rickystewart

Also build in CI as well as `dev generate docs`.

Closes #77395.

Release note: None

Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
adityamaru added a commit to adityamaru/cockroach that referenced this issue Apr 6, 2022
@adityamaru
backupccl: port job redaction tests to datadriven tests
ba33012 
Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue May 25, 2022
@adityamaru
backupccl: moved datadriven test framework to its own file
eaf196d 
This change only moves the datadriven test and its utility methods
into its own file.

Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue May 25, 2022
@adityamaru
backupccl: moved datadriven test framework to its own file
3f2603d 
This change only moves the datadriven test and its utility methods
into its own file.

Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue May 25, 2022
@adityamaru
backupccl: datadrivenify the restore OnFailOrCancel tests
c512c65 
This change ports all the "cleanup on failed or canceled restore"
tests to the datadriven framework. To make this possible new instructions
have been added to the datadriven framework, please refer to the header
comment for details about each one.

Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue May 27, 2022
@adityamaru
backupccl: moved datadriven test framework to its own file
777426d 
This change only moves the datadriven test and its utility methods
into its own file.

Informs: cockroachdb#77129

Release note: None
adityamaru added a commit to adityamaru/cockroach that referenced this issue May 27, 2022
@adityamaru
backupccl: datadrivenify the restore OnFailOrCancel tests
4d8acb3 
This change ports all the "cleanup on failed or canceled restore"
tests to the datadriven framework. To make this possible new instructions
have been added to the datadriven framework, please refer to the header
comment for details about each one.

Informs: cockroachdb#77129

Release note: None
@adityamaru adityamaru added the stability-period Intended to be worked on during a stability period. Use with the Milestone field to specify version. label Aug 3, 2022
@adityamaru
Copy link
Contributor Author

We have a datadriven framework that is now actively being used. This issue has served its purpose of familiarizing the team with this framework.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adityamaru adityamaru

Labels
A-disaster-recovery C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) stability-period Intended to be worked on during a stability period. Use with the Milestone field to specify version. T-disaster-recovery
Projects
No open projects
Disaster Recovery Backlog
Archived in project
Milestone
No milestone
Development

No branches or pull requests
1 participant
@adityamaru