Remove testutil/network package

[alexanderbez]

We use the testutil/network to run an in-process single Tendermint node. We use this primarily for CLI integration tests and handful of gRPC tests.

Using this approach/package has proven to be troublesome because of the requirement to run tests with the race detector on due to Tendermint needing to be single-threaded (it has many globals as state, especially within the RPC layer).

Many times these race-enabled tests fail, requiring tests to be reran multiple times, which causes developers to waste time having to rerun CI.

A cleaner approach would be to just "mock" Tendermint indirectly. Meaning, we wrap BaseApp as a fixture/wrapper that allows us access to the message router and context. This allows us to execute txs/messages, EndBlock, or BeginBlock, giving us roughly the same functionality that we require now.

As a reference, see what Regen has:

• (Fixture) https://github.com/regen-network/regen-ledger/blob/7c2e9041a9f930ec5890d2da0ccddab4333a2bd1/types/module/server/testutil.go
• (Example Test) https://github.com/regen-network/regen-ledger/blob/master/x/ecocredit/server/testsuite/suite.go

Note, we don't need to copy the approach verbatim, but we can use it as guidance for the fixture and tests.

---

Acceptance Criteria:

• Fixture implemented
• CLI and any other relevant tests that use testutil/network are migrated to use fixture
• Remove testutil/network
• Remove race tests from CI

[kocubinski]

After some consideration, the full network end-to-end tests in testutil/network do have some value, but we've over used them. Perhaps they can be collapsed into fewer tests with equivalent coverage. Another problem with our current usage is that nearly every module depends on simapp (and therefore every other module) in their tests.

In #12546 I'm planning to move all deps on testutil/network to something like simapp/integration/network, and as a follow up move the end-to-end tests in each module into simapp, breaking the dependency.

Mocking the network layer like this issue describes would introduce a very useful tier of tests just below end-to-end network tests, but I'm not sure it will solve the "ball of mud problem" where module tests spin up an entire app to test their specific functionality. The ones I have reviewed are often relying on module interactions above the network layer and break when the wiring logic in simapp is removed.

[amaury1093]

Also, there's a CLI command simd testnet that boots up a localnet. It uses testutil/network, and AFAIK it might be used by some users (see e.g. #12534), so I probably won't remove it without some kind of notice.

I know some app developers just copy/paste simapp/* for their app chain. If they want to have this simd testnet command, would they also need to copy/paste the contents of simapp/integration/network (ideally, no)?

[alexanderbez]

After some consideration, the full network end-to-end tests in testutil/network do have some value, but we've over used them

There are no tests in testutil/network, at least not that I know of. I wrote testutil/network solely for CLI tests, which are more or less integration tests. What are you referring to as E2E tests here?

Perhaps they can be collapsed into fewer tests with equivalent coverage.

What tests are "they" in this context?

Another problem with our current usage is that nearly every module depends on simapp (and therefore every other module) in their tests.

This is orthogonal to the usage and removal of testutil/network.

In #12546 I'm planning to move all deps on testutil/network to something like simapp/integration/network, and as a follow up move the end-to-end tests in each module into simapp, breaking the dependency.

That's OK for the super short term, but the proposal here to completely remove testutil/network as there is no reason for it to exists whatsoever.

Mocking the network layer like this issue describes would introduce a very useful tier of tests just below end-to-end network tests,

I think there is some confusion. We don't really have E2E tests, what testutil/network is used for is integration tests in the CLI packages and a handful of gRPC tests. We're not trying to mock networking or anything similar at all. We just need block execution.

That being said, we do have a liveness tests, which you would consider E2E, but this uses a more sophisticated Docker approach, which is correct IMO.

[kocubinski]

There are no tests in testutil/network

I am speaking to the extensive usage of testutil/network by modules and packages well beyond CLI tests. Not tests within testutil/network.

My original sentence should read: After some consideration, the full network end-to-end tests relying on testutil/network do have some value, but we've over used them.

What tests are "they" in this context?

The adoption of testutil/network is widespread, one example usage (there are many). For simplicity I'm referring to all these usages as e2e tests.

This is orthogonal to the usage and removal of testutil/network.

Not really so for removal. Since testutil/network starts up simapp with all modules loaded by default, e2e tests often rely on complex module interactions in their test cases. Removal of testutil/network will mean refactoring to either specify module dependencies explicitly via app-wiring or mock them in order to address this implicit dependency.

[alexanderbez]

I am speaking to the extensive usage of testutil/network by modules and packages well beyond CLI tests.

That's exactly my point, they should have never used testutil/network to begin with and still should not.

After some consideration, the full network end-to-end tests relying on testutil/network do have some value, but we've over used them.

Where are these tests? Please point me to them. I know of no such E2E tests in the SDK.

The adoption of testutil/network is widespread, one example usage (there are many). For simplicity I'm referring to all these usages as e2e tests.

This is not an E2E tests and also should not be using testutil/network.

[kocubinski]

We seem to have a disagreement on terms. The working definition for E2E test I'm using is as I've layed out in the example above, and as used in #12398.

What do you consider an E2E test, can you point me to an example? If the example I've linked above (here again) is not an E2E test, how should we identify it? I might suggest network-backed integration test.

[alexanderbez]

I call those integration tests :) and they don't need testutils/network.

[kocubinski]

Right, they need simapp, which testutils/network depends on. After discussing I can see this fact is out of scope for this issue, and I'll focus on resolving it in #12546 😅

Once module interactions are better understood, I think we'll be in a good position to address network mocking.

[ValarDragon]

This framing feels odd to me. The goal should have no Tendermint OR message routing. The CLI has three (or four) roles:

• Msg creation from args + state + signing
• Tx Broadcast
• Tx eventually getting on chain and executing

The issue is these are all bundled into one method in code and tests right now, so we have this very convoluted test. Tx broadcast is being suggested to be removed, by mocking tendermint. But I claim that tx execution should also be removed / all simulation of a network.

I want my average CLI test to be some type of table driven test, where I supply CLI command args + some blockchain state setup. (Block height, etc.). Then I get out of the CLI, a constructed message (maybe tx depending on API), and then I can test that the message generated satisfies properties I want. (Could also just run validate basic for good measure if wanted)

But I don't think tx execution should be here for CLI tests either. (Goal is to get YML specification of CLI, and then were just unit testing integration against state)

[alexanderbez]

I want my average CLI test to be some type of table driven test, where I supply CLI command args + some blockchain state setup. (Block height, etc.).

This 100%

[tac0turtle]

closing this as the path for testutil has changed a little. Ill open a new issue