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

Test: Fix polkadot-decoding smoke test #2972

Merged
merged 12 commits into from
Oct 1, 2024

Conversation

pLabarta
Copy link
Contributor

What does it do?

This PR adds a retry for query responses which do not contain entries. From some experimentation, it seems that for the accounts::assets data, random storage keys with high starting values such as f seem to be out of bounds for the existing records and fail when trying to read the first entry of the response.

There are also changes to improve reproducing failing cases, mainly to avoid running the whole test and just try to query the module using the provided key override.

What important points reviewers should know?

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

What value does it bring to the blockchain users?

@pLabarta pLabarta added B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited) labels Sep 23, 2024
Copy link
Contributor

github-actions bot commented Sep 23, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2188 KB (no changes) ✅

Moonbeam runtime: 2148 KB (no changes) ✅

Moonriver runtime: 2148 KB (no changes) ✅

Compared to latest release (runtime-3200)

Moonbase runtime: 2188 KB (+228 KB compared to latest release) ⚠️

Moonbeam runtime: 2148 KB (+224 KB compared to latest release) ⚠️

Moonriver runtime: 2148 KB (+224 KB compared to latest release) ⚠️

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Coverage Report

@@                         Coverage Diff                          @@
##           master   pablo/test/smoke/improve-decoding     +/-   ##
====================================================================
  Coverage   79.23%                              79.23%   0.00%     
  Files         299                                 299             
  Lines       87602                               87602             
====================================================================
  Hits        69405                               69405             
  Misses      18197                               18197             
Files Changed Coverage

Coverage generated Tue Oct 1 17:34:04 UTC 2024

@pLabarta pLabarta marked this pull request as draft September 27, 2024 12:49
@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Sep 30, 2024
Copy link
Contributor

@RomarQ RomarQ left a comment

Choose a reason for hiding this comment

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

The changes are not correct

test/suites/smoke/test-polkadot-decoding.ts Outdated Show resolved Hide resolved
@pLabarta pLabarta marked this pull request as ready for review October 1, 2024 18:32
@pLabarta pLabarta requested a review from RomarQ October 1, 2024 18:32
@RomarQ RomarQ merged commit df50df4 into master Oct 1, 2024
39 checks passed
@RomarQ RomarQ deleted the pablo/test/smoke/improve-decoding branch October 1, 2024 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A8-mergeoncegreen Pull request is reviewed well. B0-silent Changes should not be mentioned in any release notes D2-notlive PR doesn't change runtime code (so can't be audited)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants