Interchaintest for e2e tests #174
Conversation
agouin
force-pushed
the
andrew/interchaintest
branch
from
119abcd
to
7a23d43
Compare
agouin
force-pushed
the
andrew/interchaintest
branch
from
c34d035
to
db8ebd7
Compare
| go 1.20 | ||
|
|
||
| use ( | ||
| . | ||
| ./test | ||
| ) |
There was a problem hiding this comment.
I think the Go workspace is to support the separate go.mod under the test directory. Why not use 1 go.mod for the whole project?
There was a problem hiding this comment.
To avoid the interchaintest and it's dependencies as dependencies for the horcrux binary. This minimizes required deps to download for make install for docker and release, for example.
There was a problem hiding this comment.
This is not how Go modules work. The compiler builds a dependency graph and is smart enough to only download dependencies used for the binary. Because the test package is (rightfully) never used in horcrux's main(); test's dependencies are ignored.
I deleted the go.work and test/go.mod andI tested on a separate branch.
As you can see, no interchaintest modules were downloaded.
❯ make install
go: downloading github.com/cosmos/cosmos-sdk v0.47.3
go: downloading google.golang.org/grpc v1.55.0
go: downloading golang.org/x/crypto v0.8.0
go: downloading github.com/cosmos/gogoproto v1.4.10
go: downloading github.com/Jille/raft-grpc-transport v1.4.0
go: downloading github.com/hashicorp/raft v1.5.0
go: downloading github.com/ecies/go/v2 v2.0.6
go: downloading golang.org/x/net v0.9.0
go: downloading google.golang.org/genproto v0.0.0-20230306155012-7f2fa6fef1f4
go: downloading go.etcd.io/bbolt v1.3.7
go: downloading github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0
go: downloading github.com/ethereum/go-ethereum v1.11.5
go: downloading github.com/hashicorp/go-hclog v1.5.0
go: downloading golang.org/x/exp v0.0.0-20230515195305-f3d0a9c9a5cc
go: downloading github.com/fatih/color v1.13.0
go: downloading cosmossdk.io/math v1.0.1
go: downloading github.com/hdevalence/ed25519consensus v0.1.0
go: downloading github.com/cosmos/cosmos-proto v1.0.0-beta.2
go: downloading filippo.io/edwards25519 v1.0.0
However, I'm fine with this decision. I think documentation pointing out this decision for a go.work is useful for people unfamiliar with the codebase, though.
On my branch, I ran into a weird go mod tidy error and punted on it further:
❯ go mod tidy
go: github.com/gogo/protobuf@v1.3.3: reading github.com/gogo/protobuf/go.mod at revision v1.3.3: unknown revision v1.3.3
But, in theory, only one go.mod should be fine and shouldn't impact performance or maintenance.
The one caveat is if horcrux main() and test share transitive deps. Then one go.mod will use MVS which may have unintended consequences, which may be the problem with gogo/protobuf package above.
There was a problem hiding this comment.
The protobuf is due to the replace statement in interchaintest. We would need to copy all of the replaces also which are currently only in the test/go.mod.
Good to know about the dependency graph. I thought we would have a performance impact here since it prefetches go deps for docker caching, but I could be wrong.
There was a problem hiding this comment.
Oh yes, it might fetch all in that case.
There was a problem hiding this comment.
But that step may not be necessary since go build downloads deps anyway. But we may be trying to optimize docker builds in some fashion.
test/validator.go
Outdated
|
|
||
| // chainWrapper holds the initial configuration for a chain to start from genesis. | ||
| type chainWrapper struct { | ||
| chain **cosmos.CosmosChain |
There was a problem hiding this comment.
The double star (pointer to a pointer) is uncommon. Have you tried using the chainWrapper strust as a pointer when callers use it? This may eliminate the double pointer.
There was a problem hiding this comment.
The double star removed the need for double wrapped functions for pre-genesis, but I updated it so that it's the single pointer.
All of the e2e tests were running as part of CI, but on a single job on the external runner, which made figuring out which specific test was failing more difficult. |
ports e2e framework to use interchaintest.
use test matrix in CI, removing need for self hosted test runner.
improves test coverage by using slashing period of 10 blocks, 90% uptime required, and reducing number of validators in test from 4 to 2, which requires the horcrux validator for consensus.