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

Chopsticks tests for XCM #6712

Open
franciscoaguirre opened this issue Nov 29, 2024 · 9 comments
Open

Chopsticks tests for XCM #6712

franciscoaguirre opened this issue Nov 29, 2024 · 9 comments
Assignees
Labels
T6-XCM This PR/Issue is related to XCM. T10-tests This PR/Issue is related to tests.

Comments

@franciscoaguirre
Copy link
Contributor

Right now, most XCM tests are using the xcm-emulator, a tool that does the job but lacks a couple of good features:

  • Test with on-chain data
  • Ability to write tests in typescript instead of rust
    A tool that has both of this is chopsticks.

We want to add tests with chopsticks to use real on-chain data.
There's already a repo aimed at having chopsticks tests for XCM interactions between all chains in the ecosystem: https://github.com/open-web3-stack/polkadot-ecosystem-tests exists.
Should we do this tests on that other repo? Or here on polkadot-sdk?

Option 1: We do everything in polkadot-ecosystem-tests

The benefit of this approach is that we don't need to invest in creating our own test setup and we contribute our tests back to the ecosystem for them to be used by everyone.

The downside is that adding the tests to CI is more complicated, since we're dealing with another repository owned by a different organization.
The development flow might look like this:

  • Create a PR, with number n.
  • Create a branch in polkadot-ecosystem-tests called "polkadot-sdk-n".
  • CI can find the correct branch in polkadot-ecosystem-tests, build the runtime for the PR and use it to run the tests.
  • (while there's no CI) The reviewer can validate the tests by cloning polkadot-ecosystem-tests and running them locally.
  • When merged, the branch in polkadot-ecosystem-tests can be merged.

This reminds me of the "companion pr" system we had before moving to the monorepo.

Option 2: We do everything locally

The downside of this approach is we have to create our own setup and we don't contribute to the overall ecosystem (although tests are still all open source).

The good thing is now the development flow can look like this:

  • Create a PR, with number n.
  • Add tests to it.
  • CI can build the runtime for the PR and use it to run the local chopsticks tests.
  • (while there's no CI) The reviewer can validate the tests by cloning the repo and running them locally.
  • When merged, it's done.

Option 3: We have a local setup but also contribute to polkadot-ecosystem-tests

This option implies keeping local tests which means having a local setup and a better development flow, but also contributing to polkadot-ecosystem-tests.
What tests would we contribute? All tests of features that have already been released.
This marks a clear distinction between local setup for development but polkadot-ecosystem-tests for ecosystem-wide regression tests.

When there's a release, we could remove the local tests of that release and migrate them to polkadot-ecosystem-tests.

I think we should go for option 3 but wanted to have more discussion and ideas around the topic.

@franciscoaguirre franciscoaguirre added T6-XCM This PR/Issue is related to XCM. T10-tests This PR/Issue is related to tests. labels Nov 29, 2024
@franciscoaguirre franciscoaguirre self-assigned this Nov 29, 2024
@franciscoaguirre
Copy link
Contributor Author

cc @bkchr @acatangiu

@xlc
Copy link
Contributor

xlc commented Nov 30, 2024

I also prefer option 3. Maybe a PR with new tests, and we can have some special comment in the PR to make the CI run the tests with wasm override fetched from some location specified in the PR description and another bot comment to trigger test run.
e.g. the PR description could be

[usual PR description]

/bot set-env POLKADOT_WASM=https://some-url/some-branch/polkadot.wasm

and a comment of /bot run will trigger a test run with polkadot wasm fetched from the provided URL.
we will need CI to build and publish the wasm to somewhere

and when such feature is live, we can merge the PR

@bkchr
Copy link
Member

bkchr commented Nov 30, 2024

Yeah option 3 sounds sensible. The upstreaming then could probably be mainly automated.

@xlc i don't get your comment on why we need the remote override?

@xlc
Copy link
Contributor

xlc commented Nov 30, 2024

to specify which wasm to run the tests. the other two options will be included the wasm in PR, which is tedious to update, or use naming conventions to find it somewhere, which is error-prone

@bkchr
Copy link
Member

bkchr commented Nov 30, 2024

These tests would be run as part of the CI and we build the runtimes directly in CI, so not sure why we want to test some extra runtime?

@xlc
Copy link
Contributor

xlc commented Dec 1, 2024

I may have misunderstood how exactly options 3 is proposing. does the local setup means coping the testing framework in this repo? what I was suggesting is fork the polkadot-ecosystem-tests and write tests there and because the tests and the runtimes are not in a same place, we need some mechanism to specify the runtime

@bkchr
Copy link
Member

bkchr commented Dec 1, 2024

I would have thought we continue to use polkadot-ecosystem-tests, but introduce some way to specify extra tests. These extra tests would live in this repository until they are part of a release and can be upstreamed. Basically we would just merge the tests. But yeah while writing this, maybe not such a great idea because broken tests could not be fixed that easily.

@franciscoaguirre
Copy link
Contributor Author

franciscoaguirre commented Dec 4, 2024

My original idea was more in line with what Basti said. We create tests in polkadot-sdk until they are part of a release and then we upstream them to polkadot-ecosystem-tests.

My idea is the following:

  • Dev creates a PR
  • He also adds chopsticks tests (in the same repo, polkadot-sdk) to the PR, testing the new functionality
  • CI runs the tests by building the runtime with the code of that branch
  • PR is merged
  • When the feature is released the test can be removed and upstreamed to polkadot-ecosystem-tests

@xlc
Copy link
Contributor

xlc commented Dec 4, 2024

I see. One thing to keep in mind: polkadot-ecosystem-tests are expecting live chains, and the live runtimes are live on the fellowship runtimes repo. We can add some tests for westend networks and then it should work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM. T10-tests This PR/Issue is related to tests.
Projects
None yet
Development

No branches or pull requests

3 participants