diff --git a/substrate/bin/node/cli/src/command.rs b/substrate/bin/node/cli/src/command.rs index 0e9b23b73e..3bfed148f5 100644 --- a/substrate/bin/node/cli/src/command.rs +++ b/substrate/bin/node/cli/src/command.rs @@ -41,7 +41,7 @@ where ), Some(Subcommand::Factory(cli_args)) => { sc_cli::init(&cli_args.shared_params, &version)?; - sc_cli::load_spec(&mut config, &cli_args.shared_params, load_spec)?; + sc_cli::init_config(&mut config, &cli_args.shared_params, &version, load_spec)?; sc_cli::fill_import_params( &mut config, &cli_args.import_params, diff --git a/substrate/bin/node/cli/tests/common.rs b/substrate/bin/node/cli/tests/common.rs new file mode 100644 index 0000000000..96060bf85d --- /dev/null +++ b/substrate/bin/node/cli/tests/common.rs @@ -0,0 +1,34 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +use std::{process::{Child, ExitStatus}, thread, time::Duration}; + +/// Wait for the given `child` the given ammount of `secs`. +/// +/// Returns the `Some(exit status)` or `None` if the process did not finish in the given time. +pub fn wait_for(child: &mut Child, secs: usize) -> Option { + for _ in 0..secs { + match child.try_wait().unwrap() { + Some(status) => return Some(status), + None => thread::sleep(Duration::from_secs(1)), + } + } + eprintln!("Took to long to exit. Killing..."); + let _ = child.kill(); + child.wait().unwrap(); + + None +} diff --git a/substrate/bin/node/cli/tests/purge_chain_works.rs b/substrate/bin/node/cli/tests/purge_chain_works.rs new file mode 100644 index 0000000000..e6b71db991 --- /dev/null +++ b/substrate/bin/node/cli/tests/purge_chain_works.rs @@ -0,0 +1,53 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +use assert_cmd::cargo::cargo_bin; +use std::{convert::TryInto, process::Command, thread, time::Duration, fs, path::PathBuf}; + +mod common; + +#[test] +#[cfg(unix)] +fn purge_chain_works() { + use nix::sys::signal::{kill, Signal::SIGINT}; + use nix::unistd::Pid; + + let base_path = "purge_chain_test"; + + let _ = fs::remove_dir_all(base_path); + let mut cmd = Command::new(cargo_bin("substrate")) + .args(&["--dev", "-d", base_path]) + .spawn() + .unwrap(); + + // Let it produce some blocks. + thread::sleep(Duration::from_secs(30)); + assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running"); + + // Stop the process + kill(Pid::from_raw(cmd.id().try_into().unwrap()), SIGINT).unwrap(); + assert!(common::wait_for(&mut cmd, 30).map(|x| x.success()).unwrap_or_default()); + + let status = Command::new(cargo_bin("substrate")) + .args(&["purge-chain", "--dev", "-d", base_path, "-y"]) + .status() + .unwrap(); + assert!(status.success()); + + // Make sure that the `dev` chain folder exists, but the `db` is deleted. + assert!(PathBuf::from(base_path).join("chains/dev/").exists()); + assert!(!PathBuf::from(base_path).join("chains/dev/db").exists()); +} diff --git a/substrate/bin/node/cli/tests/running_the_node_and_interrupt.rs b/substrate/bin/node/cli/tests/running_the_node_and_interrupt.rs index 6b0d696396..6ab719de91 100644 --- a/substrate/bin/node/cli/tests/running_the_node_and_interrupt.rs +++ b/substrate/bin/node/cli/tests/running_the_node_and_interrupt.rs @@ -15,10 +15,9 @@ // along with Substrate. If not, see . use assert_cmd::cargo::cargo_bin; -use std::convert::TryInto; -use std::process::{Child, Command, ExitStatus}; -use std::thread::sleep; -use std::time::Duration; +use std::{convert::TryInto, process::Command, thread, time::Duration, fs}; + +mod common; #[test] #[cfg(unix)] @@ -26,27 +25,18 @@ fn running_the_node_works_and_can_be_interrupted() { use nix::sys::signal::{kill, Signal::{self, SIGINT, SIGTERM}}; use nix::unistd::Pid; - fn wait_for(child: &mut Child, secs: usize) -> Option { - for _ in 0..secs { - match child.try_wait().unwrap() { - Some(status) => return Some(status), - None => sleep(Duration::from_secs(1)), - } - } - eprintln!("Took to long to exit. Killing..."); - let _ = child.kill(); - child.wait().unwrap(); - - None - } - fn run_command_and_kill(signal: Signal) { - let mut cmd = Command::new(cargo_bin("substrate")).spawn().unwrap(); - sleep(Duration::from_secs(30)); + let _ = fs::remove_dir_all("interrupt_test"); + let mut cmd = Command::new(cargo_bin("substrate")) + .args(&["--dev", "-d", "interrupt_test"]) + .spawn() + .unwrap(); + + thread::sleep(Duration::from_secs(30)); assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running"); kill(Pid::from_raw(cmd.id().try_into().unwrap()), signal).unwrap(); assert_eq!( - wait_for(&mut cmd, 30).map(|x| x.success()), + common::wait_for(&mut cmd, 30).map(|x| x.success()), Some(true), "the pocess must exit gracefully after signal {}", signal, diff --git a/substrate/client/cli/src/lib.rs b/substrate/client/cli/src/lib.rs index 735f8cb27a..54a98a6ad3 100644 --- a/substrate/client/cli/src/lib.rs +++ b/substrate/client/cli/src/lib.rs @@ -234,7 +234,7 @@ where SF: AbstractService + Unpin, { init(&run_cmd.shared_params, version)?; - load_spec(&mut config, &run_cmd.shared_params, spec_factory)?; + init_config(&mut config, &run_cmd.shared_params, version, spec_factory)?; run_cmd.run(config, new_light, new_full, version) } @@ -257,9 +257,8 @@ where ::Hash: std::str::FromStr, { let shared_params = subcommand.get_shared_params(); - init(shared_params, version)?; - load_spec(&mut config, shared_params, spec_factory)?; + init_config(&mut config, shared_params, version, spec_factory)?; subcommand.run(config, builder) } @@ -267,9 +266,9 @@ where /// /// This method: /// -/// 1. set the panic handler -/// 2. raise the FD limit -/// 3. initialize the logger +/// 1. Set the panic handler +/// 2. Raise the FD limit +/// 3. Initialize the logger pub fn init(shared_params: &SharedParams, version: &VersionInfo) -> error::Result<()> { let full_version = sc_service::config::full_version_from_strs( version.version, @@ -283,6 +282,37 @@ pub fn init(shared_params: &SharedParams, version: &VersionInfo) -> error::Resul Ok(()) } +/// Initialize the given `config`. +/// +/// This will load the chain spec, set the `config_dir` and the `database_dir`. +pub fn init_config( + config: &mut Configuration, + shared_params: &SharedParams, + version: &VersionInfo, + spec_factory: F, +) -> error::Result<()> where + F: FnOnce(&str) -> Result>, String>, + G: RuntimeGenesis, + E: ChainSpecExtension, +{ + load_spec(config, shared_params, spec_factory)?; + + if config.config_dir.is_none() { + config.config_dir = Some(base_path(&shared_params, version)); + } + + if config.database.is_none() { + config.database = Some(DatabaseConfig::Path { + path: config + .in_chain_config_dir(DEFAULT_DB_CONFIG_PATH) + .expect("We provided a base_path/config_dir."), + cache_size: None, + }); + } + + Ok(()) +} + /// Run the node /// /// Builds and runs either a full or a light node, depending on the `role` within the `Configuration`. @@ -514,26 +544,10 @@ where pub fn update_config_for_running_node( mut config: &mut Configuration, cli: RunCmd, - version: &VersionInfo, ) -> error::Result<()> where G: RuntimeGenesis, { - if config.config_dir.is_none() { - config.config_dir = Some(base_path(&cli.shared_params, version)); - } - - if config.database.is_none() { - // NOTE: the loading of the DatabaseConfig is voluntarily delayed to here - // in case config.config_dir has been customized - config.database = Some(DatabaseConfig::Path { - path: config - .in_chain_config_dir(DEFAULT_DB_CONFIG_PATH) - .expect("We provided a base_path/config_dir."), - cache_size: None, - }); - } - fill_config_keystore_password_and_path(&mut config, &cli)?; let keyring = cli.get_keyring(); @@ -790,7 +804,6 @@ mod tests { update_config_for_running_node( &mut node_config, run_cmds.clone(), - TEST_VERSION_INFO, ).unwrap(); let expected_path = match keystore_path { @@ -843,8 +856,14 @@ mod tests { let cli = RunCmd::from_iter(args); let mut config = Configuration::new(TEST_VERSION_INFO); - config.chain_spec = Some(chain_spec); - update_config_for_running_node(&mut config, cli, TEST_VERSION_INFO).unwrap(); + init(&cli.shared_params, &TEST_VERSION_INFO).unwrap(); + init_config( + &mut config, + &cli.shared_params, + &TEST_VERSION_INFO, + |_| Ok(Some(chain_spec)), + ).unwrap(); + update_config_for_running_node(&mut config, cli).unwrap(); assert!(config.config_dir.is_some()); assert!(config.database.is_some()); diff --git a/substrate/client/cli/src/params.rs b/substrate/client/cli/src/params.rs index ddded79142..a99d887d25 100644 --- a/substrate/client/cli/src/params.rs +++ b/substrate/client/cli/src/params.rs @@ -933,11 +933,7 @@ impl RunCmd { { assert!(config.chain_spec.is_some(), "chain_spec must be present before continuing"); - crate::update_config_for_running_node( - &mut config, - self, - &version, - )?; + crate::update_config_for_running_node(&mut config, self)?; crate::run_node(config, new_light, new_full, &version) }