search

cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.27k stars 3.63k forks source link

Improve testing DevX #7362

Closed robert-zaremba closed 2 years ago

robert-zaremba commented 4 years ago

Summary

As a developer I want an easy way to run and manage tests. Moreover, I want to have a clear separation of unit tests, integration tests and functional tests. Finally, unit tests must be very quick to run and don't involve lot of dependencies.

NOTE

Problem Definition

Current setup is far from good (in terms of DevX):

Proposal

For Admin Use

alessio commented 4 years ago

make test-unit takes loooot of time, and it's more than unit tests

Do you mean that you'd prefer not to run commands testing via make test-unit?

alessio commented 4 years ago

We should be able to compile everything (currently we need to be careful to remember to put all build flags to test-unit job).

What do you mean by that? The Makefile helps you generating all build flags/tags.

We don't have single test command to build and tests everything (ALL files).

That's easy, it's one-line patch:

alessio@phoenix:~/work/cosmos-sdk$ git diff
diff --git a/Makefile b/Makefile
index aa03819bc..fe418355b 100644
--- a/Makefile
+++ b/Makefile
@@ -74,7 +74,7 @@ ifeq (,$(findstring nostrip,$(COSMOS_BUILD_OPTIONS)))
   BUILD_FLAGS += -trimpath
 endif

-all: tools build lint test
+all: tools build simd lint test-all

 # The below include contains the tools and runsim targets.
 include contrib/devtools/Makefile

We should be able to compile everything (currently we need to be careful to remember to put all build flags to test-unit job).

alessio commented 4 years ago

Add a make job to compile all tests (without running)

That is already a thing: make test-all

robert-zaremba commented 4 years ago

make test-unit takes loooot of time, and it's more than unit tests

Do you mean that you'd prefer not to run commands testing via make test-unit?

No, unit tests should be fast and well encapsulated. Unit test which runs a blockchain and a rest client is not a unit test. There are some tests marked as unit which run 30+ seconds.

alessio commented 4 years ago

The go test command compiles the _test files that typically contain test functions, benchmark functions, and example functions. To some extent, all such tests in Go are unit tests. We even did a lot of work to refactor command line test suites to be in the position to run them as if they were unit tests.

Unit test which runs a blockchain and a rest client is not a unit test

Yes, I know that. I must confess that the whole team knew that all along. We thought that running a single make test command was far quicker and even easier to remember than having separate commands. So you're right, we sacrificed correctness for developer's convenience. Now the question is: if we were to walk back, what value would this new approach bring?

robert-zaremba commented 4 years ago

if we were to walk back, what value would this new approach bring?

Have a look at the Proposal section of OP ;) .

make test run some tests (a mix of unit tests, integration tests etc...). And it's terribly long (make test-unit is 6+ min).

tac0turtle commented 4 years ago

How do you suggest splitting unit, integration and e2e tests?

alessio commented 4 years ago

And it's terribly long (make test-unit is 6+ min).

It might depend on the developer's latitude and longitude (can't tell on that), but...

alessio@phoenix:~/work/cosmos-sdk$ time make test-unit
go test -mod=readonly -json -tags='cgo ledger test_ledger_mock norace' ./... | tparse

