Skip to content
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

imp(tests): add utility to check balances in tests #2289

Merged
merged 3 commits into from
Jan 24, 2024

Conversation

MalteHerrmann
Copy link
Contributor

Description

This PR adds a helper function to more easily check expected balances during tests.

@MalteHerrmann MalteHerrmann requested a review from a team as a code owner January 24, 2024 11:38
@MalteHerrmann MalteHerrmann requested review from facs95 and 0xstepit and removed request for a team January 24, 2024 11:38
@github-actions github-actions bot added the tests label Jan 24, 2024
@MalteHerrmann MalteHerrmann merged commit 484c8a4 into main Jan 24, 2024
35 of 36 checks passed
@MalteHerrmann MalteHerrmann deleted the malte/add-bank-testing-utils branch January 24, 2024 17:52
}

for _, tc := range testcases {
tc := tc
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a super weird pattern in Go I've noticed 👀 in the past we've had problems where the tc fields were only using values of the last testcase - this solves it. It's very weird though and didn't occur on all tests though .. you'll also find this pattern everywhere in the Cosmos SDK repo for example and also we have it in our codebase a lot

image

)

func TestCheckBalances(t *testing.T) {
testDenom := "atest"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

just to check it also works not only for the basic Evmos denom 👀 not 100% necessary but also doesn't harm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants