-
Notifications
You must be signed in to change notification settings - Fork 639
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
feat(test): adding support for backwards compatibility testing #1912
feat(test): adding support for backwards compatibility testing #1912
Conversation
e2e/go.mod
Outdated
) | ||
|
||
replace github.com/strangelove-ventures/ibctest => ../../forks/ibctest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must be removed before the PR can be merged.
https://github.com/cosmos/ibc-go into cian/issue#1911-support-backwards-compatibility-tests
Codecov Report
@@ Coverage Diff @@
## main #1912 +/- ##
==========================================
- Coverage 80.12% 80.09% -0.04%
==========================================
Files 167 167
Lines 11766 11773 +7
==========================================
+ Hits 9428 9429 +1
- Misses 1923 1929 +6
Partials 415 415
|
https://github.com/cosmos/ibc-go into cian/issue#1911-support-backwards-compatibility-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wahoo!!
rlyTag, ok := os.LookupEnv(GoRelayerTag) | ||
chainBSimdTag, ok := os.LookupEnv(ChainBSimdTagEnv) | ||
if !ok { | ||
chainBSimdTag = chainASimdTag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we document the default behaviour somewhere? That is when the chainB image/tag isn't specified. It might be a little odd if the image is specified but that tag isn't? or vice versa?
// getGithubActionMatrixForTests returns a json string representing the contents that should go in the matrix | ||
// field in a github action workflow. This string can be used with `fromJSON(str)` to dynamically build | ||
// the workflow matrix to include all E2E tests under the e2eRootDirectory directory. | ||
func getGithubActionMatrixForTests(e2eRootDirectory string) (GithubActionTestMatrix, error) { | ||
func getGithubActionMatrixForTests(e2eRootDirectory, suite string) (GithubActionTestMatrix, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add godoc to specify the usage of an empty string to indicate runAllTests?
.github/workflows/e2e-manual.yaml
Outdated
name: Manual E2E | ||
on: | ||
workflow_dispatch: | ||
inputs: | ||
test-suite: | ||
description: 'The Test Suite To Run' | ||
required: true | ||
type: string | ||
default: "" | ||
chain-a-image: | ||
description: 'The image to use for chain A' | ||
required: true | ||
type: string | ||
default: "ghcr.io/cosmos/ibc-go-simd-e2e" | ||
chain-a-tag: | ||
description: 'The tag to use for chain A' | ||
required: true | ||
type: string | ||
default: "main" | ||
chain-b-image: | ||
description: 'The image to use for chain B' | ||
required: true | ||
type: string | ||
default: "ghcr.io/cosmos/ibc-go-simd" | ||
chain-b-tag: | ||
description: 'The tag to use for chain B' | ||
required: true | ||
default: "v4.0.0-rc2" | ||
type: string | ||
relayer-tag: | ||
description: 'The tag to use for the relayer' | ||
required: true | ||
default: "v2.0.0-rc2" | ||
type: string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice! Is the idea to add multiple files, one for each previous version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the way inputs
work is that you can specify a value when you run the workflow (vi UI or using a cli tool like gh
). This one workflow should let us run any test suite using any combination of versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it. So we would run this workflow manually rather than it being run on each pr. That makes sense. Mostly to save execution time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes this is a manual task, we can run the permutations we care about when we are doing QA. I think it might be overkill to run all these permutations on every PR.
We can also pick and choose which test suites are important for which versions, I.e. no fee middleware on v2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. Could you add instructions on how to run manually? I'd like to test it out myself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colin-axner I added instructions on how to execute these tests with gh
. Once they are merged into main
, we will be able to use the UI to trigger the workflows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a sample run here for reference: https://github.com/cosmos/ibc-go/runs/7766177908?check_suite_focus=true
https://github.com/cosmos/ibc-go into cian/issue#1911-support-backwards-compatibility-tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! This is so cool! I am very excited :)
.github/workflows/e2e-manual.yaml
Outdated
type: choice | ||
options: | ||
- v4.0.0-rc3 | ||
- v3.0.0-rc2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- v3.0.0-rc2 | |
- v3.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we get v3.1, v2.3 etc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to start with I just added a few basic entries. This are just images that exist already. All this list is doing is giving us a drop down menu. We can edit it as we see fit! We just need to make sure the images exist.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome job
options: | ||
- main | ||
- v4.0.0-rc3 | ||
- v3.0.0 | ||
- v2.2.0 | ||
- v2.1.0 | ||
- v2.0.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we missing v3.1.0
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we are, but that image hasn't actually been built yet. That will have to come in a follow up!
Description
This PR adds support for backwards compatibility testing in the form of allowing us to specify the images to be used for chain A and chain B by running a
workflow_dispatch
workflow via the Github UI or a tool like ghE.g. we can run
FeeMiddlewareTestSuite
with chain A onmain
and chain B onv4.0.0-r2
We will have UI access to this workflow once it is merged to main, in order to test it, the best option is to use
gh
. You can test it with this command once you have gh installed.Changing values as appropriate.
closes: #1911
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/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerCodecov Report
in the comment section below once CI passes