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

Fix Wasm E2E Tests #6660

Merged
merged 5 commits into from
Jun 24, 2024
Merged

Fix Wasm E2E Tests #6660

merged 5 commits into from
Jun 24, 2024

Conversation

chatton
Copy link
Contributor

@chatton chatton commented Jun 20, 2024

Description

Marking as R4R to trigger E2Es.

This PR fixes the E2E wasm tests (with the exclusion of the MsgTransfer tests which fail due to ics20-v2)

The Setup logic is moved into SetupSuite and SetupTest, and the ConfigureRelayer fn is finally removed 🥳

I added some additional log lines as debugging the hyperspace relayer was quite difficult due to it hanging rather than erroring.


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.

  • Targeted PR against the correct branch (see CONTRIBUTING.md).
  • Linked to GitHub issue with discussion and accepted design, OR link to spec that describes this work.
  • Code follows the module structure standards and Go style guide.
  • Wrote unit and integration tests.
  • Updated relevant documentation (docs/).
  • Added relevant godoc comments.
  • Provide a conventional commit message to follow the repository standards.
  • Include a descriptive changelog entry when appropriate. This may be left to the discretion of the PR reviewers. (e.g. chores should be omitted from changelog)
  • Re-reviewed Files changed in the GitHub PR explorer.
  • Review SonarCloud Report in the comment section below once CI passes.

Copy link
Member

@damiannolan damiannolan left a comment

Choose a reason for hiding this comment

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

Approving, waiting for CI

@damiannolan
Copy link
Member

damiannolan commented Jun 20, 2024

image

Been looking at failing wasm e2es with @chatton. It looks to be the merkle path being encoded as base 64 bytes when the SudoMsg is passed to wasm. This is due to #6633 and #6644 PRs

@chatton
Copy link
Contributor Author

chatton commented Jun 20, 2024

image

Been looking at failing wasm e2es with @chatton. It looks to be the merkle path being encoded as base 64 bytes when the SudoMsg is passed to wasm. This is due to #6633 and #6644 PRs

when running tests locally with older images these tests pass. We'll need to decide how to proceed with this breaking change

@DimitrisJim
Copy link
Contributor

oh, this is a contract api breaking change 👀

@damiannolan
Copy link
Member

oh, this is a contract api breaking change 👀

An attempt was made. 733539c

Copy link

sonarcloud bot commented Jun 24, 2024

@damiannolan
Copy link
Member

Can we merge this so I can run tests on #6644? or is there anything else left to do?

@chatton
Copy link
Contributor Author

chatton commented Jun 24, 2024

@damiannolan think we can merge when ready, and can look into any failures after your PR gets merged after the fact

@chatton chatton added the priority PRs that need prompt reviews label Jun 24, 2024
@chatton chatton enabled auto-merge June 24, 2024 10:05
@chatton chatton added this pull request to the merge queue Jun 24, 2024
Merged via the queue into main with commit 62e51f2 Jun 24, 2024
88 of 94 checks passed
@chatton chatton deleted the cian/fix-wasm-tests branch June 24, 2024 14:41
bznein pushed a commit that referenced this pull request Jun 26, 2024
* wip: fixing wasm tests

* chore: fix wasm tests

* chore: removed excess log lines

---------

Co-authored-by: Damian Nolan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority PRs that need prompt reviews
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants