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

roachtest: re-skip two bank tests #53833

Merged
merged 2 commits into from
Sep 3, 2020
Merged

Conversation

tbg
Copy link
Member

@tbg tbg commented Sep 2, 2020

  • Revert "roachtest: re-enable acceptance/bank/zerosum-splits"
  • Revert "roachtest: re-enable acceptance/bank/zerosum-restart"

They hit flakes: #53829

In particular, there seems to be a fatal on an untracked goroutine, which causes roachtest to terminate prematurely.

Release justification: testing
Release note: None

tbg added 2 commits September 2, 2020 18:18
This reverts commit 9bd14db.

Release justification: testing
Release note: None
This reverts commit f27dc9a.

Release justification: testing
Release note: None
@tbg tbg requested a review from knz September 2, 2020 16:20
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg
Copy link
Member Author

tbg commented Sep 3, 2020

This has since incurred two more failures on merge-to-master, so going ahead with a self-review on behalf of test infra team

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 3, 2020

Build failed:

@tbg
Copy link
Member Author

tbg commented Sep 3, 2020

bors r+

Needed a release justification on the PR description as well

Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

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

I don't understand why you use a comment in the second line instead of a skip directive

@tbg
Copy link
Member Author

tbg commented Sep 3, 2020

I don't understand why you use a comment in the second line instead of a skip directive

I simply reverted the commits. I agree that a skip directive would have been better, this isn't worth another turnaround. I could imagine (have not checked) that this test has been skipped for longer than the skip directive existed.

@knz
Copy link
Contributor

knz commented Sep 3, 2020

In fact you added the skip to the first one without adding it to the second one initially. 🤓

@craig
Copy link
Contributor

craig bot commented Sep 3, 2020

Build succeeded:

@craig craig bot merged commit 91c7ae2 into cockroachdb:master Sep 3, 2020
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