+--------+----------+--------------------------------------------------------------------+-------+------+------+------+
| STATUS | ELAPSED  |                              PACKAGE                               | COVER | PASS | FAIL | SKIP |
+--------+----------+--------------------------------------------------------------------+-------+------+------+------+
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client/grpc/reflection                | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/24-host                         | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint/client/rest                    | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/version                               | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth                                | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/types                          | 0.0%  |   27 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/legacy/v0_36           | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/internal/maps                   | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client/tx                             | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/client/rest            | 0.0%  |   29 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/02-client/simulation            | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/rest                            | 0.0%  |   35 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/keeper                 | 0.0%  |   50 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/types                          | 0.0%  |   25 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/cachekv                         | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/server                                | 0.0%  |   15 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc-transfer/simulation             | 0.0%  |    7 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/legacy/v0_36                   | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/keys/multisig                  | 0.0%  |   16 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/vesting                        | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/client/cli             | 0.0%  |   47 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/keyring                        | 0.0%  |   48 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking/types                       | 0.0%  |   40 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1                 | 0.0%  |   25 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/evidence/types                      | 0.0%  |   20 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/simulation                     | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/vesting/client/cli             | 0.0%  |    7 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/internal/proofs                 | 0.0%  |  123 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/types                  | 0.0%  |   11 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/client/rest                    | 0.0%  |   20 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/tracekv                         | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil/legacy/v0_38                | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov                                 | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/codec/types                           | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/simapp/simd/cmd                       | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/keeper                       | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/bech32                          | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/crisis/keeper                       | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/light-clients/solomachine/types | 0.0%  |   78 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/02-client/keeper                | 0.0%  |   54 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/types                        | 0.0%  |   15 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank                                | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/rootmulti                       | 0.0%  |   33 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking/keeper                      | 0.0%  |  107 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client/flags                          | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types                                 | 0.0%  |  210 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/07-tendermint/types             | 0.0%  |  106 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client/keys                           | 0.0%  |   44 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/types                          | 0.0%  |   35 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint/types                          | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/types/proposal               | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params                              | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/codec/unknownproto                    | 0.0%  |   39 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/signing                        | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/keeper                         | 0.0%  |   14 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/simulation                      | 0.0%  |   14 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/05-port/keeper                  | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/client/cli                   | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc                                 | 0.0%  |   39 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/capability/simulation               | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/client/rest                  | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil/client/cli                  | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/04-channel/types                | 0.0%  |   29 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint/client/cli                     | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil/legacy/v0_39                | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/simulation                   | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/evidence/simulation                 | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/keeper                          | 0.0%  |   95 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/upgrade/keeper                      | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/server/mock                           | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/legacy/v0_40                   | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client/grpc/simulate                  | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/capability/keeper                   | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/03-connection/simulation        | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking                             | 0.0%  |   33 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/snapshots                             | 0.0%  |   18 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/prefix                          | 0.0%  |   15 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil/legacy/v0_36                | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution                        | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/mem                             | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/transient                       | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/09-localhost/types              | 0.0%  |   33 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/simulation                      | 0.0%  |   16 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc-transfer/types                  | 0.0%  |   14 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/server/grpc                           | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/client/rest                     | 0.0%  |   33 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/client/utils                    | 0.0%  |    7 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing                            | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/simapp                                | 0.0%  |   14 |    0 |    4 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing/client/rest                | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/telemetry                             | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/hd                             | 0.0%  |   12 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/testutil/network                      | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/02-client                       | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking/client/rest                 | 0.0%  |   62 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/client                         | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/keeper                         | 0.0%  |   37 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/upgrade/types                       | 0.0%  |   23 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/errors                          | 0.0%  |   57 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto/ledger                         | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/cache                           | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/23-commitment/types             | 0.0%  |   25 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/client/cli                     | 0.0%  |   22 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/tx                             | 0.0%  |   28 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing/types                      | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/client/rest                    | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/02-client/types                 | 0.0%  |   32 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/types                           | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/client/cli                     | 0.0%  |   15 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing/client/cli                 | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking/client/cli                  | 0.0%  |   83 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil/types                       | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint                                | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing/keeper                     | 0.0%  |   14 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/03-connection/types             | 0.0%  |   16 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc-transfer                        | 0.0%  |   18 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint/keeper                         | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/client                                | 0.0%  |   20 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/baseapp                               | 0.0%  |   66 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/types                           | 0.0%  |   27 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genaccounts/legacy/v0_36            | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/slashing/simulation                 | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/iavl                            | 0.0%  |   14 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/evidence/keeper                     | 0.0%  |   19 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/module                          | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/evidence                            | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/upgrade                             | 0.0%  |   18 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/gaskv                           | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/genutil                             | 0.0%  |   13 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/04-channel/simulation           | 0.0%  |    8 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/types/query                           | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/store/dbadapter                       | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/simulation                      | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/staking/simulation                  | 0.0%  |   17 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/codec                                 | 0.0%  |   18 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/crisis                              | 0.0%  |    7 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc-transfer/keeper                 | 0.0%  |   38 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/crypto                                | 0.0%  |    4 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/legacy/legacytx                | 0.0%  |   19 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/simulation                          | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/crisis/client/cli                   | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/mint/simulation                     | 0.0%  |    6 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/params/client/utils                 | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/ante                           | 0.0%  |  143 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/simulation             | 0.0%  |   21 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/03-connection/keeper            | 0.0%  |  111 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/bank/simulation                     | 0.0%  |   10 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/legacy/v0_40                   | 0.0%  |    1 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/distribution/client/common          | 0.0%  |    5 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/capability/types                    | 0.0%  |    9 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/vesting/types                  | 0.0%  |   28 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/04-channel/keeper               | 0.0%  |  220 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/server/config                         | 0.0%  |    2 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/auth/legacy/v0_38                   | 0.0%  |    7 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/testutil                              | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/ibc/testing/mock                    | 0.0%  |    3 |    0 |    0 |
| PASS   | (cached) | github.com/cosmos/cosmos-sdk/x/gov/client/cli                      | 0.0%  |   50 |    0 |    0 |
+--------+----------+--------------------------------------------------------------------+-------+------+------+------+

real    0m3.893s
user    0m37.479s
sys 0m4.699s
robert-zaremba commented 4 years ago

@alessio - you don't need to copy paste your cached test suite. Try to modify something in /types and run it again. And the try to modify something else in /codec and re-run just to check on which files you need to update / modify.

robert-zaremba commented 4 years ago

How do you suggest splitting unit, integration and e2e tests?

