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.

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

schemachanger: Unskip some backup tests #108300

Merged
merged 1 commit into from
Aug 8, 2023

Conversation

Xiang-Gu
Copy link
Contributor

@Xiang-Gu Xiang-Gu commented Aug 7, 2023

Randomly skip subtests in the BACKUP/RESTORE suites before parallelizing them.

Epic: None
Release note: None

@blathers-crl
Copy link

blathers-crl bot commented Aug 7, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@Xiang-Gu Xiang-Gu force-pushed the enable-skipped-backup-tests branch 3 times, most recently from 4ee46f6 to 9392d02 Compare August 8, 2023 11:56
@Xiang-Gu Xiang-Gu force-pushed the enable-skipped-backup-tests branch from 9392d02 to f397c13 Compare August 8, 2023 14:24
@Xiang-Gu Xiang-Gu changed the title WIP: schemachanger: Unskip some backup tests schemachanger: Unskip some backup tests Aug 8, 2023
@Xiang-Gu Xiang-Gu marked this pull request as ready for review August 8, 2023 20:18
@Xiang-Gu Xiang-Gu requested a review from a team as a code owner August 8, 2023 20:18
@Xiang-Gu
Copy link
Contributor Author

Xiang-Gu commented Aug 8, 2023

CI succeeded for 4-5 runs when it's in draft so I think it's ready for a review to unskip those tests.

@ecwall
Copy link
Contributor

ecwall commented Aug 8, 2023

Are the parallelization and skipThreshold changes part of deflaking the skipped tests? If so, could you explain how those changes deflake the tests?

@Xiang-Gu
Copy link
Contributor Author

Xiang-Gu commented Aug 8, 2023

@ecwall They make those test run less subtests (by getting rid of skipThreshold) and faster (by skipping a test before calling t.Parallel() which otherwise would pause the process and continue it later only to find out it will be skipped anyway).

@Xiang-Gu
Copy link
Contributor Author

Xiang-Gu commented Aug 8, 2023

bors r+

@craig craig bot merged commit 69bc4c6 into cockroachdb:master Aug 8, 2023
@craig
Copy link
Contributor

craig bot commented Aug 8, 2023

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants