-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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/costfuzz: create failure.log file on demand #108739
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix!
Reviewed all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @yuzefovich)
pkg/cmd/roachtest/tests/query_comparison_util.go
line 168 at r1 (raw file):
} }() defer failureLog.Close()
Should this line be removed because now failureLog
might be nil and the deferred function is doing the closing?
This commit makes it so that `failure.log` files are created lazily on demand in `costfuzz` and `unoptimized_query_oracle` roachtests. It seems nicer this way since for successful runs previously we'd see empty failure log files which were a bit confusing. Epic: None Release note: None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @michae2 and @msirek)
pkg/cmd/roachtest/tests/query_comparison_util.go
line 168 at r1 (raw file):
Previously, msirek (Mark Sirek) wrote…
Should this line be removed because now
failureLog
might be nil and the deferred function is doing the closing?
Oops, nice catch, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @michae2)
TFTR! bors r+ |
Build succeeded: |
This commit makes it so that
failure.log
files are created lazily on demand incostfuzz
andunoptimized_query_oracle
roachtests. It seems nicer this way since for successful runs previously we'd see empty failure log files which were a bit confusing.Epic: None
Release note: None