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

test: Improve logging of errors that must be investigated #12598

Merged

Conversation

joestringer
Copy link
Member

Only print the unique logs, and ensure they're printed not only to the
checker output but also to the regular ginkgo output.

Only print the unique logs, and ensure they're printed not only to the
checker output but also to the regular ginkgo output.

Signed-off-by: Joe Stringer <joe@cilium.io>
@joestringer joestringer added the release-note/ci This PR makes changes to the CI. label Jul 20, 2020
@joestringer joestringer requested a review from a team as a code owner July 20, 2020 20:36
Copy link
Member

@christarazi christarazi left a comment

Choose a reason for hiding this comment

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

Nice!

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 37.025% when pulling 27125b0 on joestringer:submit/better-ci-required-log-messages into 0cb6c1f on cilium:master.

@christarazi
Copy link
Member

test-me-please

failures := make([]string, 0, len(uniqueFailures))
for f, c := range uniqueFailures {
failures = append(failures, f)
fmt.Fprintf(CheckLogs, "⚠️ Found %q in logs %d times\n", f, c)
Copy link
Member

@brb brb Jul 21, 2020

Choose a reason for hiding this comment

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

Bikeshedding nit: does the ⚠️ char serve any purpose in the log msg?;-)

Copy link
Member Author

Choose a reason for hiding this comment

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

It's been a while since I even thought about it but the idea is to highlight this concern in the output. It certainly gives these particular messages a certain aesthetic, breaking up the flow of the log output and helps to highlights this output (though these days there's always false positives due to certain logs triggered by our CI system :( )

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 21, 2020
@rolinh rolinh merged commit b739801 into cilium:master Jul 21, 2020
@joestringer joestringer deleted the submit/better-ci-required-log-messages branch July 21, 2020 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/ci This PR makes changes to the CI.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants