Simplify E2E Configuration #7568
Conversation
chatton
requested review from
AdityaSripal,
damiannolan,
DimitrisJim,
gjermundgaraba and
bznein
as code owners
| image: ghcr.io/cosmos/ibc-go-simd # override with CHAIN_IMAGE | ||
| binary: simd # override with CHAIN_BINARY | ||
|
|
||
| activeRelayer: hermes # override with RELAYER_ID |
There was a problem hiding this comment.
I assume this has to be one of the IDs in the list below, maybe worth specifying it in the comment?
| if strings.TrimSpace(upgrade.Tag) == "" { | ||
| return fmt.Errorf("upgrade config missing tag: %+v", upgrade) | ||
| } | ||
|
|
||
| if strings.TrimSpace(upgrade.PlanName) == "" { | ||
| return fmt.Errorf("upgrade config missing plan name: %+v", upgrade) | ||
| } |
There was a problem hiding this comment.
[nit] I wonder if it would make more sense to either:
- do this check only if
upgrade.PlanName == tc.UpgradePlanName(so a "broken" upgrade config is allowed if it's not used for the test) - split this into two different loops. Since we don't care about efficiency here, I think it's a bit more clear if validation and search are split
There was a problem hiding this comment.
I figured it would be best to just explode immediately if anything about config is bad. Don't have a strong preference for multiple loops, will always just be a few elements anyway.
There was a problem hiding this comment.
Up to you! I am slightly more inclined towards having one loop of validation followed by one of search, for clarity, but not a deal breaker
| if upgrade.PlanName == tc.UpgradePlanName { | ||
| foundPlan = true | ||
| } |
There was a problem hiding this comment.
[nit] if we go for either 1 or 2 in the above comment, we can return nil here.
Or even:
if we go with 1 above: use slices.IndexFunc() to get the index of the right upgrade plan, and then do the validation and return
if we go with 2 above: simply return slices.ContainsFunc()
|
Description
This PR modifies the way configurations are passed to E2E tests.
While every field can still be overridden by environment variables, the E2Es now have their own config file that is used, and it is possible to remove some special casing around tests being in CI vs running locally.
The TestConfig struct has been updated to behave in a similar way as the relayers, in that we provide a list of all upgrade plans, then just specify which one we want by name.
closes: #4697
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/).godoccomments.Files changedin the GitHub PR explorer.SonarCloud Reportin the comment section below once CI passes.