Skip to content

Conversation

@tbruyelle
Copy link
Contributor

The change introduces a new interface to mock the call to tx.Sign(). I found more interesting to assert the call to that function rather than trying to assert its behavior in term of code, which is basically testing the cosmos-sdk, and we don't want to do that.

By adding the test I found a bug in the error comparison that I introduced previously with the rpcWrapper, so I fixed it.

The change also adds a bunch of expect helpers function to reduce the number of lines in the setup functions, which improves readability.

The change introduces a new interface to mock the call to tx.Sign(). I
found more interesting to assert the call to that function rather than
trying to assert its behavior in term of code, which is basically
testing the cosmos-sdk, and we don't want to do that.

By adding the test I found a bug in the error comparison that I
introduced previously with the rpcWrapper, so I fixed it.
@tbruyelle tbruyelle added the skip-changelog Don't check changelog for new entries label Sep 15, 2022
Copy link
Contributor

@aljo242 aljo242 left a comment

Choose a reason for hiding this comment

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

lgtm !

@tbruyelle tbruyelle merged commit 529407a into develop Sep 17, 2022
@tbruyelle tbruyelle deleted the test/cosmosclient-broadcasttx branch September 17, 2022 14:16
Jchicode pushed a commit to Jchicode/cli that referenced this pull request Aug 9, 2023
The change introduces a new interface to mock the call to tx.Sign(). I
found more interesting to assert the call to that function rather than
trying to assert its behavior in term of code, which is basically
testing the cosmos-sdk, and we don't want to do that.

By adding the test I found a bug in the error comparison that I
introduced previously with the rpcWrapper, so I fixed it.

Co-authored-by: Alex Johnson <alex@shmeeload.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Don't check changelog for new entries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants