Change expectPass with bool fields in test cases to `expectedError` fields with error types

[mattverse]

Background

Most of the current test cases have an expectPass or an expectFail, expectError with a boolean value set as its field. (ex. https://github.com/osmosis-labs/osmosis/blob/main/x/gamm/keeper/msg_server_test.go).

Although these test cases are still valid, these test cases sometimes can turn out to be inaccurate, as the test case itself can omit different errors from different parts of the code from different reasons, not exactly from the logic we're trying to test, and not exactly from the reason we're expecting it to error.

Suggested Design

Although we do have alot of these test cases with the test cases of having these expectPass boolean fields as test cases, we should move towards chagning these to expectedError with the type of Error and precisely check the Error that's happening to ensure that we're erroring for reasons that we're expecting.

A good example to reference is https://github.com/osmosis-labs/osmosis/blob/main/x/mint/keeper/keeper_test.go#L136

For tracking:

• [x] gamm
• [ ] epoch
• [ ] incentives
• [ ] lockup
• [ ] mint
• [x] pool-incentives
• [ ] superfluid
• [ ] tokenfactory
• [ ] twap
• [ ] txfees

[hieuvubk]

I think we should break this down by each module to make it easier to follow

[hieuvubk]

And is anyone working on? I can start with it

[mattverse]

@hieuvubk please go ahead! That'd be great! Might be almost impossible to do this in a single PR since the scope is too big, might want to do it by module to module scope.

[ValarDragon]

Wait I thought we decided not to do this

[czarcas7ic]

Correct, I am going to close these respective PRs and we can re address at another time.

[czarcas7ic]

Respective PRs closed, closing this issue. Still think we should discuss internally to get a shared understanding of what is the best endstate for tests is