try-runtime-cli: execute-block & create-snapshot tests#14343
try-runtime-cli: execute-block & create-snapshot tests#14343paritytech-processbot[bot] merged 24 commits intoparitytech:masterfrom
execute-block & create-snapshot tests#14343Conversation
execute-block & create-snapshot testsexecute-block & create-snapshot tests
|
@liamaharon Could you review this? |
liamaharon
left a comment
There was a problem hiding this comment.
Looking good, thanks for this!
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
Co-authored-by: Liam Aharon <liam.aharon@hotmail.com>
| Command::new(cargo_bin("substrate")) | ||
| .stdout(process::Stdio::piped()) | ||
| .stderr(process::Stdio::piped()) | ||
| .args(&["try-runtime", "--runtime=existing"]) |
There was a problem hiding this comment.
FYI, the need for --runtime for this subcommand was a fundamental mistake that I added at some point @liamaharon 🙈
There was a problem hiding this comment.
wdyt about existing always being a default?
| .stderr(process::Stdio::piped()) | ||
| .args(&["try-runtime", "--runtime=existing"]) | ||
| .args(&["execute-block"]) | ||
| .args(&["live", format!("--uri={}", ws_url).as_str()]) |
There was a problem hiding this comment.
This will only execute the latest block. Would be interesting if you also think about incorporating --at, if reasonably possible.
Same applies to the other one as well.
There was a problem hiding this comment.
Sure, I will do that.
Please let us know once ready for final review. |
|
@liamaharon I realised that the problem I encountered was not in the In the substrate/utils/frame/try-runtime/cli/src/commands/execute_block.rs Lines 99 to 107 in 6e0059a |
|
Hmm.. Intuitively I would also expect |
| #[tokio::test] | ||
| async fn create_snapshot_works() { | ||
| // Build substrate so binaries used in the test use the latest code. | ||
| common::build_substrate(&["--features=try-runtime"]); |
There was a problem hiding this comment.
I now understood why we have to build again. Just very annoying and slow…
Maybe we can somehow inherit the build args in this function, so that it re-uses the cargo cache.
There was a problem hiding this comment.
That would be great. It does take quite some time to run all of these tests.
There was a problem hiding this comment.
Yea that would be a follow up as well; debug that build_substrate function and check if we can make it re-use the already existing build artifacts by using the same compile args that were used for the test.
|
|
||
| frame_support::log::info!( | ||
| target: LOG_TARGET, | ||
| "try-runtime: Block #{:?} successfully executed", |
There was a problem hiding this comment.
Just some ideas, we could do stuff here like:
- State trace (output of all modified storage keys)
- Print all executed calls + Events
- Consumed weight
- Block size etc
Probably most of it in JSON, so we can render it eventually (like chopsticks already does for state changes).
There was a problem hiding this comment.
Should this be done in this PR?
There was a problem hiding this comment.
No. Just some ideas for the future 😄
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
Needs to merge master to make the CI happy. |
|
Going to merge this. We can investigate afterwards if this |
|
bot merge |
|
Waiting for commit status. |
…h#14343) * execute-block test * test create-snapshot * oops * Update utils/frame/try-runtime/cli/tests/create_snapshot.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * Update utils/frame/try-runtime/cli/tests/create_snapshot.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * Update utils/frame/try-runtime/cli/tests/create_snapshot.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * remove snapshot * execute block: new log * use prefix & make tempfile a dev dependencie * Update utils/frame/try-runtime/cli/tests/execute_block.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * Update utils/frame/try-runtime/cli/tests/create_snapshot.rs Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> * ".git/.scripts/commands/fmt/fmt.sh" * --at option in execute-block test * fixes & use --at option in create-snapshot test * hmm * fmt * remove nonsense * Update utils/frame/try-runtime/cli/tests/create_snapshot.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * Update utils/frame/try-runtime/cli/tests/execute_block.rs Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> * remove unnecessary test modules * try to load snapshot file * fix --------- Co-authored-by: Liam Aharon <liam.aharon@hotmail.com> Co-authored-by: command-bot <> Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
|
/tip small |
|
@kianenigma A small (2 KSM) tip was successfully submitted for @Szegoo (DfqY6XQUSETTszBQ1juocTcG9iiDoXhvq1CoVadBSUqTGJS on kusama). https://polkadot.js.org/apps/?rpc=wss%3A%2F%2Fkusama-rpc.polkadot.io#/referenda |

This PR adds tests for the following
try-runtimecommands:execute-blockcreate-snapshotPart of: #13796
Kusama address: DfqY6XQUSETTszBQ1juocTcG9iiDoXhvq1CoVadBSUqTGJS