strangelove-ventures / interchaintest

e2e testing framework for the interchain
https://interchaintest-docs.vercel.app
Apache License 2.0
190 stars 122 forks source link

Configuration should be less confusing. #151

Closed DavidNix closed 1 year ago

DavidNix commented 2 years ago

Problem

I know we've sunk a lot of time into this already, but I still think configuration is confusing.

See https://github.com/DavidNix/ibc-go/pull/1 as an example. I gave this PR to the ibc-go team to help them integrate ibctest into their CICD.

Namely:

cf := ibctest.NewBuiltinChainFactory(logger, []*ibctest.ChainSpec{
        {Name: "gaia", ChainConfig: setup.NewSimappConfig("simapp-a", "chain-a", "atoma")},
        {Name: "gaia", ChainConfig: setup.NewSimappConfig("simapp-b", "chain-b", "atomb")},
    })

ChainSpec and ChainConfig duplicate a lot of the same fields like Name, GasAdjustment, NoHostMount, etc.

In the above they want to test a simapp. I feel there should be a more obvious way to hand over a complete ChainConfig or select a default one we provide, perhaps with optional builder methods to override.

I'm also wondering if we can use the Interchain here as well and perhaps even forgo factories entirely. (That's a large hypothesis though).

DavidNix commented 2 years ago

From cian on ibc-go team:

ah when using a custom chain I get

=== RUN   TestConformance
    reporter.go:226: 
            Error Trace:    test.go:262
            Error:          Received unexpected error:
                            failed to build chain config simapp-a: no chain configuration for simapp-a (available chains are: agoric, gaia, icad, juno, osmosis, penumbra)
            Messages:       failed to get chains

Those ones also default to being in this registry ghcr.io/strangelove-ventures/heighliner Maybe there is still a simpler way of configuring a custom chain?

jtieri commented 2 years ago

I got a custom chain configured using a local docker image and the chains seem to spin up properly with no errors. I agree it's not the most straightforward setup though and there seems to be issues if you try to configure a chain with the denom stake

https://github.com/strangelove-ventures/ibctest/blob/894619c9255d36d6b9055d7edd24d9f38794da14/icq_test.go#L24-L55

DavidNix commented 2 years ago

Given short term feedback, it seems there is more appetite to use as a library vs. the JSON matrix file. I wonder if there are simplifications we can make to the JSON file such that:

Mutually exclusive: 1) Provide name for default config (gaia, penumbra, osmosis, etc.) OR 2) Provide all the config.

I argue it's not that difficult to provide all the config, as ibc-go team's has done.

boojamya commented 2 years ago

there seems to be issues if you try to configure a chain with the denom stake

When the "interchain" is built, each validator is funded with "stake" denom during genesis. This may be the reason for this denom issue.

jonathanpberger commented 1 year ago

Iceboxing for now, but feel free to close this as stale if it's no longer worth doing. cc @DavidNix