Skip to content

schemachanger: make rollback tests data-driven#73177

Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom
postamar:better-rollback-test
Nov 26, 2021
Merged

schemachanger: make rollback tests data-driven#73177
craig[bot] merged 1 commit intocockroachdb:masterfrom
postamar:better-rollback-test

Conversation

@postamar
Copy link
Copy Markdown

@postamar postamar commented Nov 25, 2021

We recently introduced a TestRollback suite in the schemachanger package
to test the declarative schema changer's behavior in the face of
rollbacks. This commit rewrites this test suite to make it data-driven,
also it now consumes exactly the same test cases as the side-effects
tests. This is possible because the rollback tests don't have any
expected output (they only pass or fail). This is desirable because we
want these tests to be executed on an equally wide variety of cases.

This commit also adds a timeout on the execution of the test statements
to prevent them from hanging due to unrelated failures.

Release note: None

@postamar postamar requested a review from a team November 25, 2021 21:12
@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

@postamar postamar force-pushed the better-rollback-test branch 2 times, most recently from ebe3f13 to 22c91a4 Compare November 25, 2021 21:18
Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Reviewed 3 of 3 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @postamar)

Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @postamar)

@postamar postamar force-pushed the better-rollback-test branch from 22c91a4 to 7fe39a3 Compare November 25, 2021 21:33
@postamar
Copy link
Copy Markdown
Author

Thanks for the review! I force-pushed some changes which should make alter_table_add_column pass, I'd generated the contents while on a branch based off a stale master.

Copy link
Copy Markdown
Collaborator

@fqazi fqazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (and 1 stale) (waiting on @postamar)

We recently introduced a TestRollback suite in the schemachanger package
to test the declarative schema changer's behavior in the face of
rollbacks. This commit rewrites this test suite to make it data-driven,
also it now consumes exactly the same test cases as the side-effects
tests. This is possible because the rollback tests don't have any
expected output (they only pass or fail). This is desirable because we
want these tests to be executed on an equally wide variety of cases.

This commit also adds a timeout on the execution of the test statements
to prevent them from hanging due to unrelated failures.

Release note: None
@postamar postamar force-pushed the better-rollback-test branch from 7fe39a3 to af7b6b5 Compare November 26, 2021 14:50
@postamar
Copy link
Copy Markdown
Author

Just some more cosmetic changes.

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 26, 2021

Build failed:

@postamar
Copy link
Copy Markdown
Author

Failure seems completely unrelated 😕

bors r+

@craig
Copy link
Copy Markdown
Contributor

craig bot commented Nov 26, 2021

Build succeeded:

@craig craig bot merged commit 77ce3e2 into cockroachdb:master Nov 26, 2021
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.

3 participants