Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve testing DevX #7362

Closed
3 of 11 tasks
robert-zaremba opened this issue Sep 22, 2020 · 25 comments
Closed
3 of 11 tasks

Improve testing DevX #7362

robert-zaremba opened this issue Sep 22, 2020 · 25 comments
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code) T: Tests tooling dev tooling within the sdk

Comments

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented Sep 22, 2020

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

  • This is a meta-issue.
  • If you have a quick question - please make a comment.
  • Feel free to add a proposal (prefer to do it through a comment)
  • Don't discuss details - this we can do in a sub-issue.

Problem Definition

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

  • make test-unit takes loooot of time, and it's more than unit tests
  • We should be able to compile everything (currently we need to be careful to remember to put all build flags to test-unit job).
  • We don't have single test command to build and tests everything (ALL files).

Proposal


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@robert-zaremba robert-zaremba added T: Dev UX UX for SDK developers (i.e. how to call our code) T: Tests tooling dev tooling within the sdk labels Sep 22, 2020
@alessio
Copy link
Contributor

alessio commented Sep 22, 2020

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
Copy link
Contributor

alessio commented Sep 22, 2020

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
Copy link
Contributor

alessio commented Sep 22, 2020

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

That is already a thing: make test-all

@robert-zaremba
Copy link
Collaborator Author

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
Copy link
Contributor

alessio commented Sep 22, 2020

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
Copy link
Collaborator Author

robert-zaremba commented Sep 22, 2020

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
Copy link
Member

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

@alessio
Copy link
Contributor

alessio commented Sep 22, 2020

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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
Copy link
Contributor

alessio commented Sep 22, 2020

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
Copy link
Collaborator Author

robert-zaremba commented Sep 22, 2020

@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:

  • allow to quickly check if things work on various stages:
    • compilation (should be very fast)
    • 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)
    • integrations tests (these can be slower) -- part of breaking down unit tests
  • Use test suites more extensively to better manage tests.

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

@alessio
Copy link
Contributor

alessio commented Sep 23, 2020

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
Copy link
Collaborator Author

@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
Copy link
Contributor

alessio commented Sep 28, 2020

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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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
Copy link
Contributor

alessio commented Sep 28, 2020

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
Copy link
Member

aaronc commented Sep 29, 2020

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

@alessio
Copy link
Contributor

alessio commented Sep 30, 2020

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
Copy link
Member

aaronc commented Sep 30, 2020

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
Copy link
Contributor

I believe this is now obsolete?

@robert-zaremba
Copy link
Collaborator Author

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

@ValarDragon
Copy link
Contributor

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

@robert-zaremba
Copy link
Collaborator Author

@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.

@robert-zaremba robert-zaremba mentioned this issue Sep 8, 2022
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Dev UX UX for SDK developers (i.e. how to call our code) T: Tests tooling dev tooling within the sdk
Projects
None yet
Development

No branches or pull requests

5 participants