-
Notifications
You must be signed in to change notification settings - Fork 579
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
chore: use expected errors in ica host test. #7181
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!
@@ -171,7 +171,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { | |||
) (string, error) { | |||
return "", fmt.Errorf("mock ica auth fails") | |||
} | |||
}, true, | |||
}, nil, |
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 could move this test up together the success ones.
@@ -291,7 +291,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenConfirm() { | |||
) error { | |||
return fmt.Errorf("mock ica auth fails") | |||
} | |||
}, true, | |||
}, nil, |
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.
Ditto.
}, | ||
{ | ||
"invalid metadata bytestring", | ||
func() { | ||
// the try step will propose a new valid version | ||
path.EndpointA.ChannelConfig.Version = "invalid-metadata-bytestring" | ||
}, | ||
true, | ||
nil, |
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 could also relocate this one up.
@@ -136,7 +136,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() { | |||
testCases := []struct { | |||
name string | |||
instantiateFn func() | |||
expPass bool | |||
exxPanicMsg string |
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.
exxPanicMsg string | |
expPanicMsg string |
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 could also just use panicMsg
, since you use errMsg
in other tests, but it's just a nit :)
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 fixing these up!
Quality Gate passed for 'ibc-go'Issues Measures |
Description
closes: #7180
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
).godoc
comments.Files changed
in the GitHub PR explorer.SonarCloud Report
in the comment section below once CI passes.