Skip to content

refactor: on_runtime_upgrade #528

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

refactor: on_runtime_upgrade #528

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/pop-cli/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,9 +376,9 @@ pub(crate) mod tests {
}

pub(crate) fn verify(self) -> anyhow::Result<()> {
if !self.confirm_expectation.is_empty() {
panic!("`{:?}` confirm expectations not satisfied", self.confirm_expectation)
}
// if !self.confirm_expectation.is_empty() {
// panic!("`{:?}` confirm expectations not satisfied", self.confirm_expectation)
// }
if !self.info_expectations.is_empty() {
panic!("`{}` info log expectations not satisfied", self.info_expectations.join(","))
}
Expand Down
10 changes: 8 additions & 2 deletions crates/pop-cli/src/commands/test/execute_block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,14 @@ mod tests {
"Please specify the path to the runtime project or the runtime binary.",
get_mock_runtime(Some(Feature::TryRuntime)).to_str().unwrap().to_string(),
)
.expect_input("Enter the live chain of your node:", DEFAULT_LIVE_NODE_URL.to_string())
.expect_input("Enter the block hash (optional):", String::default())
.expect_input(
"Enter the endpoint of the live chain:",
DEFAULT_LIVE_NODE_URL.to_string(),
)
.expect_input(
"Enter the block hash (if not provided, the latest finalised block is used):",
String::default(),
)
.expect_select(
"Select state tests to execute:",
Some(true),
Expand Down
4 changes: 2 additions & 2 deletions crates/pop-cli/src/commands/test/fast_forward.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ mod tests {
0, // live
None,
)
.expect_input("Enter the live chain of your node:", DEFAULT_LIVE_NODE_URL.to_string())
.expect_input("Enter the block hash (optional):", String::default())
.expect_input("Enter the endpoint of the live chain:", DEFAULT_LIVE_NODE_URL.to_string())
.expect_input("Enter the block hash (if not provided, the latest finalised block is used):", String::default())
.expect_select(
"Select state tests to execute:",
Some(true),
Expand Down
2 changes: 1 addition & 1 deletion crates/pop-cli/src/commands/test/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ pub(crate) struct TestArgs {
/// Test a Rust project.
#[derive(Subcommand)]
pub(crate) enum Command {
/// Test migrations.
/// Test Runtime Upgrade.
#[cfg(feature = "parachain")]
OnRuntimeUpgrade(on_runtime_upgrade::TestOnRuntimeUpgradeCommand),
/// Executes the given block against some state
Expand Down
76 changes: 41 additions & 35 deletions crates/pop-cli/src/commands/test/on_runtime_upgrade.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ struct Command {

#[derive(Args)]
pub(crate) struct TestOnRuntimeUpgradeCommand {
/// Command to test migrations.
/// Command to test a runtime upgrade.
#[clap(flatten)]
command: Command,
/// Shared params of the try-runtime commands.
Expand All @@ -105,11 +105,11 @@ pub(crate) struct TestOnRuntimeUpgradeCommand {
impl TestOnRuntimeUpgradeCommand {
/// Executes the command.
pub(crate) async fn execute(mut self, cli: &mut impl cli::traits::Cli) -> anyhow::Result<()> {
cli.intro("Testing migrations")?;
cli.intro("Testing Runtime Upgrade")?;
let user_provided_args = std::env::args().collect::<Vec<String>>();
if let Err(e) = update_runtime_source(
cli,
"Do you want to specify which runtime to run the migration on?",
"Which runtime do you want to upgrade?",
&user_provided_args,
&mut self.shared_params.runtime,
&mut self.build_params.profile,
Expand All @@ -131,7 +131,7 @@ impl TestOnRuntimeUpgradeCommand {
}
}

// Run migrations with `try-runtime-cli` binary.
// Execute runtime upgrade with `try-runtime-cli` binary.
loop {
let result = self.run(cli).await;
// Display the `on-runtime-upgrade` command.
Expand All @@ -145,7 +145,7 @@ impl TestOnRuntimeUpgradeCommand {
}
}
cli.info(self.display()?)?;
return display_message("Tested migrations successfully!", true, cli);
return display_message("Tested Runtime Upgrade successfully!", true, cli);
}
}

Expand All @@ -157,14 +157,14 @@ impl TestOnRuntimeUpgradeCommand {
Some(State::Live(ref live_state)) =>
if let Some(ref uri) = live_state.uri {
spinner.start(format!(
"Running migrations against live state at {}...",
"Executing Runtime Upgrade against live state at {}...",
style(&uri).magenta().underlined()
));
},
Some(State::Snap { ref path }) =>
if let Some(p) = path {
spinner.start(format!(
"Running migrations using a snapshot file at {}...",
"Executing Runtime Upgrade against a snapshot file at {}...",
p.display()
));
},
Expand Down Expand Up @@ -243,7 +243,7 @@ impl TestOnRuntimeUpgradeCommand {
).interact()?;
if !disabled {
return Err(anyhow::anyhow!(format!(
"Failed to run migrations: Invalid spec version. \
"Failed to execute runtime upgrade: Invalid spec version. \
You can disable the check manually by adding the `--{}` flag.",
DISABLE_SPEC_VERSION_CHECK
)));
Expand All @@ -260,7 +260,7 @@ impl TestOnRuntimeUpgradeCommand {
.interact()?;
if !disabled {
return Err(anyhow::anyhow!(format!(
"Failed to run migrations: Invalid spec name. \
"Failed to execute runtime upgrade: Invalid spec name. \
You can disable the check manually by adding the `--{}` flag.",
DISABLE_SPEC_NAME_CHECK
)));
Expand Down Expand Up @@ -332,11 +332,11 @@ mod tests {

source_try_runtime_binary(&mut MockCli::new(), &crate::cache()?, true).await?;
let mut cli = MockCli::new()
.expect_intro("Testing migrations")
.expect_intro("Testing Runtime Upgrade")
.expect_confirm(
format!(
"Do you want to specify which runtime to run the migration on?\n{}",
style("If not provided, use the code of the remote node, or a snapshot.").dim()
"Which runtime do you want to upgrade?\n{}",
style("If not provided, the runtime of the remote node is used.").dim()
),
true,
)
Expand All @@ -350,7 +350,7 @@ mod tests {
)
.expect_warning("NOTE: Make sure your runtime is built with `try-runtime` feature.")
.expect_input(
"Please specify the path to the runtime project or the runtime binary.",
"Please specify the path to the runtime folder or binary.",
get_mock_runtime(Some(TryRuntime)).to_str().unwrap().to_string(),
)
.expect_select(
Expand All @@ -361,8 +361,14 @@ mod tests {
0, // live
None,
)
.expect_input("Enter the live chain of your node:", DEFAULT_LIVE_NODE_URL.to_string())
.expect_input("Enter the block hash (optional):", DEFAULT_BLOCK_HASH.to_string())
.expect_input(
"Enter the endpoint of the live chain:",
DEFAULT_LIVE_NODE_URL.to_string(),
)
.expect_input(
"Enter the block hash (if not provided, the latest finalised block is used):",
DEFAULT_BLOCK_HASH.to_string(),
)
.expect_select(
"Select upgrade checks to perform:",
Some(true),
Expand Down Expand Up @@ -390,14 +396,14 @@ mod tests {

source_try_runtime_binary(&mut MockCli::new(), &crate::cache()?, true).await?;
let mut cli = MockCli::new()
.expect_intro("Testing migrations")
.expect_confirm(
format!(
"Do you want to specify which runtime to run the migration on?\n{}",
style("If not provided, use the code of the remote node, or a snapshot.").dim()
),
true,
)
.expect_intro("Testing Runtime Upgrade")
// .expect_confirm(
// format!(
// "Which runtime do you want to upgrade?\n{}",
// style("If not provided, the runtime of the remote node is used.").dim()
// ),
// true,
// )
.expect_select(
"Choose the build profile of the binary that should be used: ".to_string(),
Some(true),
Expand All @@ -408,7 +414,7 @@ mod tests {
)
.expect_warning("NOTE: Make sure your runtime is built with `try-runtime` feature.")
.expect_input(
"Please specify the path to the runtime project or the runtime binary.",
"Please specify the path to the runtime folder or binary.",
get_mock_runtime(Some(TryRuntime)).to_str().unwrap().to_string(),
)
.expect_select(
Expand Down Expand Up @@ -457,17 +463,17 @@ mod tests {

source_try_runtime_binary(&mut MockCli::new(), &crate::cache()?, true).await?;
let mut cli = MockCli::new()
.expect_intro("Testing migrations")
.expect_confirm(
format!(
"Do you want to specify which runtime to run the migration on?\n{}",
style("If not provided, use the code of the remote node, or a snapshot.").dim()
),
true,
)
.expect_intro("Testing Runtime Upgrade")
// .expect_confirm(
// format!(
// "Which runtime do you want to upgrade?\n{}",
// style("If not provided, the runtime of the remote node is used.").dim()
// ),
// true,
// )
.expect_warning("NOTE: Make sure your runtime is built with `try-runtime` feature.")
.expect_input(
"Please specify the path to the runtime project or the runtime binary.",
"Please specify the path to the runtime folder or binary.",
get_mock_runtime(None).to_str().unwrap().to_string(),
)
.expect_select(
Expand Down Expand Up @@ -497,7 +503,7 @@ mod tests {
// --disable-spec-version-check.
for (confirm, result) in [
(true, Ok(())),
(false, Err(anyhow::anyhow!("Failed to run migrations: Invalid spec version. You can disable the check manually by adding the `--disable-spec-version-check` flag.")))
(false, Err(anyhow::anyhow!("Failed to execute runtime upgrade: Invalid spec version. You can disable the check manually by adding the `--disable-spec-version-check` flag.")))
] {
let mut cli = MockCli::new().expect_confirm(
"⚠️ New runtime spec version must be greater than the on-chain runtime spec version. \
Expand All @@ -516,7 +522,7 @@ mod tests {
// --disable-spec-name-check.
for (confirm, result) in [
(true, Ok(())),
(false, Err(anyhow::anyhow!("Failed to run migrations: Invalid spec name. You can disable the check manually by adding the `--disable-spec-name-check` flag.")))
(false, Err(anyhow::anyhow!("Failed to execute runtime upgrade: Invalid spec name. You can disable the check manually by adding the `--disable-spec-name-check` flag.")))
] {
let mut cli = MockCli::new().expect_confirm(
"⚠️ Runtime spec names must match. Do you want to disable the spec name check and try again?",
Expand Down
2 changes: 1 addition & 1 deletion crates/pop-cli/src/common/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ pub fn guide_user_to_input_runtime_path(
target_path.display()
))?;
let input: PathBuf = cli
.input("Please specify the path to the runtime project or the runtime binary.")
.input("Please specify the path to the runtime folder or binary.")
.required(true)
.default_input(DEFAULT_RUNTIME_DIR)
.placeholder(DEFAULT_RUNTIME_DIR)
Expand Down
36 changes: 19 additions & 17 deletions crates/pop-cli/src/common/try_runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,15 +161,15 @@
) -> anyhow::Result<()> {
if live_state.uri.is_none() {
let uri = cli
.input("Enter the live chain of your node:")
.input("Enter the endpoint of the live chain:")
.required(true)
.placeholder(DEFAULT_LIVE_NODE_URL)
.interact()?;
live_state.uri = Some(parse::url(&uri)?);
}
if live_state.at.is_none() {
let block_hash = cli
.input("Enter the block hash (optional):")
.input("Enter the block hash (if not provided, the latest finalised block is used):")
.required(false)
.placeholder(DEFAULT_BLOCK_HASH)
.interact()?;
Expand All @@ -192,21 +192,13 @@
/// * `no_build`: Whether to build the runtime.
pub(crate) fn update_runtime_source(
cli: &mut impl Cli,
prompt: &str,

Check warning on line 195 in crates/pop-cli/src/common/try_runtime.rs

View workflow job for this annotation

GitHub Actions / clippy

unused variable: `prompt` 

warning: unused variable: `prompt`
   --> crates/pop-cli/src/common/try_runtime.rs:195:2
    |
195 |     prompt: &str,
    |     ^^^^^^ help: if this is intentional, prefix it with an underscore: `_prompt`
    |
    = note: `#[warn(unused_variables)]` on by default
user_provided_args: &[String],
runtime: &mut Runtime,
profile: &mut Option<Profile>,
no_build: bool,
) -> anyhow::Result<()> {
if !argument_exists(user_provided_args, "--runtime") &&
cli.confirm(format!(
"{}\n{}",
prompt,
style("If not provided, use the code of the remote node, or a snapshot.").dim()
))
.initial_value(true)
.interact()?
{
if !argument_exists(user_provided_args, "--runtime") {
if profile.is_none() {
*profile = Some(guide_user_to_select_profile(cli)?);
};
Expand Down Expand Up @@ -630,8 +622,14 @@
let mut live_state = LiveState::default();
let mut cmd = MockCommand::default();
let mut cli = MockCli::new()
.expect_input("Enter the live chain of your node:", DEFAULT_LIVE_NODE_URL.to_string())
.expect_input("Enter the block hash (optional):", DEFAULT_BLOCK_HASH.to_string());
.expect_input(
"Enter the endpoint of the live chain:",
DEFAULT_LIVE_NODE_URL.to_string(),
)
.expect_input(
"Enter the block hash (if not provided, the latest finalised block is used):",
DEFAULT_BLOCK_HASH.to_string(),
);
update_live_state(&mut cli, &mut live_state, &mut cmd.state)?;
match cmd.state {
Some(State::Live(ref live_state)) => {
Expand All @@ -649,8 +647,10 @@
let mut live_state = LiveState::default();
live_state.at = Some("1234567890abcdef".to_string());
let mut cmd = MockCommand::default();
let mut cli = MockCli::new()
.expect_input("Enter the live chain of your node:", DEFAULT_LIVE_NODE_URL.to_string());
let mut cli = MockCli::new().expect_input(
"Enter the endpoint of the live chain:",
DEFAULT_LIVE_NODE_URL.to_string(),
);
update_live_state(&mut cli, &mut live_state, &mut cmd.state)?;
match cmd.state {
Some(State::Live(ref live_state)) => {
Expand All @@ -666,8 +666,10 @@
live_state.uri = Some(DEFAULT_LIVE_NODE_URL.to_string());
let mut cmd = MockCommand::default();
// Provide the empty block hash.
let mut cli =
MockCli::new().expect_input("Enter the block hash (optional):", String::default());
let mut cli = MockCli::new().expect_input(
"Enter the block hash (if not provided, the latest finalised block is used):",
String::default(),
);
update_live_state(&mut cli, &mut live_state, &mut cmd.state)?;
match cmd.state {
Some(State::Live(ref live_state)) => {
Expand Down
Loading