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

Update substrate integration test environment #931

Merged

Conversation

xermicus
Copy link
Contributor

@xermicus xermicus commented Jul 20, 2022

This PR upgrades the Substrate integration tests to work with an up-to-date Substrate runtime. The individual commits resemble more or less the steps needed to make this work. In summary:

  • Switched to the pullparitytech/contracts-ci-linux:production image for having a node that will always be up-to-date
  • Bump many npm deps in the testing frameworks
  • Fix all test specs so the work again with the new runtime and updated deps
  • Disable discovered regressions to make the GHA job pass

The regressions are most likely all related to issue #666. As discussed in today's stand up call, the plan is the following:

  1. [In this PR] Update the integration test suite to work with current Substrate runtime, disabling any regressions related to runtime changes. This will let us continue to have a green CI setup on main. Based on that, a note in our README.md transparently informs what is not working and that we are aware.
  2. Priority is on fixing issue Cross contract passes wrong AccountId to seal_call on substrate #666 (and other potential metadata issues that may be discovered). Ideally this fixes all regressions.
  3. Re-enable all tests that were disabled in this PR.

@xermicus xermicus requested a review from seanyoung as a code owner July 20, 2022 16:00
@xermicus xermicus requested a review from LucasSte July 20, 2022 16:01
@LucasSte
Copy link
Contributor

LucasSte commented Jul 20, 2022

If this is still a work in progress, you should mark your PR as a draft and not request a review.

@xermicus xermicus marked this pull request as draft July 20, 2022 17:27
@xermicus xermicus marked this pull request as ready for review July 21, 2022 22:40
@xermicus
Copy link
Contributor Author

@athei maybe you want to have a quick look as well

@xermicus xermicus changed the title [WIP] Update substrate integration test environment Update substrate integration test environment Jul 21, 2022
Copy link
Contributor

@seanyoung seanyoung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, this is already a big improvement (note the typo)

There is a Substrate documentation section in docs/targets/substrate.rst which is also out of date.

README.md Outdated Show resolved Hide resolved
Copy link

@athei athei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

integration/substrate/issue666.spec.ts Outdated Show resolved Hide resolved
integration/substrate/index.ts Show resolved Hide resolved
@xermicus xermicus force-pushed the update-substrate-integration-tests branch from 3486010 to 29b0654 Compare July 22, 2022 10:58
@seanyoung
Copy link
Contributor

@xermicus please squash merge or squash your commits into a single commit before merging

Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
…or a single contract deploy

Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
…lang#666 so we can enable them again afterwards

Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
Signed-off-by: Cyrill Leutwiler <[email protected]>
@xermicus xermicus force-pushed the update-substrate-integration-tests branch from 421c0c4 to f5c4dd3 Compare July 22, 2022 14:58
@xermicus
Copy link
Contributor Author

@athei FYI I updated the issue666 test spec to reproduce the exact same issue that is described there (expecting it to fail so CI is green).

@athei
Copy link

athei commented Jul 22, 2022

@athei FYI I updated the issue666 test spec to reproduce the exact same issue that is described there (expecting it to fail so CI is green).

Nice. Thanks :)

@xermicus xermicus merged commit 3c47c21 into hyperledger-solang:main Jul 22, 2022
@xermicus xermicus added this to the `ink! v4` compatibility milestone Oct 5, 2022
@xermicus xermicus deleted the update-substrate-integration-tests branch May 15, 2023 10:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants