-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Avoid panicking in tests #204
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.
can you please change to something like Expect(err).ToNot(HaveOccurred())
Yep |
/lgtm |
@adrianchiris PTAL |
@zeeke can you please have a look on the CI status? |
Using Gomega assertions makes test errors more readable. Upgrade GolangCI to v.1.45 to avoid errors like: "export data is newer version - update tool" Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Yes, It seems to be related to an incompatibility between GolangCI 1.29 and Go 1.18 as GolangCI actions I encountered the same problem in #205 and I think it's not related to the code changes themselves. I fixed upgrading golangci to 1.49 |
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.
LGTM. Thanks for the cleaning this up
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.
LGTM !
Using Gomega assertions makes test errors more readable. Upgrade GolangCI to v.1.45 to avoid errors like: "export data is newer version - update tool" Signed-off-by: Andrea Panattoni <apanatto@redhat.com>
Using Gomega assertions makes test errors more readable in most cases.
Signed-off-by: Andrea Panattoni apanatto@redhat.com