Skip to content

Commit

Permalink
fix: build spec experience (#331)
Browse files Browse the repository at this point in the history
* fix: add chain to specify the chain specification

* fix: default_bootnode by default to true

* chore: fmt

* chore: deprecate flag --release in build specs

* fix: clean output (#334)

* fix: undo deprecation of --release flag

* refactor: small fix

* style: remove extra space

* fix(spec): better handling of spinner

* style: use spinner instead of multispinner

* docs: help message to include build

* feat: reuse existing chain spec

* refactor: remove clone

* refactor: opt in to edit provided chain spec

* docs: improve

* refactor: flow flag input

* fix: prepare_output_path

* refactor: resolve small improvements

* fix: protocol id prompt

* fix: spinner

* fix: docs

* test: test cli

* chore: refactor

* chore: amend test

* feat: production profile

* refactor: improve profile experience

* chore: feedback and rebase

* chore: add profile tests

* fix(test): parachain_lifecycle

* style: fmt

* fix: clippy

* fix: cli required changes introduced by PR

* fix: test

* fix: clippy

* docs: deprecation message

---------

Co-authored-by: Alejandro Martinez Andres <[email protected]>
Co-authored-by: Daanvdplas <[email protected]>
  • Loading branch information
3 people authored Dec 5, 2024
1 parent d1cf48d commit 3f86d18
Show file tree
Hide file tree
Showing 15 changed files with 1,088 additions and 363 deletions.
10 changes: 10 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ tar = "0.4.40"
tempfile = "3.10"
thiserror = "1.0.58"
tokio-test = "0.4.4"
toml = "0.5.0"

# networking
reqwest = { version = "0.12", features = ["json"] }
Expand Down
67 changes: 51 additions & 16 deletions crates/pop-cli/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ pub(crate) mod traits {

/// A select prompt.
pub trait Select<T> {
/// Sets the initially selected value.
fn initial_value(self, initial_value: T) -> Self;
/// Starts the prompt interaction.
fn interact(&mut self) -> Result<T>;
/// Adds an item to the selection prompt.
Expand Down Expand Up @@ -133,20 +135,22 @@ impl traits::Cli for Cli {

/// A confirmation prompt using cliclack.
struct Confirm(cliclack::Confirm);

impl traits::Confirm for Confirm {
/// Starts the prompt interaction.
fn interact(&mut self) -> Result<bool> {
self.0.interact()
}
/// Sets the initially selected value.
fn initial_value(mut self, initial_value: bool) -> Self {
self.0 = self.0.initial_value(initial_value);
self
}
/// Starts the prompt interaction.
fn interact(&mut self) -> Result<bool> {
self.0.interact()
}
}

/// A input prompt using cliclack.
struct Input(cliclack::Input);

impl traits::Input for Input {
/// Sets the default value for the input.
fn default_input(mut self, value: &str) -> Self {
Expand Down Expand Up @@ -203,6 +207,12 @@ impl<T: Clone + Eq> traits::MultiSelect<T> for MultiSelect<T> {
struct Select<T: Clone + Eq>(cliclack::Select<T>);

impl<T: Clone + Eq> traits::Select<T> for Select<T> {
/// Sets the initially selected value.
fn initial_value(mut self, initial_value: T) -> Self {
self.0 = self.0.initial_value(initial_value);
self
}

/// Starts the prompt interaction.
fn interact(&mut self) -> Result<T> {
self.0.interact()
Expand Down Expand Up @@ -231,8 +241,7 @@ pub(crate) mod tests {
multiselect_expectation:
Option<(String, Option<bool>, bool, Option<Vec<(String, String)>>)>,
outro_cancel_expectation: Option<String>,
select_expectation:
Option<(String, Option<bool>, bool, Option<Vec<(String, String)>>, usize)>,
select_expectation: Vec<(String, Option<bool>, bool, Option<Vec<(String, String)>>, usize)>,
success_expectations: Vec<String>,
warning_expectations: Vec<String>,
}
Expand All @@ -243,17 +252,17 @@ pub(crate) mod tests {
}

pub(crate) fn expect_confirm(mut self, prompt: impl Display, confirm: bool) -> Self {
self.confirm_expectation.push((prompt.to_string(), confirm));
self.confirm_expectation.insert(0, (prompt.to_string(), confirm));
self
}

pub(crate) fn expect_input(mut self, prompt: impl Display, input: String) -> Self {
self.input_expectations.push((prompt.to_string(), input));
self.input_expectations.insert(0, (prompt.to_string(), input));
self
}

pub(crate) fn expect_info(mut self, message: impl Display) -> Self {
self.info_expectations.push(message.to_string());
self.info_expectations.insert(0, message.to_string());
self
}

Expand Down Expand Up @@ -283,15 +292,16 @@ pub(crate) mod tests {
self
}

pub(crate) fn expect_select<T>(
pub(crate) fn expect_select(
mut self,
prompt: impl Display,
required: Option<bool>,
collect: bool,
items: Option<Vec<(String, String)>>,
item: usize,
) -> Self {
self.select_expectation = Some((prompt.to_string(), required, collect, items, item));
self.select_expectation
.insert(0, (prompt.to_string(), required, collect, items, item));
self
}

Expand Down Expand Up @@ -327,8 +337,15 @@ pub(crate) mod tests {
if let Some(expectation) = self.outro_cancel_expectation {
panic!("`{expectation}` outro cancel expectation not satisfied")
}
if let Some((prompt, _, _, _, _)) = self.select_expectation {
panic!("`{prompt}` select prompt expectation not satisfied")
if !self.select_expectation.is_empty() {
panic!(
"`{}` select prompt expectation not satisfied",
self.select_expectation
.iter()
.map(|(s, _, _, _, _)| s.clone()) // Extract the `String` part
.collect::<Vec<_>>()
.join(", ")
);
}
if !self.success_expectations.is_empty() {
panic!(
Expand Down Expand Up @@ -425,10 +442,16 @@ pub(crate) mod tests {
fn select<T: Clone + Eq>(&mut self, prompt: impl Display) -> impl Select<T> {
let prompt = prompt.to_string();
if let Some((expectation, _, collect, items_expectation, item)) =
self.select_expectation.take()
self.select_expectation.pop()
{
assert_eq!(expectation, prompt, "prompt does not satisfy expectation");
return MockSelect { items_expectation, collect, items: vec![], item };
return MockSelect {
items_expectation,
collect,
items: vec![],
item,
initial_value: None,
};
}

MockSelect::default()
Expand Down Expand Up @@ -553,15 +576,27 @@ pub(crate) mod tests {
collect: bool,
items: Vec<T>,
item: usize,
initial_value: Option<T>,
}

impl<T> MockSelect<T> {
pub(crate) fn default() -> Self {
Self { items_expectation: None, collect: false, items: vec![], item: 0 }
Self {
items_expectation: None,
collect: false,
items: vec![],
item: 0,
initial_value: None,
}
}
}

impl<T: Clone + Eq> Select<T> for MockSelect<T> {
fn initial_value(mut self, initial_value: T) -> Self {
self.initial_value = Some(initial_value);
self
}

fn interact(&mut self) -> Result<T> {
Ok(self.items[self.item].clone())
}
Expand Down
80 changes: 49 additions & 31 deletions crates/pop-cli/src/commands/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use clap::{Args, Subcommand};
#[cfg(feature = "contract")]
use contract::BuildContractCommand;
use duct::cmd;
use pop_common::Profile;
use std::path::PathBuf;
#[cfg(feature = "parachain")]
use {parachain::BuildParachainCommand, spec::BuildSpecCommand};
Expand All @@ -29,8 +30,11 @@ pub(crate) struct BuildArgs {
#[arg(short = 'p', long)]
pub(crate) package: Option<String>,
/// For production, always build in release mode to exclude debug features.
#[clap(short, long)]
#[clap(short, long, conflicts_with = "profile")]
pub(crate) release: bool,
/// Build profile [default: debug].
#[clap(long, value_enum)]
pub(crate) profile: Option<Profile>,
/// Parachain ID to be used when generating the chain spec files.
#[arg(short = 'i', long = "id")]
#[cfg(feature = "parachain")]
Expand Down Expand Up @@ -62,19 +66,26 @@ impl Command {
#[cfg(feature = "contract")]
if pop_contracts::is_supported(args.path.as_deref())? {
// All commands originating from root command are valid
BuildContractCommand { path: args.path, release: args.release, valid: true }
.execute()?;
let release = match args.profile {
Some(profile) => profile.into(),
None => args.release,
};
BuildContractCommand { path: args.path, release, valid: true }.execute()?;
return Ok("contract");
}

// If only parachain feature enabled, build as parachain
#[cfg(feature = "parachain")]
if pop_parachains::is_supported(args.path.as_deref())? {
let profile = match args.profile {
Some(profile) => profile,
None => args.release.into(),
};
// All commands originating from root command are valid
BuildParachainCommand {
path: args.path,
package: args.package,
release: args.release,
profile: Some(profile),
id: args.id,
valid: true,
}
Expand All @@ -101,13 +112,15 @@ impl Command {
_args.push("--package");
_args.push(package)
}
if args.release {
let profile = args.profile.unwrap_or(Profile::Debug);
if profile == Profile::Release {
_args.push("--release");
} else if profile == Profile::Production {
_args.push("--profile=production");
}
cmd("cargo", _args).dir(args.path.unwrap_or_else(|| "./".into())).run()?;

let mode = if args.release { "RELEASE" } else { "DEBUG" };
cli.info(format!("The {project} was built in {mode} mode."))?;
cli.info(format!("The {project} was built in {} mode.", profile))?;
cli.outro("Build completed successfully!")?;
Ok(project)
}
Expand All @@ -117,41 +130,46 @@ impl Command {
mod tests {
use super::*;
use cli::MockCli;
use pop_common::manifest::add_production_profile;
use strum::VariantArray;

#[test]
fn build_works() -> anyhow::Result<()> {
let name = "hello_world";
let temp_dir = tempfile::tempdir()?;
let path = temp_dir.path();
let project_path = path.join(name);
cmd("cargo", ["new", name, "--bin"]).dir(&path).run()?;
add_production_profile(&project_path)?;

for package in [None, Some(name.to_string())] {
for release in [false, true] {
let project = if package.is_some() { "package" } else { "project" };
let mode = if release { "RELEASE" } else { "DEBUG" };
let mut cli = MockCli::new()
.expect_intro(format!("Building your {project}"))
.expect_info(format!("The {project} was built in {mode} mode."))
.expect_outro("Build completed successfully!");

assert_eq!(
Command::build(
BuildArgs {
command: None,
path: Some(path.join(name)),
package: package.clone(),
release,
id: None,
},
&mut cli,
)?,
project
);

cli.verify()?;
for release in [true, false] {
for profile in Profile::VARIANTS {
let profile = if release { Profile::Release } else { profile.clone() };
let project = if package.is_some() { "package" } else { "project" };
let mut cli = MockCli::new()
.expect_intro(format!("Building your {project}"))
.expect_info(format!("The {project} was built in {profile} mode."))
.expect_outro("Build completed successfully!");

assert_eq!(
Command::build(
BuildArgs {
command: None,
path: Some(project_path.clone()),
package: package.clone(),
release,
profile: Some(profile.clone()),
id: None,
},
&mut cli,
)?,
project
);
cli.verify()?;
}
}
}

Ok(())
}
}
Loading

0 comments on commit 3f86d18

Please sign in to comment.