Skip to content
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

Create importable workflow for chains to run upgrade tests #2501

Closed
chatton opened this issue Oct 10, 2022 · 2 comments · Fixed by #3155
Closed

Create importable workflow for chains to run upgrade tests #2501

chatton opened this issue Oct 10, 2022 · 2 comments · Fixed by #3155
Assignees
Labels

Comments

@chatton
Copy link
Contributor

chatton commented Oct 10, 2022

Summary

To make it as easy as possible for chains to perform upgrade tests, we should provide a github action which can be easily imported and used from any Github Workflow.

Proposal

Create a workflow_call type workflow in ibc-go.

In order to create a re-usable workflow, we can use a workflow_call. This special type of workflow is essentially importable into any github action. See this example of how to define one.

In order to use a workflow_call, this syntax can be used.

Note: the inputs defined are specified under the with object from the calling workflow.

Workflows can be called from other repos with this syntax (taken from the github docs)

jobs:
  call-workflow-passing-data:
    uses: octo-org/example-repo/.github/workflows/reusable-workflow.yml@main
    with:
      config-path: .github/labeler.yml
    secrets: inherit

In order for the workflow to actually trigger an upgrade test, we will need to do the following

  1. Checkout the ibc-go repo at a specific tag
  2. Cd into the e2e directory
  3. Run the upgrade test

We currently do this here

These values will be required when an arbitrary chain is being used.

The e2e tests look for these env vars when running tests.

In terms of our exported workflow, we could do in in a few different ways. We could have the workflow run a single test (i.e. not use a strategy.matrix ) or we could use a matrix to run all of the tests.

I would lean slightly in favour of not using a matrix as this gives more flexibility to the caller to run only the tests that are relevant to them.

@chatton
Copy link
Contributor Author

chatton commented Jan 25, 2023

After discussion with @damiannolan

  • We can't make any assertions on application specific post upgrade logic, so we can limit the upgrade test to core ibc functionality (we can do transfers before and after the upgrade) .
  • The current test TestV4ToV5ChainUpgrade looks like it almost already does exactly this. We can rename this to something generic like TestIBCUpgrade.
  • We need to make the upgrade plan an argument, currently it is set to normal upgrade. This can be done with an env var in a workflow.
  • We should be able to re-use the existing workflow call and just provide examples of how to call this specific test rather than a new workflow.
  • We can include yaml samples in the e2e readme.

@chatton
Copy link
Contributor Author

chatton commented Feb 15, 2023

In addition to a basic ibc upgrade test, we should also add a single chain upgrade to test migrations as a bare minimum. This can always be expanded upon afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants