refactor: include Cli in methods

r0gue-io · Dec 18, 2024 · 2075be5 · 2075be5
1 parent 8491d30
commit 2075be5
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 34 deletions.
diff --git a/crates/pop-cli/src/commands/test/contract.rs b/crates/pop-cli/src/commands/test/contract.rs
@@ -48,7 +48,7 @@ impl TestContractCommand {
 				sleep(Duration::from_secs(3)).await;
 			}
 
-			self.node = match check_contracts_node_and_prompt(self.skip_confirm).await {
+			self.node = match check_contracts_node_and_prompt(&mut Cli, self.skip_confirm).await {
 				Ok(binary_path) => Some(binary_path),
 				Err(_) => {
 					warning("🚫 substrate-contracts-node is necessary to run e2e tests. Will try to run tests anyway...")?;

diff --git a/crates/pop-cli/src/commands/up/contract.rs b/crates/pop-cli/src/commands/up/contract.rs
@@ -139,15 +139,16 @@ impl UpContractCommand {
 			let log = NamedTempFile::new()?;
 
 			// uses the cache location
-			let binary_path = match check_contracts_node_and_prompt(self.skip_confirm).await {
-				Ok(binary_path) => binary_path,
-				Err(_) => {
-					Cli.outro_cancel(
-						"🚫 You need to specify an accessible endpoint to deploy the contract.",
-					)?;
-					return Ok(());
-				},
-			};
+			let binary_path =
+				match check_contracts_node_and_prompt(&mut Cli, self.skip_confirm).await {
+					Ok(binary_path) => binary_path,
+					Err(_) => {
+						Cli.outro_cancel(
+							"🚫 You need to specify an accessible endpoint to deploy the contract.",
+						)?;
+						return Ok(());
+					},
+				};
 
 			let spinner = spinner();
 			spinner.start("Starting local node...");
@@ -181,7 +182,7 @@ impl UpContractCommand {
 				Ok(data) => data,
 				Err(e) => {
 					error(format!("An error occurred getting the call data: {e}"))?;
-					terminate_node(process)?;
+					terminate_node(&mut Cli, process)?;
 					Cli.outro_cancel(FAILED)?;
 					return Ok(());
 				},
@@ -200,7 +201,7 @@ impl UpContractCommand {
 						Err(e) => {
 							spinner
 								.error(format!("An error occurred uploading your contract: {e}"));
-							terminate_node(process)?;
+							terminate_node(&mut Cli, process)?;
 							Cli.outro_cancel(FAILED)?;
 							return Ok(());
 						},
@@ -223,7 +224,7 @@ impl UpContractCommand {
 								spinner.error(format!(
 									"An error occurred uploading your contract: {e}"
 								));
-								terminate_node(process)?;
+								terminate_node(&mut Cli, process)?;
 								Cli.outro_cancel(FAILED)?;
 								return Ok(());
 							},
@@ -243,19 +244,19 @@ impl UpContractCommand {
 				}
 			} else {
 				Cli.outro_cancel("Signed payload doesn't exist.")?;
-				terminate_node(process)?;
+				terminate_node(&mut Cli, process)?;
 				return Ok(());
 			}
 
-			terminate_node(process)?;
+			terminate_node(&mut Cli, process)?;
 			Cli.outro(COMPLETE)?;
 			return Ok(());
 		}
 
 		// Check for upload only.
 		if self.upload_only {
 			let result = self.upload_contract().await;
-			terminate_node(process)?;
+			terminate_node(&mut Cli, process)?;
 			match result {
 				Ok(_) => {
 					Cli.outro(COMPLETE)?;
@@ -272,7 +273,7 @@ impl UpContractCommand {
 			Ok(i) => i,
 			Err(e) => {
 				error(format!("An error occurred instantiating the contract: {e}"))?;
-				terminate_node(process)?;
+				terminate_node(&mut Cli, process)?;
 				Cli.outro_cancel(FAILED)?;
 				return Ok(());
 			},
@@ -290,7 +291,7 @@ impl UpContractCommand {
 				},
 				Err(e) => {
 					spinner.error(format!("{e}"));
-					terminate_node(process)?;
+					terminate_node(&mut Cli, process)?;
 					Cli.outro_cancel(FAILED)?;
 					return Ok(());
 				},
@@ -308,7 +309,7 @@ impl UpContractCommand {
 				contract_info.code_hash,
 			);
 
-			terminate_node(process)?;
+			terminate_node(&mut Cli, process)?;
 			Cli.outro(COMPLETE)?;
 		}
 

diff --git a/crates/pop-cli/src/common/contracts.rs b/crates/pop-cli/src/common/contracts.rs
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-3.0
 
-use cliclack::{confirm, log::warning, spinner};
+use crate::cli::traits::*;
+use cliclack::spinner;
 use pop_common::{manifest::from_path, sourcing::set_executable_permission};
 use pop_contracts::contracts_node_generator;
 use std::{
@@ -14,13 +15,17 @@ use tempfile::NamedTempFile;
 ///
 /// # Arguments
 /// * `skip_confirm`: A boolean indicating whether to skip confirmation prompts.
-pub async fn check_contracts_node_and_prompt(skip_confirm: bool) -> anyhow::Result<PathBuf> {
+pub async fn check_contracts_node_and_prompt(
+	cli: &mut impl Cli,
+	skip_confirm: bool,
+) -> anyhow::Result<PathBuf> {
 	let cache_path: PathBuf = crate::cache()?;
 	let mut binary = contracts_node_generator(cache_path, None).await?;
 	let mut node_path = binary.path();
 	if !binary.exists() {
-		warning("⚠️ The substrate-contracts-node binary is not found.")?;
-		if confirm("📦 Would you like to source it automatically now?")
+		cli.warning("⚠️ The substrate-contracts-node binary is not found.")?;
+		if cli
+			.confirm("📦 Would you like to source it automatically now?")
 			.initial_value(true)
 			.interact()?
 		{
@@ -37,14 +42,14 @@ pub async fn check_contracts_node_and_prompt(skip_confirm: bool) -> anyhow::Resu
 		}
 	}
 	if binary.stale() {
-		warning(format!(
+		cli.warning(format!(
 			"ℹ️ There is a newer version of {} available:\n {} -> {}",
 			binary.name(),
 			binary.version().unwrap_or("None"),
 			binary.latest().unwrap_or("None")
 		))?;
 		let latest = if !skip_confirm {
-			confirm(
+			cli.confirm(
 				"📦 Would you like to source it automatically now? It may take some time..."
 					.to_string(),
 			)
@@ -73,12 +78,16 @@ pub async fn check_contracts_node_and_prompt(skip_confirm: bool) -> anyhow::Resu
 }
 
 /// Handles the optional termination of a local running node.
-pub fn terminate_node(process: Option<(Child, NamedTempFile)>) -> anyhow::Result<()> {
+pub fn terminate_node(
+	cli: &mut impl Cli,
+	process: Option<(Child, NamedTempFile)>,
+) -> anyhow::Result<()> {
 	// Prompt to close any launched node
 	let Some((process, log)) = process else {
 		return Ok(());
 	};
-	if confirm("Would you like to terminate the local node?")
+	if cli
+		.confirm("Would you like to terminate the local node?")
 		.initial_value(true)
 		.interact()?
 	{
@@ -89,7 +98,7 @@ pub fn terminate_node(process: Option<(Child, NamedTempFile)>) -> anyhow::Result
 			.wait()?;
 	} else {
 		log.keep()?;
-		warning(format!("NOTE: The node is running in the background with process ID {}. Please terminate it manually when done.", process.id()))?;
+		cli.warning(format!("NOTE: The node is running in the background with process ID {}. Please terminate it manually when done.", process.id()))?;
 	}
 
 	Ok(())
@@ -117,7 +126,14 @@ mod tests {
 	use super::*;
 	use crate::cli::MockCli;
 	use duct::cmd;
-	use std::fs::{self, File};
+	use pop_common::find_free_port;
+	use pop_contracts::{is_chain_alive, run_contracts_node};
+	use std::{
+		fs::{self, File},
+		thread::sleep,
+		time::Duration,
+	};
+	use url::Url;
 
 	#[test]
 	fn has_contract_been_built_works() -> anyhow::Result<()> {
@@ -143,25 +159,36 @@ mod tests {
 	#[tokio::test]
 	async fn check_contracts_node_and_prompt_works() -> anyhow::Result<()> {
 		let cache_path: PathBuf = crate::cache()?;
-		let cli = MockCli::new()
+		let mut cli = MockCli::new()
 			.expect_warning("⚠️ The substrate-contracts-node binary is not found.")
 			.expect_confirm("📦 Would you like to source it automatically now?", true)
 			.expect_warning("⚠️ The substrate-contracts-node binary is not found.");
 
-		let node_path = check_contracts_node_and_prompt(false).await?;
+		let node_path = check_contracts_node_and_prompt(&mut cli, false).await?;
 		// Binary path is at least equals cache path + "substrate-contracts-node".
 		assert!(node_path
 			.to_str()
 			.unwrap()
 			.starts_with(&cache_path.join("substrate-contracts-node").to_str().unwrap()));
+		cli.verify()?;
 		Ok(())
 	}
 
 	#[tokio::test]
 	async fn node_is_terminated() -> anyhow::Result<()> {
-		let cli = MockCli::new()
-			.expect_confirm("Would you like to terminate the local node?", false)
-			.expect_warning("NOTE: The node is running in the background with process ID 0. Please terminate it manually when done.");
+		let cache = tempfile::tempdir().expect("Could not create temp dir");
+		let binary = contracts_node_generator(PathBuf::from(cache.path()), None).await?;
+		binary.source(false, &(), true).await?;
+		set_executable_permission(binary.path())?;
+		let port = find_free_port(None);
+		let process = run_contracts_node(binary.path(), None, port).await?;
+		let log = NamedTempFile::new()?;
+		// Terminate the process.
+		let mut cli =
+			MockCli::new().expect_confirm("Would you like to terminate the local node?", true);
+		assert!(terminate_node(&mut cli, Some((process, log))).is_ok());
+		cli.verify()?;
+		assert_eq!(is_chain_alive(Url::parse(&format!("ws:localhost:{}", port))?).await?, false);
 		Ok(())
 	}
 }