Added tests and clean-up code

.gitlab-ci.yml

@@ -235,15 +235,6 @@ check-web-wasm:
     - time cargo build --manifest-path=bin/node/cli/Cargo.toml --no-default-features --features "browser" --target=wasm32-unknown-unknown
     - sccache -s
 
-node-exits:
-  stage:                           test
-  <<:                              *docker-env
-  except:
-    - /^v[0-9]+\.[0-9]+.*$/        # i.e. v1.0, v2.1rc1
-  script:
-    - ./.maintain/check_for_exit.sh
-
-
 test-full-crypto-feature:
   stage:                           test
   <<:                              *docker-env

bin/node/cli/Cargo.toml

@@ -95,6 +95,8 @@ sc-consensus-babe = { version = "0.8", features = ["test-helpers"], path = "../.
 sc-service-test = { version = "2.0.0", path = "../../../client/service/test" }
 futures = "0.3.1"
 tempfile = "3.1.0"
+assert_cmd = "0.12"
+libc = "0.2"
 
 [build-dependencies]
 build-script-utils = { version = "2.0.0", package = "substrate-build-script-utils", path = "../../../utils/build-script-utils" }

bin/node/cli/tests/running_the_node_and_interrupt.rs

@@ -0,0 +1,22 @@
+use assert_cmd::cargo::cargo_bin;
+use std::process::Command;
+use std::thread::sleep;
+use std::time::Duration;
+
+#[test]
+#[cfg(unix)]
+fn running_the_node_works_and_can_be_interrupted() {
+	use libc::{kill, SIGINT, SIGTERM};
+
+	let mut cmd = Command::new(cargo_bin("substrate")).spawn().unwrap();
+	sleep(Duration::from_secs(30));
+	assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running");
+	unsafe { kill(cmd.id() as i32, SIGINT) };
+	assert!(cmd.wait().unwrap().success(), "the process must exit gracefully after SIGINT");
+
+	let mut cmd = Command::new(cargo_bin("substrate")).spawn().unwrap();
+	sleep(Duration::from_secs(30));
+	assert!(cmd.try_wait().unwrap().is_none(), "the process should still be running");
+	unsafe { kill(cmd.id() as i32, SIGTERM) };
+	assert!(cmd.wait().unwrap().success(), "the process must exit gracefully after SIGTERM");
+}

client/cli/src/lib.rs

@@ -398,8 +398,6 @@ fn fill_network_configuration(
 		];
 	}
 
-	config.public_addresses = Vec::new();
-
 	config.client_version = client_id;
 	config.node_key = node_key::node_key_config(cli.node_key_params, &config.net_config_path)?;
 
@@ -573,16 +571,13 @@ where
 		(_, Some(keyring)) => keyring.to_string(),
 		(None, None) => generate_node_name(),
 	};
-	match node_key::is_node_name_valid(&config.name) {
-		Ok(_) => (),
-		Err(msg) => Err(
-			error::Error::Input(
-				format!("Invalid node name '{}'. Reason: {}. If unsure, use none.",
-					config.name,
-					msg
-				)
+	if let Err(msg) = node_key::is_node_name_valid(&config.name) {
+		return Err(error::Error::Input(
+			format!("Invalid node name '{}'. Reason: {}. If unsure, use none.",
+				config.name,
+				msg,
 			)
-		)?
+		));
 	}
 
 	// set sentry mode (i.e. act as an authority but **never** actively participate)
@@ -809,4 +804,60 @@ mod tests {
 			assert_eq!(expected_path, node_config.keystore.path().unwrap().to_owned());
 		}
 	}
+
+	#[test]
+	fn ensure_load_spec_provide_defaults() {
+		let chain_spec = ChainSpec::from_genesis(
+			"test",
+			"test-id",
+			|| (),
+			vec!["boo".to_string()],
+			Some(TelemetryEndpoints::new(vec![("foo".to_string(), 42)])),
+			None,
+			None,
+			None::<()>,
+		);
+
+		let args: Vec<&str> = vec![];
+		let cli = RunCmd::from_iter(args);
+
+		let mut config = Configuration::new(TEST_VERSION_INFO);
+		load_spec(&mut config, &cli.shared_params, |_| Ok(Some(chain_spec))).unwrap();
+
+		assert!(config.chain_spec.is_some());
+		assert!(!config.network.boot_nodes.is_empty());
+		assert!(config.telemetry_endpoints.is_some());
+	}
+
+	#[test]
+	fn ensure_update_config_for_running_node_provides_defaults() {
+		let chain_spec = ChainSpec::from_genesis(
+			"test",
+			"test-id",
+			|| (),
+			vec![],
+			None,
+			None,
+			None,
+			None::<()>,
+		);
+
+		let args: Vec<&str> = vec![];
+		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();
+
+		assert!(config.config_dir.is_some());
+		assert!(config.database.is_some());
+		if let Some(DatabaseConfig::Path { ref cache_size, .. }) = config.database {
+			assert!(cache_size.is_some());
+		} else {
+			panic!("invalid config.database variant");
+		}
+		assert_ne!(config.name, "");
+		assert!(config.network.config_path.is_some());
+		assert!(!config.network.listen_addresses.is_empty());
+	}
 }