-
Notifications
You must be signed in to change notification settings - Fork 358
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
add unlock democracy funds TS test #2673
Conversation
"0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000" + | ||
"00000000000000000000000000", | ||
}, | ||
]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move theses raw storage definitions to a json file or generate them via a ts script
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is data scrapped from Moonbeam and I'd like to keep this one. I'll move it in a json file.
I suppose the right place is next to .ts
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@timbrinded do you have a strong opinion on where to put this json file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might have to do it a .ts
file in order to please the max line length requirement.
Unless there is another doing it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@librelois not with the tests but with the other test data files in /helpers
Another example of this would be /helpers/modexp.ts
which gives the expected results for modexp precompile testing
@links234 feel free to use annotation // editorconfig-checker-disable-file
on this expected test results file, since raw data is always going to be verbose
test/suites/dev/moonbase/test-moonbeam-lazy-migrations/test-unlock-democracy-funds.ts
Outdated
Show resolved
Hide resolved
} | ||
} | ||
|
||
const tx2 = await context.createBlock( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each check should be on a separate it
block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Hopefully I understood correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've asked @timbrinded and since this is a "complex" test where we need to do multiple sequential things, it should be in 1 single test.
However, I did extract the one checking the upper limit.
test/suites/dev/moonbase/test-moonbeam-lazy-migrations/test-unlock-democracy-funds.ts
Outdated
Show resolved
Hide resolved
test/suites/dev/moonbase/test-moonbeam-lazy-migrations/test-unlock-democracy-funds.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider changing this to a TS file like this:
https://github.com/moonbeam-foundation/moonbeam/blob/master/test/helpers/modexp.ts
As it'll be easier to maintain and reason about than JSON. Namely you'll be able to use comments, format it, get type inference, import it nicely.
What does it do?
Adds E2E TS test for unlock democracy funds lazy migration added in PR 2651.
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?