Cosmos-sdk: x: can we improve testing by reducing dependence on `mock` ?

[ebuchman]

In each module, we're using mock to mock out a BaseApp that we can load with what we need. But we use x/auth in there too, which means to test any of our modules we have to get setup with private keys and building and signing transactions.

It means we need to keep track of things like accounts nonces and increment them to send the next transaction, which is an unnecessary concern when we're eg. just trying to test the bank. It clutters the tests with long function calls, and leads to a weaker separation of concerns. For instance we were led to write replay-protection tests in the bank, where they don't belong.

It might be much nicer if module tests just directly tested their handler/keeper/msgs, without setting up the whole BaseApp. Instead of an app_test.go, with no corresponding app.go, we'd have a handler_test.go, which tests the handlers with various messages on various states of the keepers.

This would help achieve looser coupling between the modules. Could potentially enable us to remove mock all together

[alexanderbez]

I like this idea a lot. I think it'll make writing and reading tests much simpler. Although it would be also helpful to extract common functionality into a single place.

Also having a single end-to-end mock integration test would be good too.

[ValarDragon]

I think mock app is still going to be useful as an import for benchmarking. Other than that, I agree that tests for handling would be better than testing the txs. I think alot of the modules already do that though?

Also I'm not sure we want to remove the code for building the transactions, since that may need to be used by people building on the sdk. I think that code can be moved into a new file per module which has examples. (Following the golang example format)

[ebuchman]

I think mock app is still going to be useful as an import for benchmarking.

Let's split up our benchmarking the same way we want to split up our testing. To benchmark the bank module, all we need are its handlers/keeper/msgs - no app necessary. We don't want to contaminate our pure bank benchmark with amino decoding and signature validation.

Of course we want to benchmark the larger application - ie. full tx processing - but it doesn't seem to belong in the modules. Probably it belongs in gaia, where we have access to all the handlers and full setup that we truly want to benchmark so we can also get an idea of how the system behaves when fully composed.

Then maybe we can get rid of the mock all together...

I'm not sure we want to remove the code for building the transactions,

Surely not. We want to consolidate that in the client package where it seems we already have most of what we need, just perhaps with poor abstractions.

[rigelrozanski]

We are still going to need mock for abstracting gaia-lite (lcd) tests out to their respective modules... I reckon https://github.com/cosmos/cosmos-sdk/issues/1553

It might be much nicer if module tests just directly tested their handler/keeper/msgs, without setting up the whole BaseApp. Instead of an app_test.go, with no corresponding app.go, we'd have a handler_test.go, which tests the handlers with various messages on various states of the keepers.

Agreed, however I don't see the problem with using mock to test some more complicated logic which involved transactions from multiple modules.

[ebuchman]

Checkout the mocks used in baseapp_test.go in this PR: https://github.com/cosmos/cosmos-sdk/pull/1579

No dependence on outside modules, lets us fine tune exactly what we want to test without setting up more complex object-graphs (eg. signing txs)

[jackzampolin]

Can we close this issue?

[alexanderbez]

I dont think this has been resolved @jackzampolin -- just moved lower in priority.