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-unittakes 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
- [ ] Add a make job to compile all tests without running them. #7364, ...
- [x] Make sure tests are cached (currently they are not if we use
make) #7314 - [ ] Use
testing.T.Skipinstead of build flags whenever possible to skip some tests. - [ ] Make sure unit tests are proper unit tests.
- [ ] Use test suites more extensively (this will also help with tests separation).
- [ ] Improve test output summary (@alessio working on it)
- [ ] Limit amount of logs in tests https://github.com/cosmos/cosmos-sdk/issues/7257
For Admin Use
- [x] Not duplicate issue
- [x] Appropriate labels applied
- [ ] Appropriate contributors tagged
- [ ] Contributor assigned/self-assigned
robert-zaremba
All 21 comments
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
on 22 Sep 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
on 22 Sep 2020
Add a make job to compile all tests (without running)
That is already a thing: make test-all
alessio
on 22 Sep 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?
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.
robert-zaremba
on 22 Sep 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?
alessio
on 22 Sep 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).
robert-zaremba
on 22 Sep 2020
How do you suggest splitting unit, integration and e2e tests?
marbar3778
on 22 Sep 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
@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
on 22 Sep 2020
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.
robert-zaremba
on 22 Sep 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?
alessio
on 23 Sep 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.
robert-zaremba
on 23 Sep 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?
alessio
on 23 Sep 2020
@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.
robert-zaremba
on 23 Sep 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.
alessio
on 28 Sep 2020
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
on 28 Sep 2020
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.
robert-zaremba
on 28 Sep 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).
alessio
on 28 Sep 2020
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.
aaronc
on 30 Sep 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.
alessio
on 30 Sep 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.
aaronc
on 30 Sep 2020
Related issues
kevlubkcm
路
3Comments
rigelrozanski
路
3Comments
ValarDragon
路
3Comments
fedekunze
路
3Comments
cwgoes
路
3Comments
Most helpful comment
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.