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

Tests: add inline snapshots for gas estimation & costs for Randomness #2942

Merged
merged 5 commits into from
Sep 30, 2024

Conversation

pLabarta
Copy link
Contributor

What does it do?

Add a new inline snapshot for estimates gas costs and their estimations for Lottery Demo tests.

What important points reviewers should know?

When running tests without the inline values in place, the tests will generate them and edit the test files.

Is there something left for follow-up PRs?

Vitest has support for storing snapshots in files next to the tests but Moonwall does not support it, it thinks they are other tests and thus fails.

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 D2-notlive PR doesn't change runtime code (so can't be audited) B0-silent Changes should not be mentioned in any release notes labels Sep 11, 2024
Copy link
Contributor

github-actions bot commented Sep 11, 2024

WASM runtime size check:

Compared to target branch

Moonbase runtime: 2196 KB (no changes) ✅

Moonbeam runtime: 2140 KB (no changes) ✅

Moonriver runtime: 2132 KB (no changes) ✅

Compared to latest release (runtime-3200)

Moonbase runtime: 2196 KB (+236 KB compared to latest release) ⚠️

Moonbeam runtime: 2140 KB (+216 KB compared to latest release) ⚠️

Moonriver runtime: 2132 KB (+208 KB compared to latest release) ⚠️

Copy link
Contributor

github-actions bot commented Sep 11, 2024

Coverage Report

@@                      Coverage Diff                      @@
##           master   pablo/randomness-snapshots     +/-   ##
=============================================================
  Coverage   79.22%                       79.22%   0.00%     
  Files         299                          299             
  Lines       87253                        87253             
=============================================================
  Hits        69118                        69118             
  Misses      18135                        18135             
Files Changed Coverage

Coverage generated Fri Sep 27 17:49:09 UTC 2024

@RomarQ
Copy link
Contributor

RomarQ commented Sep 24, 2024

How ready is this?

We should add the snapshots to more tests and commit the snap files.

@pLabarta
Copy link
Contributor Author

Now it should be good to go. I would merge this as it was required particularly for these tests and open an issue to track adding snapshots to (ideally) most of the tests that involve interacting with contracts.

@noandrea noandrea added the A8-mergeoncegreen Pull request is reviewed well. label Sep 30, 2024
@RomarQ RomarQ merged commit 9854dad into master Sep 30, 2024
41 checks passed
@RomarQ RomarQ deleted the pablo/randomness-snapshots branch September 30, 2024 13:30
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