refactor calls based on required error success in broadcast message. #3475
Conversation
e2e/tests/transfer/base_test.go
Outdated
| } else { | ||
| s.Require().Equal(uint32(2), transferTxResp.Code) | ||
| s.Require().Contains(transferTxResp.RawLog, "errUnknownField") | ||
| s.Require().Contains("errUnknownField", transferTxResp.RawLog) |
There was a problem hiding this comment.
as far as I could tell this is coming from unknownproto in the sdk, couldn't find a registered error for it elsewhere.
There was a problem hiding this comment.
That's correct. I can't remember exactly where the SDK is constructing the returned error. The test is passing a sdk.Msg with an extra field the chain doesn't understand, so when it tries to unmarshal the sdk.Msg, the unknown field error is returned. I think it's fine to leave as is. It would be nice if we knew the root error though. I wonder if it is this error. Ah I think I found it!
Is the updated change the right ordering? Looks like the Contains godoc has the elem in question as the second argument?
DimitrisJim
force-pushed
the
3435-e2e-replace-assertvalidtxresponse-and-error-check-with-assertsuccesstx-or-assertfailedtx
branch
from
5ba660d to
8acc156
Compare
DimitrisJim
force-pushed
the
3435-e2e-replace-assertvalidtxresponse-and-error-check-with-assertsuccesstx-or-assertfailedtx
branch
from
8acc156 to
54d245e
Compare
DimitrisJim
requested review from
AdityaSripal,
crodriguezvega,
charleenfei and
damiannolan
as code owners
e2e/tests/transfer/base_test.go
Outdated
| } else { | ||
| s.Require().Equal(uint32(2), transferTxResp.Code) | ||
| s.Require().Contains(transferTxResp.RawLog, "errUnknownField") | ||
| s.Require().Contains("errUnknownField", transferTxResp.RawLog) |
There was a problem hiding this comment.
That's correct. I can't remember exactly where the SDK is constructing the returned error. The test is passing a sdk.Msg with an extra field the chain doesn't understand, so when it tries to unmarshal the sdk.Msg, the unknown field error is returned. I think it's fine to leave as is. It would be nice if we knew the root error though. I wonder if it is this error. Ah I think I found it!
Is the updated change the right ordering? Looks like the Contains godoc has the elem in question as the second argument?
…rror-check-with-assertsuccesstx-or-assertfailedtx
…rror-check-with-assertsuccesstx-or-assertfailedtx
…rror-check-with-assertsuccesstx-or-assertfailedtx
…rror-check-with-assertsuccesstx-or-assertfailedtx
…rror-check-with-assertsuccesstx-or-assertfailedtx
|
note: would be nice if #3552 is handled first in order to get the tests that require specific errors from there to use them here, see #3475 (comment) |
…rror-check-with-assertsuccesstx-or-assertfailedtx
…rror-check-with-assertsuccesstx-or-assertfailedtx
DimitrisJim
deleted the
3435-e2e-replace-assertvalidtxresponse-and-error-check-with-assertsuccesstx-or-assertfailedtx
branch
Description
closes: #3435
Commit Message / Changelog Entry
imp: refactor calls based on required error success in broadcast message.see the guidelines for commit messages. (view raw markdown for examples)
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/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.