Cosmos-sdk: Investigate potential areas of improvement in testing user experience

[alessio]

Improve SDK developer UX for testing in the following areas:

• [ ] [#4533](https://github.com/cosmos/cosmos-sdk/issues/4533) Standardize and document keeper and module unit testing patterns.
• [ ] [#4534](https://github.com/cosmos/cosmos-sdk/issues/4534) Move as much of the simulation code into the modules as possible and document pattern for new module to add simulations.
• [x] [#4535](https://github.com/cosmos/cosmos-sdk/issues/4535) Improve the errors coming out of the simulation.

---

For Admin Use

• [ ] Not duplicate issue
• [ ] Appropriate labels applied
• [ ] Appropriate contributors tagged
• [ ] Contributor assigned/self-assigned

[alessio]

I seem to remember that @jackzampolin added this to the original wishlist - Jack would you elaborate please?

[fedekunze]

Imo this is a huge pain point. Having to create all the test inputs in the test_common.go files and defining multiple times the the keys, keepers, etc is not optimal. Also there's lack of standardization as some modules use x/mock while others have a custom setup having to mount the store, create keys and keepers, nAccounts, etc.

One idea could be upgrading x/mock to use the new module generalization and create a test app by just defining the module names. Then we can use that test app to test the keeper functions

[alessio]

Agreed - just a small thing on the name, I'd call such a module x/testing rather than x/mock to indicate that it consists of testing utilities.

[jackzampolin]

So I think after talking with @fedekunze I think this breaks down to the following improvements:

• Standardize and document keeper and module unit testing patterns.
• Move as much of the simulation code into the modules as possible and document pattern for new module to add simulations.
• Improve the errors coming out of the simulation.

[fedekunze]

@okwme @marbar3778 @hschoenburg any ideas here?

[cbarraford]

I've started building a testing framework for building custom modules, and been experimentally using it on my custom module, specifically the keeper functions (so far). The problem I'm running into as i get further into this, many of the objects that get passed around are type struct (like sdk.Context, sdk.Handler, etc), instead of type interface (like bank.Keeper is).

What I would like to see happen is that these objects we pass around to keepers/handlers/etc are interfaces instead of structs. That then gives me the ability to create mock structs that adhere to those interfaces and pass it around as needed in my tests. Does this make sense? Thoughts?

If people are into this, I don't mind forking the cosmos-sdk, making the appropriate changes, and opening a pull request.

[alexanderbez]

@cbarraford, with regards to keepers, we've already started on this path (e.g. x/crisis). Namely, we use the notion of expected_keepers.go (really should be expected_types.go imho) to define the contracts that input parameters into keepers must fulfill.

Where else do you see a need for this?

[cbarraford]

i see a use case in a bunch of places. So far i've created a mock sdk.Context, bank.Keeper, KVstore in my Parsec repo. I started to try to add a mock sdk.Handler but ran into complications that seem to require a change in the cosmos-sdk to fix. The issue i ran into is...

x/multisig/module.go:23:2: cannot use composite literal (type AppModule) as type module.AppModule in assignment:
    AppModule does not implement module.AppModule (wrong type for NewHandler method)
        have NewHandler() parsec.Handler
        want NewHandler() "github.com/cosmos/cosmos-sdk/types".Handler

Basically, I'm trying to figure out a testing pattern to break out each part into small easy to write, unit tests. Starting with keepers, then moving onto handlers, all the way up to cli/rest endpoints. But I am relatively new to cosmos development. Maybe theres knowledge I'm missing here.

[alexanderbez]

I'm not sure why the Context needs to be mocked? Can you elaborate as to why? Maybe we need to improve that. Context should not be an interface IMHO. Wrt to handlers and the issue you ran into, why not have your functions return the Handler interface? Am I missing something?

[cbarraford]

@alexanderbez The context needs to be mocked because its an input into functions in keeper.go. From the context that is passed in as an argument, we pull the KVStore to be able to set/get something. So I was thinking if I mock the context, and make a simple in-memory kv store designed specifically for testing (getters, setters, reset funcs for test suite teardowns, helper funcs for large pre population test suite setups), you can test a keeper setter function and then check that the set was called with the proper inputs. Same for getter functions, you can easily prepopulate the in-memory test db with records (key/values). Does that make sense?

For an example of this in action, check out https://github.com/cbarraford/cosmos-multisig/blob/master/x/multisig/keeper.go#L40-L43 and the corresponding keeper_test.go.

What makes you say that Context should NOT be an interface? I'm a bit surprised considering that the sdk Context inherits from the standard library context.Context, which IS an interface. Can you explain your thinking behind that? If Context is an interface, people can design their own if desired. Maybe for testing purposes (like I've explained a bit and done), or maybe to add additional functionality/data 🤷‍♂. By making these module components more flexible, it makes cosmos more flexible, and therefore more powerful in my mind.
I hope I'm making sense here.

[fedekunze]

closing this in favour of the individual issues