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

roachtest: remove confusing "--- PASS" log lines #52273

Merged
merged 1 commit into from
Aug 4, 2020
Merged

roachtest: remove confusing "--- PASS" log lines #52273

merged 1 commit into from
Aug 4, 2020

Conversation

andreimatei
Copy link
Contributor

Two workloads were printing PASS/FAIL lines from inside the predicates
passed to Searcher.Search. This is really confusing when reading the
test's output, because they don't correspond to the test's disposition.

Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@nvanbenschoten
Copy link
Member

I don't think we should remove this. It's very useful to be able to quickly track through the search history of the test. Is it really that confusing?

@andreimatei
Copy link
Contributor Author

Is it really that confusing?

It is! It's the second time I see these lines and get confused. And I know my way around roachtest logs some...
Do you not like the new messages I'm logging? I'm happy to format them in any other way.

@nvanbenschoten
Copy link
Member

Do you not like the new messages I'm logging?

Well it just doesn't stand out at all, so it will make it hard to skim. Would changing --- PASS: to --- ITER PASS: help? Or adding four spaces before the --- PASS:? FWIW, the idea was to format this like subtest statuses are formatted within a parent test.

@andreimatei
roachtest: remove confusing "--- PASS" log lines
d1d27ba 
Two workloads were printing PASS/FAIL lines from inside the predicates
passed to Searcher.Search. This is really confusing when reading the
test's output, because they don't correspond to the test's disposition.

Release note: None
andreimatei
andreimatei commented Aug 3, 2020
View reviewed changes
Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

Went with --- SEARCH ITER {PASS,FAIL}.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15 and @nvanbenschoten)

nvanbenschoten
nvanbenschoten approved these changes Aug 3, 2020
View reviewed changes
Copy link
Member

@nvanbenschoten nvanbenschoten left a comment

Choose a reason for hiding this comment

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

LGTM

andreimatei
andreimatei commented Aug 4, 2020
View reviewed changes
Copy link
Contributor Author

@andreimatei andreimatei left a comment

Choose a reason for hiding this comment

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

bors r+

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

@craig
Copy link
Contributor

craig bot commented Aug 4, 2020

This PR was included in a batch that was canceled, it will be automatically retried

@craig
Copy link
Contributor

craig bot commented Aug 4, 2020

Build succeeded:

@craig craig bot merged commit 986e308 into cockroachdb:master Aug 4, 2020
@andreimatei andreimatei deleted the raochtest.fake-pass branch August 21, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nvanbenschoten nvanbenschoten nvanbenschoten approved these changes

@aayushshah15 aayushshah15 Awaiting requested review from aayushshah15

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andreimatei @cockroach-teamcity @nvanbenschoten