@marbar3778 thanks for asking. I suggest to use https://godoc.org/testing#hdr-Skipping. On top of that we can add our own command line flags (eg -t.integration) and do appropriate check in Test* function when they are not a part of a test suite, or a test suite Setup functions. With test suites we can break tests into multiple categories much easier.

alessio commented 4 years ago

Try to modify something in /types and run it again. And the try to modify something else in /codec and re-run just to check on which files you need to update / modify.

If I modify something in types/*, I expect the changes to be propagated across all the packages that use those packages. Therefore, tests cache is invalidated and tests need to run again. Why? Because this is how tests cache works.

On top of that we can add our own command line flags (eg -t.integration) and do appropriate check in Test* function when they are not a part of a test suite, or a test suite Setup functions. With test suites we can break tests into multiple categories much easier.

This sounds interesting as well as a bit convoluted. Could you prepare a draft PR so that the teams can be in the position to assess the impact of the proposed change please?

robert-zaremba commented 4 years ago

@alessio it seams that I have difficulty to explain you the motivation for wisely splitting unit tests. I know how go test works, and it's clear what invalidates the cache. The whole purpose of this issue is to make a better developer experience:

All this things are written in the proposal. This is a meta issue to collect all improvements proposals.

alessio commented 4 years ago

compilation (should be very fast)

Please don't bother about this. It si already very, very fast. The whole point of the Go compiler is to compile fast. Let's avoid engaging in flights of fancy and solve problems that are not problems at all, shall we?

unit tests -- should be fast (above I was showing that unit tests are slow, and if you do a small change it can have a big impact in the Development Experience, so showing that cached tests are faster doesn't solve the problem)

Yes, unit tests should be fast, you're right and I agree to that. Yet this I was showing that unit tests are slow is not true (see my tests log), and yes - if you make a change in sdk types you expect to invalidate tests cache of mostly everything.

Now, I like using go test flags to differentiate integration tests from unit tests - we actually used to do something similar. I was just asking whether it would be possible to have a tiny patch to show us what you have a mind?

robert-zaremba commented 4 years ago

@alessio - you are showing cached test results. Not relevant. I already explained that few times above.

I was just asking whether it would be possible to have a tiny patch to show us what you have a mind?

Yes, but now I'm busy with other tasks. I have described the idea above.

Again, this is a meta task. let's don't spam here. If you want to contribute or share some advice about one of the proposals, then let's do it in a sub-issue. Thanks.

alessio commented 4 years ago

Using github.com/stretchr/testify/suite everywhere unconditionally is far from a good idea, i.e. it becomes impossible to parallelise simple test cases.

EDIT: As in: test suites can run in parallel, yet test cases of a suite cannot. github.com/stretchr/testify/suite seems far from handling parallelism fully and correctly.

robert-zaremba commented 4 years ago

Ah, that's bad. I'm usually using https://labix.org/gocheck and have even a set of baked checkers: https://github.com/robert-zaremba/checkers

AFAIK gocheck handles parallel tests and has nice support skipping tests out of the box.

robert-zaremba commented 4 years ago

Hmm, I think gocheck has same support as testify.

I think you can mimic parallel tests in testify using t.Run - as described in the sdk testing godoc.

alessio commented 4 years ago

Switching to gocheck wouldn't help. go test natively handes parallel testing.T environments. Problems is that a suite.Suite is just a collection of functions that run within the the same testing.T context (currently impossible to be parallelised by design).

aaronc commented 4 years ago

Here's the relevant issue https://github.com/stretchr/testify/issues/187. Suites could be parallelized if a deep copy was done of the suite for every test. Seems feasible to me.

alessio commented 4 years ago

Suites could be parallelized if a deep copy was done of the suite for every test. Seems feasible to me.

It is, though we would need to introduce some hack-ish, custom code to handle parallelism. So unless test execution times become untenable, I'd wait until upstream introduces this as a feature.

aaronc commented 4 years ago

Suites could be parallelized if a deep copy was done of the suite for every test. Seems feasible to me.

It is, though we would need to introduce some hack-ish, custom code to handle parallelism. So unless test execution times become untenable, I'd wait until upstream introduces this as a feature.

I'm assuming this would need to come from upstream. And we could gently put pressure or introduce a PR if it's an issue for us.

ValarDragon commented 2 years ago

I believe this is now obsolete?

robert-zaremba commented 2 years ago

Test separation is technically not done (there are many of unit tests which are integration tests rather than unit tests).

ValarDragon commented 2 years ago

Want to capture that / a list of places to look out for in a new issue?

robert-zaremba commented 2 years ago

@ValarDragon -- you mean to list "bad tests"? I was talking about it in various events. Instead of having a right test distribution (70% unit tests, 20 % integration, 10% functional: image

Most of our tests are integration - in every module. There is tonnes of copy-paste around, lacking a proper setup nor fixtures repository. At Regen we were experimenting with better setup to enable more managable unit-tests and mocking capabilities. Not sure what to put in a new issue. I think this issue is kind of an epic to have better testing patterns in the SDK.