In Process Integration Tests

[aaronc]

Summary

Running integration tests in a single process will improve code coverage metrics and make debugging easier.

Problem Definition

Currently CLI integration tests work by building CLI binaries and then calling the CLI binaries as sub-processes. While this is a somewhat realistic simulation of actual usage, it has the following downsides:

• integration test coverage isn't reflected in code coverage metrics, therefore we can't
  • account for code coverage which is actually achieved by integration tests, and
  • have no way of knowing which CLI/REST/etc. code isn't covered
• debugging is hard - there's no way to set breakpoints for failure either in the simd or simcli binaries

Proposal

• testnet framework (Test Network Testing Framework #6489)

Use the new testutil package to test command instances directly and in-process. As we're refactoring existing tests, we should be careful to construct/replace clean auxiliary functions for common operations (e.g. MsgSend).

• x/bank - x/bank: Refactor CLI & Tests #6525
• x/auth - x/auth: in-process test refactor #6573, x/auth: Finish CLI Refactor #6765
• x/crisis - x/crisis: In-Process CLI Integration Tests #6634
• x/genutil
• x/distribution - x/distribution: In-Process Testing & CLI Refactor #6652
• x/evidence Update x/{mint,slashing,evidence} cli to use gRPC query client #6704
• x/params - x/params: CLI Refactor & In-Process Tests #6720
• x/gov - CLI/Tests: Remove Fixtures #6799
• x/mint - CLI/Tests: Remove Fixtures #6799
• x/slashing - x/slashing: In-Process CLI Integration Tests #6702
• x/staking - CLI/Tests: Remove Fixtures #6799
• x/ibc - x/ibc: refactor CLI #6639
• x/ibc-transfer
• x/upgrade - CLI/Tests: Remove Fixtures #6799

---

Original Proposal (for future reference)

1. Make *cobra.Cmds respect stdin, stdout, & args overrides

All CLI methods are implemented using *cobra.Cmd which has SetIn, SetOut, SetArgs, etc. methods to override the default os.StdIn, os.Stdout, os.Args. As long as Cmds and the code they call respect these (which so far seems to be generally the case), Cmds can be called in the same process as test code. Only minimal changes to CLI test code would be required, mainly the fixtures.

2. Expose rootCmd's to integration test fixture

• Expose GetRootCmd func's for simd/simcli
• Setup build flags for cli_test_in_process to either use binaries as we currently do -or- use simd/simcli GetRootCmds and run in process
• Adjust cli test fixture accordingly

3. Make every cli test suite a testify Suite that can be re-used by other apps

Each CLI test file should be restructured into a Suite that takes a config object. The config object will define how tests are run and will allow other apps building on the SDK to inject their own app config's and run the same suites for their apps.

4. Run all CLI tests in the SDK from a single simapp/cli_test.go file

This single simapp/cli_test.go file should import the suites from each module, populate them with the simapp config and run them one by one. This file would serve as an example for other apps as to how to setup their CLI tests.

---

For Admin Use

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

[sahith-narahari]

I feel using a testify Suite would be better and clean. This should also help us eliminate cli_tests from gaia and other applications using sdk.
Any thoughts @anilcse

[anilcse]

Thanks @aaronc for the proposal. I had a feeling that in-process might not work best for cli tests but looks easy with the mentined approaches.

I vote for proposal 4 and 3. Especially 4 looks clean and simple. 2 looks flexible as it allows to chose binaries or GetRootCmd to run in-process but I don't see any advantage with that.

[aaronc]

Actually @anilcse, these aren't separate proposals as it's currently written, they're just steps towards implementing. Maybe I shouldn't have numbered them and that makes it more confusing...

[alexanderbez]

@aaronc this does not belong in 0.39.

[aaronc]

I know this is somewhat involved @alexanderbez but we may want to consider this as part of battle testing 0.39. If we have accurate coverage metrics of client-facing code, we can improve coverage and potentially prevent bugs from getting into the release. (I would note that we only need to do steps 1 & 2 to get this benefit...)

[alessio]

I support this (this may involve forking spf13/cobra though)

[alexanderbez]

Assigning this to me and @alessio. I have ideas in my head on how to approach this. I will take a stab at a PoC over the coming days. However, I will take this on full-time after we release 0.39.

[sahith-narahari]

Sharing here for future reference,

1. https://github.com/urfave/cli
2. https://github.com/mitchellh/cli