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: categorise post-test assertions separately #103946

Merged
merged 2 commits into from
Jun 12, 2023

Conversation

smg260
Copy link
Contributor

@smg260 smg260 commented May 26, 2023

roachtest: categorise post-test assertion errors separately

  1. Simple change to mark failures that occur in post-test validation (teardown), so that we can more clearly report a test that has experience a failure after its execution has ended.

  2. Every test, unless opted-out, runs various post validations such as node liveness and consistency checks. These checks have been moved to their own function, and failures that occur here are now logged to a separate file, and indicated as such in github when an issue is created.

    Previously, these checks were done during teardown and logged to the teardown.log.

Epic: none
Fixes: #98366

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@smg260 smg260 changed the title 98366 teardown errors roachtest: categorise post-test assertions separately May 26, 2023
@smg260
Copy link
Contributor Author

smg260 commented May 26, 2023

I was in 2 minds about perhaps keeping the post-test logging in the primary test.log vs having its own test-post-assertions.log (I went with the latter)

@smg260 smg260 marked this pull request as ready for review May 26, 2023 16:53
@smg260 smg260 requested a review from a team as a code owner May 26, 2023 16:53
@smg260 smg260 requested review from srosenberg, renatolabs and herkolategan and removed request for a team May 26, 2023 16:53
Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Nice change! Just some small nits about maybe using a switch for category/failure. I like the log separation, so I'm glad you chose to go with that.

Reviewed 5 of 5 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @renatolabs, @smg260, and @srosenberg)


pkg/cmd/roachtest/github.go line 122 at r1 (raw file):

	var infraFlake bool
	// Overrides to shield eng teams from potential flakes
	if cat == clusterCreationErr {

Nit: switch statement might give this a cleaner look.

@smg260 smg260 force-pushed the 98366_teardown_errors branch 2 times, most recently from f1c492d to 0821d4b Compare June 6, 2023 15:29
@smg260
Copy link
Contributor Author

smg260 commented Jun 6, 2023

Will merge after this nightly roachtest completes. tpc* tests excluded.

Miral Gadani added 2 commits June 9, 2023 21:27
Every test, unless opted-out, runs various post validations such as
node liveness and consistency checks. Failures that occur here are
now logged to a separate file, and indicated as such in github when
an issue is created.

Previously, these checks were done during teardown and logged to the
teardown.log.
@smg260
Copy link
Contributor Author

smg260 commented Jun 9, 2023

bors r=herkolategan

TFTR

@smg260
Copy link
Contributor Author

smg260 commented Jun 9, 2023

bors r-

@craig
Copy link
Contributor

craig bot commented Jun 9, 2023

Canceled.

@smg260
Copy link
Contributor Author

smg260 commented Jun 9, 2023

bors r+ single on p=999

@craig
Copy link
Contributor

craig bot commented Jun 9, 2023

Build failed:

@smg260
Copy link
Contributor Author

smg260 commented Jun 10, 2023

bors retry single on p=999

@craig
Copy link
Contributor

craig bot commented Jun 10, 2023

Build failed:

@smg260
Copy link
Contributor Author

smg260 commented Jun 10, 2023

the build posted by craig actually succeeded, despite it claming otherwise... bors is messed up

bors retry

@craig
Copy link
Contributor

craig bot commented Jun 10, 2023

Build failed:

@smg260
Copy link
Contributor Author

smg260 commented Jun 12, 2023

bors retry

@craig
Copy link
Contributor

craig bot commented Jun 12, 2023

Build succeeded:

@craig craig bot merged commit 9c2f650 into cockroachdb:master Jun 12, 2023
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.

roachtest: add failure message to test if post validations fail
3 participants