Skip to content
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.

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

test: improve coverage #391

Merged
merged 9 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion crates/pop-cli/src/commands/test/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,13 @@ 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,
&crate::cache()?,
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...")?;
Expand Down
26 changes: 16 additions & 10 deletions crates/pop-cli/src/commands/up/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,13 @@ impl UpContractCommand {
let log = NamedTempFile::new()?;

// uses the cache location
let binary_path = match check_contracts_node_and_prompt(self.skip_confirm).await {
let binary_path = match check_contracts_node_and_prompt(
&mut Cli,
&crate::cache()?,
self.skip_confirm,
)
.await
{
Ok(binary_path) => binary_path,
Err(_) => {
Cli.outro_cancel(
Expand Down Expand Up @@ -181,7 +187,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(());
},
Expand All @@ -200,7 +206,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(());
},
Expand All @@ -223,7 +229,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(());
},
Expand All @@ -243,19 +249,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)?;
Expand All @@ -272,7 +278,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(());
},
Expand All @@ -290,7 +296,7 @@ impl UpContractCommand {
},
Err(e) => {
spinner.error(format!("{e}"));
terminate_node(process)?;
terminate_node(&mut Cli, process)?;
Cli.outro_cancel(FAILED)?;
return Ok(());
},
Expand All @@ -308,7 +314,7 @@ impl UpContractCommand {
contract_info.code_hash,
);

terminate_node(process)?;
terminate_node(&mut Cli, process)?;
Cli.outro(COMPLETE)?;
}

Expand Down
74 changes: 63 additions & 11 deletions crates/pop-cli/src/common/contracts.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -13,14 +14,20 @@ use tempfile::NamedTempFile;
/// prompts the user to update it if the existing binary is not the latest version.
///
/// # Arguments
/// * `cli`: Command line interface.
/// * `cache_path`: The cache directory path.
/// * `skip_confirm`: A boolean indicating whether to skip confirmation prompts.
pub async fn check_contracts_node_and_prompt(skip_confirm: bool) -> anyhow::Result<PathBuf> {
let cache_path: PathBuf = crate::cache()?;
let mut binary = contracts_node_generator(cache_path, None).await?;
pub async fn check_contracts_node_and_prompt(
cli: &mut impl Cli,
cache_path: &Path,
skip_confirm: bool,
) -> anyhow::Result<PathBuf> {
let mut binary = contracts_node_generator(PathBuf::from(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()?
{
Expand All @@ -37,14 +44,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(),
)
Expand Down Expand Up @@ -73,12 +80,19 @@ 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<()> {
/// # Arguments
/// * `cli`: Command line interface.
/// * `process`: Tuple identifying the child process to terminate and its log file.
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()?
{
Expand All @@ -89,7 +103,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(())
Expand All @@ -115,8 +129,12 @@ pub fn has_contract_been_built(path: Option<&Path>) -> bool {
#[cfg(test)]
mod tests {
use super::*;
use crate::cli::MockCli;
use duct::cmd;
use pop_common::find_free_port;
use pop_contracts::{is_chain_alive, run_contracts_node};
use std::fs::{self, File};
use url::Url;

#[test]
fn has_contract_been_built_works() -> anyhow::Result<()> {
Expand All @@ -138,4 +156,38 @@ mod tests {
assert!(has_contract_been_built(Some(&path.join(name))));
Ok(())
}

#[tokio::test]
async fn check_contracts_node_and_prompt_works() -> anyhow::Result<()> {
let cache_path = tempfile::tempdir().expect("Could create temp dir");
AlexD10S marked this conversation as resolved.
Show resolved Hide resolved
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(&mut cli, cache_path.path(), false).await?;
// Binary path is at least equal to the cache path + "substrate-contracts-node".
assert!(node_path
.to_str()
.unwrap()
.starts_with(&cache_path.path().join("substrate-contracts-node").to_str().unwrap()));
cli.verify()
}

#[tokio::test]
async fn node_is_terminated() -> anyhow::Result<()> {
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());
assert_eq!(is_chain_alive(Url::parse(&format!("ws://localhost:{}", port))?).await?, false);
cli.verify()
}
}
Loading