Add tests & Service's Configuration has optional fields that shouldn't be optional

bin/node-template/node/src/command.rs

@@ -15,32 +15,35 @@
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
 use sp_consensus_aura::sr25519::{AuthorityPair as AuraPair};
-use sc_cli::{VersionInfo, error};
+use sc_cli::{VersionInfo, self};
 use crate::service;
 use crate::chain_spec;
 use crate::cli::Cli;
 
 /// Parse and run command line arguments
-pub fn run(version: VersionInfo) -> error::Result<()> {
+pub fn run(version: VersionInfo) -> sc_cli::Result<()> {
 	let opt = sc_cli::from_args::<Cli>(&version);
 
-	let config = sc_service::Configuration::new(&version);
+	let mut config = sc_service::Configuration::from_version(&version);
 
 	match opt.subcommand {
-		Some(subcommand) => sc_cli::run_subcommand(
-			config,
-			subcommand,
-			chain_spec::load_spec,
-			|config: _| Ok(new_full_start!(config).0),
-			&version,
-		),
-		None => sc_cli::run(
-			config,
-			opt.run,
-			service::new_light,
-			service::new_full,
-			chain_spec::load_spec,
-			&version,
-		)
+		Some(subcommand) => {
+			subcommand.init(&version)?;
+			subcommand.update_config(&mut config, chain_spec::load_spec, &version)?;
+			subcommand.run(
+				config,
+				|config: _| Ok(new_full_start!(config).0),
+			)
+		},
+		None => {
+			opt.run.init(&version)?;
+			opt.run.update_config(&mut config, chain_spec::load_spec, &version)?;
+			opt.run.run(
+				config,
+				service::new_light,
+				service::new_full,
+				&version,
+			)
+		},
 	}
 }

bin/node-template/node/src/main.rs

@@ -7,10 +7,10 @@ mod service;
 mod cli;
 mod command;
 
-pub use sc_cli::{VersionInfo, error};
+pub use sc_cli;
 
-fn main() -> Result<(), error::Error> {
-	let version = VersionInfo {
+fn main() -> sc_cli::Result<()> {
+	let version = sc_cli::VersionInfo {
 		name: "Substrate Node",
 		commit: env!("VERGEN_SHA_SHORT"),
 		version: env!("CARGO_PKG_VERSION"),

bin/node/cli/bin/main.rs

@@ -18,10 +18,10 @@
 
 #![warn(missing_docs)]
 
-use sc_cli::VersionInfo;
+use sc_cli;
 
-fn main() -> Result<(), sc_cli::error::Error> {
-	let version = VersionInfo {
+fn main() -> sc_cli::Result<()> {
+	let version = sc_cli::VersionInfo {
 		name: "Substrate Node",
 		commit: env!("VERGEN_SHA_SHORT"),
 		version: env!("CARGO_PKG_VERSION"),

bin/node/cli/src/cli.rs

@@ -19,11 +19,6 @@ use structopt::StructOpt;
 
 #[allow(missing_docs)]
 #[derive(Clone, Debug, StructOpt)]
-#[structopt(settings = &[
-	structopt::clap::AppSettings::GlobalVersion,
-	structopt::clap::AppSettings::ArgsNegateSubcommands,
-	structopt::clap::AppSettings::SubcommandsNegateReqs,
-])]
 pub struct Cli {
 	#[allow(missing_docs)]
 	#[structopt(subcommand)]

bin/node/cli/src/command.rs

@@ -14,46 +14,49 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-use sc_cli::{VersionInfo, error};
+use sc_cli::{VersionInfo, self};
 use sc_service::{Roles as ServiceRoles};
 use node_transaction_factory::RuntimeAdapter;
 use crate::{Cli, service, ChainSpec, load_spec, Subcommand, factory_impl::FactoryState};
 
 /// Parse command line arguments into service configuration.
-pub fn run<I, T>(args: I, version: VersionInfo) -> error::Result<()>
+pub fn run<I, T>(args: I, version: VersionInfo) -> sc_cli::Result<()>
 where
 	I: Iterator<Item = T>,
 	T: Into<std::ffi::OsString> + Clone,
 {
 	let args: Vec<_> = args.collect();
 	let opt = sc_cli::from_iter::<Cli, _>(args.clone(), &version);
 
-	let mut config = sc_service::Configuration::new(&version);
+	let mut config = sc_service::Configuration::from_version(&version);
 
 	match opt.subcommand {
-		None => sc_cli::run(
-			config,
-			opt.run,
-			service::new_light,
-			service::new_full,
-			load_spec,
-			&version,
-		),
+		None => {
+			opt.run.init(&version)?;
+			opt.run.update_config(&mut config, load_spec, &version)?;
+			opt.run.run(
+				config,
+				service::new_light,
+				service::new_full,
+				&version,
+			)
+		},
 		Some(Subcommand::Factory(cli_args)) => {
-			sc_cli::init(&cli_args.shared_params, &version)?;
-			sc_cli::init_config(&mut config, &cli_args.shared_params, &version, load_spec)?;
-			sc_cli::fill_import_params(
+			cli_args.shared_params.init(&version)?;
+			cli_args.shared_params.update_config(&mut config, load_spec, &version)?;
+			cli_args.import_params.update_config(
 				&mut config,
-				&cli_args.import_params,
 				ServiceRoles::FULL,
 				cli_args.shared_params.dev,
 			)?;
 
-			sc_cli::fill_config_keystore_in_memory(&mut config)?;
+			config.use_in_memory_keystore()?;
 
 			match ChainSpec::from(config.expect_chain_spec().id()) {
 				Some(ref c) if c == &ChainSpec::Development || c == &ChainSpec::LocalTestnet => {},
-				_ => panic!("Factory is only supported for development and local testnet."),
+				_ => return Err(
+					"Factory is only supported for development and local testnet.".into()
+				),
 			}
 
 			// Setup tracing.
@@ -62,7 +65,9 @@ where
 					cli_args.import_params.tracing_receiver.into(), tracing_targets
 				);
 				if let Err(e) = tracing::subscriber::set_global_default(subscriber) {
-					panic!("Unable to set global default subscriber {}", e);
+					return Err(
+						format!("Unable to set global default subscriber {}", e).into()
+					);
 				}
 			}
 
@@ -81,12 +86,13 @@ where
 
 			Ok(())
 		},
-		Some(Subcommand::Base(subcommand)) => sc_cli::run_subcommand(
-			config,
-			subcommand,
-			load_spec,
-			|config: service::NodeConfiguration| Ok(new_full_start!(config).0),
-			&version,
-		),
+		Some(Subcommand::Base(subcommand)) => {
+			subcommand.init(&version)?;
+			subcommand.update_config(&mut config, load_spec, &version)?;
+			subcommand.run(
+				config,
+				|config: service::NodeConfiguration| Ok(new_full_start!(config).0),
+			)
+		},
 	}
 }

bin/node/cli/tests/build_spec_works.rs

@@ -0,0 +1,39 @@
+// 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 <http://www.gnu.org/licenses/>.
+
+#![cfg(unix)]
+
+use assert_cmd::cargo::cargo_bin;
+use std::{process::{Command, Stdio}, fs, path::PathBuf};
+
+mod common;
+
+#[test]
+fn build_spec_works() {
+	let base_path = "build_spec_test";
+
+	let _ = fs::remove_dir_all(base_path);
+	let status = Command::new(cargo_bin("substrate"))
+		.stdout(Stdio::null())
+		.args(&["build-spec", "--dev", "-d", base_path])
+		.status()
+		.unwrap();
+	assert!(status.success());
+
+	// Make sure that the `dev` chain folder exists, but the `db` doesn't
+	assert!(PathBuf::from(base_path).join("chains/dev/").exists());
+	assert!(!PathBuf::from(base_path).join("chains/dev/db").exists());
+}

client/cli/src/traits.rs → bin/node/cli/tests/check_block_works.rs

@@ -1,4 +1,4 @@
-// Copyright 2017-2020 Parity Technologies (UK) Ltd.
+// Copyright 2020 Parity Technologies (UK) Ltd.
 // This file is part of Substrate.
 
 // Substrate is free software: you can redistribute it and/or modify
@@ -14,10 +14,23 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
-use crate::params::SharedParams;
+#![cfg(unix)]
 
-/// Supports getting common params.
-pub trait GetSharedParams {
-	/// Returns shared params if any.
-	fn shared_params(&self) -> Option<&SharedParams>;
+use assert_cmd::cargo::cargo_bin;
+use std::{process::Command, fs};
+
+mod common;
+
+#[test]
+fn check_block_works() {
+	let base_path = "check_block_test";
+
+	let _ = fs::remove_dir_all(base_path);
+	common::run_command_for_a_while(&["-d", base_path]);
+
+	let status = Command::new(cargo_bin("substrate"))
+		.args(&["check-block", "-d", base_path, "1"])
+		.status()
+		.unwrap();
+	assert!(status.success());
 }

bin/node/cli/tests/common.rs

@@ -14,7 +14,13 @@
 // You should have received a copy of the GNU General Public License
 // along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
 
+#![cfg(unix)]
+
 use std::{process::{Child, ExitStatus}, thread, time::Duration};
+use assert_cmd::cargo::cargo_bin;
+use std::{convert::TryInto, process::Command};
+use nix::sys::signal::{kill, Signal::SIGINT};
+use nix::unistd::Pid;
 
 /// Wait for the given `child` the given ammount of `secs`.
 ///
@@ -32,3 +38,18 @@ pub fn wait_for(child: &mut Child, secs: usize) -> Option<ExitStatus> {
 
 	None
 }
+
+pub fn run_command_for_a_while(args: &[&str]) {
+	let mut cmd = Command::new(cargo_bin("substrate"))
+		.args(args)
+		.spawn()
+		.unwrap();
+
+	// Let it produce some blocks.
+	thread::sleep(Duration::from_secs(20));
+	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!(wait_for(&mut cmd, 20).map(|x| x.success()).unwrap_or_default());
+}

bin/node/cli/tests/factory.rs

@@ -0,0 +1,39 @@
+// 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 <http://www.gnu.org/licenses/>.
+
+#![cfg(unix)]
+
+use assert_cmd::cargo::cargo_bin;
+use std::{process::{Command, Stdio}, fs, path::PathBuf};
+
+mod common;
+
+#[test]
+fn factory_works() {
+	let base_path = "factory_test";
+
+	let _ = fs::remove_dir_all(base_path);
+	let status = Command::new(cargo_bin("substrate"))
+		.stdout(Stdio::null())
+		.args(&["factory", "--dev", "-d", base_path])
+		.status()
+		.unwrap();
+	assert!(status.success());
+
+	// Make sure that the `dev` chain folder exists & `db`
+	assert!(PathBuf::from(base_path).join("chains/dev/").exists());
+	assert!(PathBuf::from(base_path).join("chains/dev/db").exists());
+}

bin/node/cli/tests/import_export_and_revert_work.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 <http://www.gnu.org/licenses/>.
+
+#![cfg(unix)]
+
+use assert_cmd::cargo::cargo_bin;
+use std::{process::Command, fs};
+
+mod common;
+
+#[test]
+fn import_export_and_revert_work() {
+	let base_path = "import_export_and_revert_test";
+
+	let _ = fs::remove_dir_all(base_path);
+	common::run_command_for_a_while(&["-d", base_path]);
+
+	let status = Command::new(cargo_bin("substrate"))
+		.args(&["export-blocks", "-d", base_path, "exported_blocks"])
+		.status()
+		.unwrap();
+	assert!(status.success());
+
+	let metadata = fs::metadata("exported_blocks").unwrap();
+	assert!(metadata.len() > 0, "file exported_blocks should not be empty");
+
+	let _ = fs::remove_dir_all(base_path);
+
+	let status = Command::new(cargo_bin("substrate"))
+		.args(&["import-blocks", "-d", base_path, "exported_blocks"])
+		.status()
+		.unwrap();
+	assert!(status.success());
+
+	let status = Command::new(cargo_bin("substrate"))
+		.args(&["revert", "-d", base_path])
+		.status()
+		.unwrap();
+	assert!(status.success());
+}