-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
chore: Replace testify with gotest.tools in genutil integration tests #14479
chore: Replace testify with gotest.tools in genutil integration tests #14479
Conversation
…ita/replace-testify-genutil-integration-tests
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, two nits.
…ita/replace-testify-genutil-integration-tests
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, couple of nits.
"github.com/stretchr/testify/require" | ||
tmproto "github.com/tendermint/tendermint/proto/tendermint/types" | ||
"gotest.tools/v3/assert" |
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 we use any of assert
or require
? or both are required?
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.
we should be using only assert
but there are less utilities available in gotest.tools
, so I used require
whenever there is a panic check since there isn't one in assert
to do that.
This will be fine as of now because our main goal is to run tests standalone without any suite
.
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't we write our own util using assert
?
func AssertNotPanics(fn func()) {
defer {
// recover and use assert to ensure no error
}()
fn()
}
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.
certainly, I could do that in next PR's and will do a follow-up for genutil tests
Description
closes: #14492
ref: #12332 #14145
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!
to the type prefix if API or client breaking changeCHANGELOG.md
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!
in the type prefix if API or client breaking change