From ae63db35210d4ac61e54e3616e380e6baf4afea5 Mon Sep 17 00:00:00 2001 From: Lukas Wagner Date: Sat, 2 May 2026 21:03:32 +0200 Subject: [PATCH 1/2] Introduce CliWriter trait to decouple CLI output from println! Replace bare println!/eprintln!/print! calls across all command modules with a CliWriter trait abstraction. Commands now receive &dyn CliWriter, enabling testable output without filesystem or terminal dependencies. - Add CliWriter trait with semantic methods (line, heading, list_item, indented, detail, blank, message, warn_stderr, raw) - Add StdoutCliWriter for production use (identical output to before) - Add BufferCliWriter (cfg(test)) with typed OutputEntry enum for semantic assertions in unit tests - Wire writer through Commands::execute() into all command modules - Add unit tests for add, manage, and additional_packages output --- crates/cargo-changeset/src/commands/add.rs | 121 ++++++- .../src/commands/additional_packages.rs | 245 ++++++++++--- crates/cargo-changeset/src/commands/init.rs | 138 +++++--- crates/cargo-changeset/src/commands/manage.rs | 168 +++++++-- crates/cargo-changeset/src/commands/mod.rs | 28 +- .../cargo-changeset/src/commands/release.rs | 308 ++++++++++++++-- crates/cargo-changeset/src/commands/status.rs | 6 +- crates/cargo-changeset/src/commands/verify.rs | 193 +++++++++- crates/cargo-changeset/src/output/mod.rs | 5 + crates/cargo-changeset/src/output/writer.rs | 335 ++++++++++++++++++ .../src/operations/release/types.rs | 19 +- .../src/verification/result.rs | 11 +- 12 files changed, 1355 insertions(+), 222 deletions(-) create mode 100644 crates/cargo-changeset/src/output/writer.rs diff --git a/crates/cargo-changeset/src/commands/add.rs b/crates/cargo-changeset/src/commands/add.rs index 9578e6e..01b912c 100644 --- a/crates/cargo-changeset/src/commands/add.rs +++ b/crates/cargo-changeset/src/commands/add.rs @@ -12,8 +12,9 @@ use super::AddArgs; use crate::environment::is_interactive; use crate::error::{CliError, Result}; use crate::interaction::{NonInteractiveProvider, TerminalInteractionProvider}; +use crate::output::CliWriter; -pub(super) fn run(args: AddArgs, start_path: &Path) -> Result<()> { +pub(super) fn run(args: AddArgs, start_path: &Path, writer: &dyn CliWriter) -> Result<()> { validate_package_bump_args(&args.package_bumps)?; let project_provider = FileSystemProjectProvider::new(); @@ -22,7 +23,10 @@ pub(super) fn run(args: AddArgs, start_path: &Path) -> Result<()> { let is_single_package = *project.kind() == ProjectKind::SinglePackage && args.packages.is_empty(); if is_single_package && let Some(pkg) = project.packages().first() { - println!("Using package: {} ({})", pkg.name(), pkg.version()); + writer.message( + crate::output::MessageLevel::Info, + &format!("Using package: {} ({})", pkg.name(), pkg.version()), + ); } let changeset_writer = FileSystemChangesetIO::new(project.root()); @@ -39,33 +43,41 @@ pub(super) fn run(args: AddArgs, start_path: &Path) -> Result<()> { operation.execute(start_path, &input)? }; + print_add_result(&result, writer); + Ok(()) +} + +fn print_add_result(result: &AddResult, writer: &dyn CliWriter) { match result { AddResult::Created { changeset, file_path, uncovered_dependents, } => { - println!(); - println!("Created changeset: {}", file_path.display()); - println!(); - println!("Summary: {}", changeset.summary()); - println!("Category: {}", changeset.category()); - println!(); - println!("Releases:"); + writer.blank(); + writer.line(&format!("Created changeset: {}", file_path.display())); + writer.blank(); + writer.line(&format!("Summary: {}", changeset.summary())); + writer.line(&format!("Category: {}", changeset.category())); + writer.blank(); + writer.heading("Releases:"); for release in changeset.releases() { - println!(" - {}: {}", release.name(), release.bump_type()); + writer.list_item(&format!("{}: {}", release.name(), release.bump_type())); } if !uncovered_dependents.is_empty() { - println!(); - println!( - "Info: The following transitive dependents are not covered by this changeset:" + writer.blank(); + writer.message( + crate::output::MessageLevel::Info, + "Info: The following transitive dependents are not covered by this changeset:", + ); + writer.indented(&uncovered_dependents.join(", ")); + writer.message( + crate::output::MessageLevel::Hint, + "Consider creating separate changesets for these packages.", ); - println!(" {}", uncovered_dependents.join(", ")); - println!("Consider creating separate changesets for these packages."); } - Ok(()) } - AddResult::Cancelled | AddResult::NoPackages => Ok(()), + AddResult::Cancelled | AddResult::NoPackages => {} } } @@ -136,10 +148,14 @@ fn read_description_from_stdin() -> Result { #[cfg(test)] mod tests { - use changeset_core::BumpType; + use std::path::PathBuf; + + use changeset_core::{BumpType, ChangeCategory, Changeset, PackageRelease}; + use changeset_operations::operations::AddResult; - use super::{parse_package_bump, parse_package_bumps}; + use super::{parse_package_bump, parse_package_bumps, print_add_result}; use crate::error::CliError; + use crate::output::{BufferCliWriter, MessageLevel, OutputEntry}; #[test] fn parse_package_bump_valid_major() { @@ -221,4 +237,71 @@ mod tests { assert!(map.is_empty()); } + + #[test] + fn created_changeset_output_includes_summary_and_releases() { + let writer = BufferCliWriter::new(); + let changeset = Changeset::new( + "Fix login bug".to_string(), + vec![PackageRelease::new("my-crate".to_string(), BumpType::Patch)], + ChangeCategory::Fixed, + ); + let result = AddResult::Created { + changeset, + file_path: PathBuf::from(".changeset/abc123.md"), + uncovered_dependents: vec![], + }; + + print_add_result(&result, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Created changeset: .changeset/abc123.md")); + assert!(text.contains("Summary: Fix login bug")); + assert!(text.contains("Category: Fixed")); + assert!(text.contains("my-crate: patch")); + } + + #[test] + fn created_changeset_with_uncovered_dependents_shows_warning() { + let writer = BufferCliWriter::new(); + let changeset = Changeset::new( + "Update API".to_string(), + vec![PackageRelease::new("core".to_string(), BumpType::Minor)], + ChangeCategory::Changed, + ); + let result = AddResult::Created { + changeset, + file_path: PathBuf::from(".changeset/def456.md"), + uncovered_dependents: vec!["pkg-a".to_string(), "pkg-b".to_string()], + }; + + print_add_result(&result, &writer); + + let entries = writer.stdout_entries(); + assert!( + entries.contains(&OutputEntry::Message { + level: MessageLevel::Info, + text: + "Info: The following transitive dependents are not covered by this changeset:" + .to_string(), + }) + ); + assert!(entries.contains(&OutputEntry::Indented("pkg-a, pkg-b".to_string()))); + } + + #[test] + fn cancelled_produces_no_output() { + let writer = BufferCliWriter::new(); + print_add_result(&AddResult::Cancelled, &writer); + + assert!(writer.stdout_entries().is_empty()); + } + + #[test] + fn no_packages_produces_no_output() { + let writer = BufferCliWriter::new(); + print_add_result(&AddResult::NoPackages, &writer); + + assert!(writer.stdout_entries().is_empty()); + } } diff --git a/crates/cargo-changeset/src/commands/additional_packages.rs b/crates/cargo-changeset/src/commands/additional_packages.rs index 7ccee3f..b7ea43b 100644 --- a/crates/cargo-changeset/src/commands/additional_packages.rs +++ b/crates/cargo-changeset/src/commands/additional_packages.rs @@ -25,6 +25,7 @@ use changeset_operations::traits::{ use crate::environment::is_interactive; use crate::error::{CliError, Result}; use crate::interaction::{AdditionalPackageFieldSelectionOption, select_variant}; +use crate::output::CliWriter; #[derive(Args)] pub(crate) struct AdditionalPackagesArgs { @@ -354,17 +355,25 @@ impl AdditionalPackageInteractionProvider for TerminalAdditionalPackageInteracti } } -pub(crate) fn run(args: AdditionalPackagesArgs, start_path: &Path) -> Result<()> { +pub(super) fn run( + args: AdditionalPackagesArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { match args.command { - AdditionalPackageCommand::Add(args) => run_add(args, start_path), - AdditionalPackageCommand::Remove(args) => run_remove(args, start_path), - AdditionalPackageCommand::Edit(args) => run_edit(args, start_path), - AdditionalPackageCommand::List => run_list(start_path), - AdditionalPackageCommand::Dependencies(args) => run_dependencies(args, start_path), + AdditionalPackageCommand::Add(args) => run_add(args, start_path, writer), + AdditionalPackageCommand::Remove(args) => run_remove(args, start_path, writer), + AdditionalPackageCommand::Edit(args) => run_edit(args, start_path, writer), + AdditionalPackageCommand::List => run_list(start_path, writer), + AdditionalPackageCommand::Dependencies(args) => run_dependencies(args, start_path, writer), } } -fn run_add(args: AddAdditionalPackageArgs, start_path: &Path) -> Result<()> { +fn run_add( + args: AddAdditionalPackageArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let manifest_format = args.manifest_format.or_else(|| { args.manifest_file .as_deref() @@ -412,11 +421,15 @@ fn run_add(args: AddAdditionalPackageArgs, start_path: &Path) -> Result<()> { return Err(CliError::IncompleteArgs); }; - print_additional_package_events(&events); + print_additional_package_events(&events, writer); Ok(()) } -fn run_remove(args: RemoveAdditionalPackageArgs, start_path: &Path) -> Result<()> { +fn run_remove( + args: RemoveAdditionalPackageArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let events = if let Some(name) = args.name { let op = AdditionalPackageDirectRemoveOperation::new(project_provider(), manifest_writer()); op.execute(start_path, &name)? @@ -431,11 +444,15 @@ fn run_remove(args: RemoveAdditionalPackageArgs, start_path: &Path) -> Result<() return Err(CliError::IncompleteArgs); }; - print_additional_package_events(&events); + print_additional_package_events(&events, writer); Ok(()) } -fn run_edit(args: EditAdditionalPackageArgs, start_path: &Path) -> Result<()> { +fn run_edit( + args: EditAdditionalPackageArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let has_updates = args.path.is_some() || !args.influence.is_empty() || args.manifest_file.is_some() @@ -468,26 +485,34 @@ fn run_edit(args: EditAdditionalPackageArgs, start_path: &Path) -> Result<()> { return Err(CliError::IncompleteArgs); }; - print_additional_package_events(&events); + print_additional_package_events(&events, writer); Ok(()) } -fn run_list(start_path: &Path) -> Result<()> { +fn run_list(start_path: &Path, writer: &dyn CliWriter) -> Result<()> { let op = AdditionalPackageListOperation::new(project_provider()); let events = op.execute(start_path)?; - print_additional_package_events(&events); + print_additional_package_events(&events, writer); Ok(()) } -fn run_dependencies(args: DependenciesArgs, start_path: &Path) -> Result<()> { +fn run_dependencies( + args: DependenciesArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { match args.command { - DependencySubcommand::Add(args) => run_dependency_add(args, start_path), - DependencySubcommand::Remove(args) => run_dependency_remove(args, start_path), - DependencySubcommand::List(args) => run_dependency_list(args, start_path), + DependencySubcommand::Add(args) => run_dependency_add(args, start_path, writer), + DependencySubcommand::Remove(args) => run_dependency_remove(args, start_path, writer), + DependencySubcommand::List(args) => run_dependency_list(args, start_path, writer), } } -fn run_dependency_add(args: AddDependencyArgs, start_path: &Path) -> Result<()> { +fn run_dependency_add( + args: AddDependencyArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let manifest_format = args.manifest_format.or_else(|| { args.manifest_file .as_deref() @@ -548,7 +573,7 @@ fn run_dependency_add(args: AddDependencyArgs, start_path: &Path) -> Result<()> .collect(); if dep_items.is_empty() { - println!("No other packages available to select as a dependency."); + writer.line("No other packages available to select as a dependency."); return Ok(()); } @@ -576,11 +601,15 @@ fn run_dependency_add(args: AddDependencyArgs, start_path: &Path) -> Result<()> return Err(CliError::IncompleteArgs); }; - print_dependency_events(&events); + print_dependency_events(&events, writer); Ok(()) } -fn run_dependency_remove(args: RemoveDependencyArgs, start_path: &Path) -> Result<()> { +fn run_dependency_remove( + args: RemoveDependencyArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let events = if let (Some(package), Some(dependency)) = (args.package, args.dependency) { execute_dependency_remove(package, dependency, start_path)? } else if is_interactive() { @@ -607,7 +636,9 @@ fn run_dependency_remove(args: RemoveDependencyArgs, start_path: &Path) -> Resul ); if dep_items.is_empty() { - println!("No version-tracking dependencies configured for '{package}'."); + writer.line(&format!( + "No version-tracking dependencies configured for '{package}'." + )); return Ok(()); } @@ -622,11 +653,15 @@ fn run_dependency_remove(args: RemoveDependencyArgs, start_path: &Path) -> Resul return Err(CliError::IncompleteArgs); }; - print_dependency_events(&events); + print_dependency_events(&events, writer); Ok(()) } -fn run_dependency_list(args: ListDependencyArgs, start_path: &Path) -> Result<()> { +fn run_dependency_list( + args: ListDependencyArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let events = if let Some(package) = args.package { let op = VersionTrackingDependencyListOperation::new(project_provider()); op.execute(start_path, &package)? @@ -649,7 +684,7 @@ fn run_dependency_list(args: ListDependencyArgs, start_path: &Path) -> Result<() return Err(CliError::IncompleteArgs); }; - print_dependency_events(&events); + print_dependency_events(&events, writer); Ok(()) } @@ -682,79 +717,83 @@ fn execute_dependency_remove( Ok(op.execute(start_path, input)?) } -fn print_dependency_events(events: &[VersionTrackingDependencyEvent]) { +fn print_dependency_events(events: &[VersionTrackingDependencyEvent], writer: &dyn CliWriter) { for event in events { match event { VersionTrackingDependencyEvent::Added { package_name, dependency_name, } => { - println!( + writer.line(&format!( "Added version-tracking dependency '{dependency_name}' to package '{package_name}'" - ); + )); } VersionTrackingDependencyEvent::Removed { package_name, dependency_name, } => { - println!( + writer.line(&format!( "Removed version-tracking dependency '{dependency_name}' from package '{package_name}'" - ); + )); } VersionTrackingDependencyEvent::Listed(summaries) => { - println!(); - println!("Version-tracking dependencies:"); + writer.blank(); + writer.heading("Version-tracking dependencies:"); for s in summaries { - println!( - " {} -> {} [{}]", + writer.indented(&format!( + "{} -> {} [{}]", s.dependency_name(), s.manifest_file_path().display(), s.manifest_format() - ); - println!(" version field: {}", s.version_field_path()); + )); + writer.indented(&format!(" version field: {}", s.version_field_path())); } - println!(); + writer.blank(); } VersionTrackingDependencyEvent::NoDependencies { package_name } => { - println!("No version-tracking dependencies configured for '{package_name}'."); + writer.line(&format!( + "No version-tracking dependencies configured for '{package_name}'." + )); } } } } -fn print_additional_package_events(events: &[AdditionalPackageEvent]) { +fn print_additional_package_events(events: &[AdditionalPackageEvent], writer: &dyn CliWriter) { for event in events { match event { AdditionalPackageEvent::Added { name } => { - println!("Added additional package '{name}'"); + writer.line(&format!("Added additional package '{name}'")); } AdditionalPackageEvent::Removed { name } => { - println!("Removed additional package '{name}'"); + writer.line(&format!("Removed additional package '{name}'")); } AdditionalPackageEvent::Updated { name, field } => { - println!("Updated additional package '{name}' (fields: {field})"); + writer.line(&format!( + "Updated additional package '{name}' (fields: {field})" + )); } AdditionalPackageEvent::Listed(summaries) => { - println!(); - println!("Additional packages:"); + writer.blank(); + writer.heading("Additional packages:"); for s in summaries { - println!(" {} ({})", s.name, s.path.display()); - println!( - " manifest: {} [{}]", + writer.indented(&format!("{} ({})", s.name, s.path.display())); + writer.indented(&format!( + " manifest: {} [{}]", s.manifest_file_path.display(), s.manifest_format - ); + )); } - println!(); + writer.blank(); } AdditionalPackageEvent::NotFound { name } => { - println!("Additional package '{name}' not found"); + writer.line(&format!("Additional package '{name}' not found")); } AdditionalPackageEvent::AlreadyExists { name } => { - println!("A package named '{name}' already exists"); + writer.line(&format!("A package named '{name}' already exists")); } AdditionalPackageEvent::NoAdditionalPackages => { - println!("No additional packages configured."); + writer.line("No additional packages configured."); } } } @@ -1013,4 +1052,104 @@ mod tests { fn manifest_format_from_path_returns_none_for_no_extension() { assert_eq!(ManifestFormat::from_path(Path::new("Makefile")), None); } + + mod output_tests { + use std::path::PathBuf; + + use changeset_core::ManifestFormat; + use changeset_operations::operations::{ + AdditionalPackageEvent, AdditionalPackageSummaryData, VersionTrackingDependencyEvent, + }; + + use crate::commands::additional_packages::{ + print_additional_package_events, print_dependency_events, + }; + use crate::output::BufferCliWriter; + + #[test] + fn added_package_shows_name() { + let writer = BufferCliWriter::new(); + let events = vec![AdditionalPackageEvent::Added { + name: "my-chart".to_string(), + }]; + + print_additional_package_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Added additional package 'my-chart'")); + } + + #[test] + fn removed_package_shows_name() { + let writer = BufferCliWriter::new(); + let events = vec![AdditionalPackageEvent::Removed { + name: "old-pkg".to_string(), + }]; + + print_additional_package_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Removed additional package 'old-pkg'")); + } + + #[test] + fn no_additional_packages_message() { + let writer = BufferCliWriter::new(); + let events = vec![AdditionalPackageEvent::NoAdditionalPackages]; + + print_additional_package_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("No additional packages configured.")); + } + + #[test] + fn listed_packages_shows_details() { + let writer = BufferCliWriter::new(); + let events = vec![AdditionalPackageEvent::Listed(vec![ + AdditionalPackageSummaryData { + name: "chart-a".to_string(), + path: PathBuf::from("charts/chart-a"), + manifest_file_path: PathBuf::from("charts/chart-a/Chart.yaml"), + manifest_format: ManifestFormat::Yaml, + }, + ])]; + + print_additional_package_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Additional packages:")); + assert!(text.contains("chart-a")); + assert!(text.contains("charts/chart-a/Chart.yaml")); + } + + #[test] + fn dependency_added_shows_both_names() { + let writer = BufferCliWriter::new(); + let events = vec![VersionTrackingDependencyEvent::Added { + package_name: "my-chart".to_string(), + dependency_name: "core-lib".to_string(), + }]; + + print_dependency_events(&events, &writer); + + let text = writer.stdout_text(); + assert!( + text.contains("Added version-tracking dependency 'core-lib' to package 'my-chart'") + ); + } + + #[test] + fn dependency_no_deps_message() { + let writer = BufferCliWriter::new(); + let events = vec![VersionTrackingDependencyEvent::NoDependencies { + package_name: "lonely-pkg".to_string(), + }]; + + print_dependency_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("No version-tracking dependencies configured for 'lonely-pkg'.")); + } + } } diff --git a/crates/cargo-changeset/src/commands/init.rs b/crates/cargo-changeset/src/commands/init.rs index baa9b9e..fd87d84 100644 --- a/crates/cargo-changeset/src/commands/init.rs +++ b/crates/cargo-changeset/src/commands/init.rs @@ -16,8 +16,9 @@ use crate::commands::InitArgs; use crate::environment::is_interactive; use crate::error::Result; use crate::interaction::{TerminalInitInteractionProvider, confirm_proceed}; +use crate::output::{CliWriter, MessageLevel}; -pub(crate) fn run(args: InitArgs, start_path: &Path) -> Result<()> { +pub(super) fn run(args: InitArgs, start_path: &Path, writer: &dyn CliWriter) -> Result<()> { let project_provider = FileSystemProjectProvider::new(); let manifest_writer = FileSystemManifestWriter::new(); let interaction_provider = TerminalInitInteractionProvider::new(); @@ -39,7 +40,7 @@ pub(crate) fn run(args: InitArgs, start_path: &Path) -> Result<()> { let input = match build_init_input(&args, provider, context) { Ok(input) => input, Err(crate::error::CliError::Operation(changeset_operations::OperationError::Cancelled)) => { - println!("Cancelled."); + writer.line("Cancelled."); return Ok(()); } Err(e) => return Err(e), @@ -67,11 +68,11 @@ pub(crate) fn run(args: InitArgs, start_path: &Path) -> Result<()> { config, ); - print_summary(&plan); + print_summary(&plan, writer); let skip_confirmation = args.defaults || args.no_interactive || !interactive_mode; if !skip_confirmation && !confirm_proceed("Proceed with initialization?")? { - println!("Aborted."); + writer.line("Aborted."); return Ok(()); } @@ -81,31 +82,34 @@ pub(crate) fn run(args: InitArgs, start_path: &Path) -> Result<()> { let output = operation.execute_plan(start_path, &plan)?; - println!(); + writer.blank(); if output.created_dir() { - println!( + writer.line(&format!( "Created changeset directory at '{}'", output.changeset_dir().display() - ); + )); } else { - println!( + writer.line(&format!( "Changeset directory already exists at '{}'", output.changeset_dir().display() - ); + )); } if output.created_gitkeep() { - println!("Created .gitkeep file"); + writer.line("Created .gitkeep file"); } if output.wrote_config() && let Some(section) = output.config_location() { - println!("Wrote configuration to {section} in Cargo.toml"); + writer.line(&format!("Wrote configuration to {section} in Cargo.toml")); } - println!(); - println!("Tip: Use 'cargo changeset additional-packages add' to declare non-Rust packages."); + writer.blank(); + writer.message( + MessageLevel::Hint, + "Tip: Use 'cargo changeset additional-packages add' to declare non-Rust packages.", + ); Ok(()) } @@ -137,94 +141,112 @@ fn has_any_filtering_args(args: &InitArgs) -> bool { !args.ignored_files.is_empty() } -fn print_summary(plan: &InitPlan) { - println!(); - println!("=== Initialization Summary ==="); - println!(); +fn print_summary(plan: &InitPlan, writer: &dyn CliWriter) { + writer.blank(); + writer.heading("=== Initialization Summary ==="); + writer.blank(); if plan.dir_exists() { - println!( + writer.line(&format!( "Directory: {} (already exists)", plan.changeset_dir().display() - ); + )); } else { - println!( + writer.line(&format!( "Directory: {} (will be created)", plan.changeset_dir().display() - ); + )); } if !plan.gitkeep_exists() { - println!(" - .gitkeep file will be created"); + writer.list_item(".gitkeep file will be created"); } if !plan.config().is_empty() { - println!(); - println!( + writer.blank(); + writer.line(&format!( "Configuration to be written to {}:", plan.metadata_section() - ); - print_config_summary(plan.config()); + )); + print_config_summary(plan.config(), writer); } else { - println!(); - println!("No configuration will be written (using defaults)."); + writer.blank(); + writer.line("No configuration will be written (using defaults)."); } - println!(); + writer.blank(); } -fn print_config_summary(config: &InitConfig) { +fn print_config_summary(config: &InitConfig, writer: &dyn CliWriter) { if let Some(commit) = config.commit { - println!(" commit = {commit}"); + writer.detail("commit", &commit.to_string()); } if let Some(tags) = config.tags { - println!(" tags = {tags}"); + writer.detail("tags", &tags.to_string()); } if let Some(keep_changesets) = config.keep_changesets { - println!(" keep_changesets = {keep_changesets}"); + writer.detail("keep_changesets", &keep_changesets.to_string()); } if let Some(ref tag_format) = config.tag_format { - println!(" tag_format = \"{}\"", tag_format.as_str()); + writer.detail("tag_format", &format!("\"{}\"", tag_format.as_str())); } if let Some(ref changelog) = config.changelog { - println!(" changelog = \"{}\"", changelog.as_str()); + writer.detail("changelog", &format!("\"{}\"", changelog.as_str())); } if let Some(ref comparison_links) = config.comparison_links { - println!(" comparison_links = \"{}\"", comparison_links.as_str()); + writer.detail( + "comparison_links", + &format!("\"{}\"", comparison_links.as_str()), + ); } if let Some(ref zero_version_behavior) = config.zero_version_behavior { - println!( - " zero_version_behavior = \"{}\"", - zero_version_behavior.as_str() + writer.detail( + "zero_version_behavior", + &format!("\"{}\"", zero_version_behavior.as_str()), ); } if let Some(ref base_branch) = config.base_branch { - println!(" base_branch = \"{base_branch}\""); + writer.detail("base_branch", &format!("\"{base_branch}\"")); } if let Some(ref none_bump_behavior) = config.none_bump_behavior { - println!(" none_bump_behavior = \"{}\"", none_bump_behavior.as_str()); + writer.detail( + "none_bump_behavior", + &format!("\"{}\"", none_bump_behavior.as_str()), + ); } if let Some(ref none_bump_promote_message_template) = config.none_bump_promote_message_template { - println!(" none_bump_promote_message_template = \"{none_bump_promote_message_template}\""); + writer.detail( + "none_bump_promote_message_template", + &format!("\"{none_bump_promote_message_template}\""), + ); } if let Some(ref commit_title_template) = config.commit_title_template { - println!(" commit_title_template = \"{commit_title_template}\""); + writer.detail( + "commit_title_template", + &format!("\"{commit_title_template}\""), + ); } if let Some(changes_in_body) = config.changes_in_body { - println!(" changes_in_body = {changes_in_body}"); + writer.detail("changes_in_body", &changes_in_body.to_string()); } if let Some(ref comparison_links_template) = config.comparison_links_template { - println!(" comparison_links_template = \"{comparison_links_template}\""); + writer.detail( + "comparison_links_template", + &format!("\"{comparison_links_template}\""), + ); } if let Some(ref dependency_bump_changelog_template) = config.dependency_bump_changelog_template { - println!(" dependency_bump_changelog_template = \"{dependency_bump_changelog_template}\""); + writer.detail( + "dependency_bump_changelog_template", + &format!("\"{dependency_bump_changelog_template}\""), + ); } if let Some(ref ignored_files) = config.ignored_files && !ignored_files.is_empty() { - println!(" ignored_files = {:?}", ignored_files); + writer.detail("ignored_files", &format!("{ignored_files:?}")); } } @@ -307,6 +329,7 @@ fn build_init_input( mod tests { use super::*; use crate::commands::NoneBumpBehaviorArg; + use crate::output::BufferCliWriter; fn default_init_args() -> InitArgs { InitArgs { @@ -352,21 +375,29 @@ mod tests { #[test] fn print_config_summary_includes_none_bump_behavior() { + let writer = BufferCliWriter::new(); let config = changeset_manifest::InitConfig { none_bump_behavior: Some(changeset_manifest::NoneBumpBehavior::Disallow), ..Default::default() }; - print_config_summary(&config); + print_config_summary(&config, &writer); + let text = writer.stdout_text(); + assert!(text.contains("none_bump_behavior")); + assert!(text.contains("disallow")); } #[test] fn print_config_summary_includes_none_bump_promote_message_template() { + let writer = BufferCliWriter::new(); let config = changeset_manifest::InitConfig { none_bump_behavior: Some(changeset_manifest::NoneBumpBehavior::PromoteToPatch), none_bump_promote_message_template: Some("Internal changes".to_string()), ..Default::default() }; - print_config_summary(&config); + print_config_summary(&config, &writer); + let text = writer.stdout_text(); + assert!(text.contains("none_bump_promote_message_template")); + assert!(text.contains("Internal changes")); } #[test] @@ -578,6 +609,7 @@ mod tests { #[test] fn print_config_summary_shows_new_fields() { + let writer = BufferCliWriter::new(); let config = changeset_manifest::InitConfig { commit_title_template: Some("Release {new-version}".to_string()), changes_in_body: Some(true), @@ -590,6 +622,12 @@ mod tests { ignored_files: Some(vec!["*.lock".to_string()]), ..Default::default() }; - print_config_summary(&config); + print_config_summary(&config, &writer); + let text = writer.stdout_text(); + assert!(text.contains("commit_title_template")); + assert!(text.contains("changes_in_body")); + assert!(text.contains("comparison_links_template")); + assert!(text.contains("dependency_bump_changelog_template")); + assert!(text.contains("ignored_files")); } } diff --git a/crates/cargo-changeset/src/commands/manage.rs b/crates/cargo-changeset/src/commands/manage.rs index bbf8242..377a2c9 100644 --- a/crates/cargo-changeset/src/commands/manage.rs +++ b/crates/cargo-changeset/src/commands/manage.rs @@ -17,6 +17,7 @@ use super::{ManageArgs, ManageCommand, ManageGraduationArgs, ManagePrereleaseArg use crate::environment::is_interactive; use crate::error::{CliError, Result}; use crate::interaction::select_from_options; +use crate::output::CliWriter; struct TerminalManageInteractionProvider; @@ -153,14 +154,22 @@ impl GraduationInteractionProvider for TerminalManageInteractionProvider { } } -pub(crate) fn run(args: ManageArgs, start_path: &Path) -> Result<()> { +pub(super) fn run(args: ManageArgs, start_path: &Path, writer: &dyn CliWriter) -> Result<()> { match args.command { - ManageCommand::Prerelease(prerelease_args) => run_prerelease(prerelease_args, start_path), - ManageCommand::Graduation(graduation_args) => run_graduation(graduation_args, start_path), + ManageCommand::Prerelease(prerelease_args) => { + run_prerelease(prerelease_args, start_path, writer) + } + ManageCommand::Graduation(graduation_args) => { + run_graduation(graduation_args, start_path, writer) + } } } -fn run_prerelease(args: ManagePrereleaseArgs, start_path: &Path) -> Result<()> { +fn run_prerelease( + args: ManagePrereleaseArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let no_flags_provided = args.add.is_empty() && args.remove.is_empty() && args.graduate.is_empty() && !args.list; @@ -185,11 +194,15 @@ fn run_prerelease(args: ManagePrereleaseArgs, start_path: &Path) -> Result<()> { operation.execute(start_path, &input)? }; - print_prerelease_events(&events); + print_prerelease_events(&events, writer); Ok(()) } -fn run_graduation(args: ManageGraduationArgs, start_path: &Path) -> Result<()> { +fn run_graduation( + args: ManageGraduationArgs, + start_path: &Path, + writer: &dyn CliWriter, +) -> Result<()> { let no_flags_provided = args.add.is_empty() && args.remove.is_empty() && !args.list; let events = if no_flags_provided { @@ -213,82 +226,175 @@ fn run_graduation(args: ManageGraduationArgs, start_path: &Path) -> Result<()> { operation.execute(start_path, &input)? }; - print_graduation_events(&events); + print_graduation_events(&events, writer); Ok(()) } -fn print_prerelease_events(events: &[PrereleaseEvent]) { +fn print_prerelease_events(events: &[PrereleaseEvent], writer: &dyn CliWriter) { for event in events { match event { PrereleaseEvent::DisplayState(items) => { - println!(); + writer.blank(); if items.is_empty() { - println!("(No packages in pre-release mode)"); + writer.line("(No packages in pre-release mode)"); } else { - println!("Pre-release configuration (.changeset/pre-release.toml):"); + writer.heading("Pre-release configuration (.changeset/pre-release.toml):"); let mut sorted = items.clone(); sorted.sort_by(|a, b| a.0.cmp(&b.0)); for (crate_name, tag) in &sorted { - println!(" {crate_name}: {tag}"); + writer.indented(&format!("{crate_name}: {tag}")); } } - println!(); + writer.blank(); } PrereleaseEvent::Added { crate_name, tag } => { - println!("Added {crate_name} to pre-release configuration with tag '{tag}'"); + writer.line(&format!( + "Added {crate_name} to pre-release configuration with tag '{tag}'" + )); } PrereleaseEvent::Removed { crate_name } => { - println!("Removed {crate_name} from pre-release configuration"); + writer.line(&format!( + "Removed {crate_name} from pre-release configuration" + )); } PrereleaseEvent::MovedToGraduation { crate_name } => { - println!("Moved {crate_name} to graduation queue"); + writer.line(&format!("Moved {crate_name} to graduation queue")); } PrereleaseEvent::AllPackagesInPrerelease => { - println!("All packages are already in pre-release mode."); + writer.line("All packages are already in pre-release mode."); } PrereleaseEvent::NoPrereleasePackages => { - println!("No packages are currently in pre-release mode."); + writer.line("No packages are currently in pre-release mode."); } PrereleaseEvent::NoEligibleForGraduation => { - println!( - "No eligible packages for graduation (must be 0.x stable version and not already queued)." + writer.line( + "No eligible packages for graduation (must be 0.x stable version and not already queued).", ); } } } } -fn print_graduation_events(events: &[GraduationEvent]) { +fn print_graduation_events(events: &[GraduationEvent], writer: &dyn CliWriter) { for event in events { match event { GraduationEvent::DisplayState(items) => { - println!(); + writer.blank(); if items.is_empty() { - println!("(No packages queued for graduation)"); + writer.line("(No packages queued for graduation)"); } else { - println!("Graduation queue (.changeset/graduation.toml):"); + writer.heading("Graduation queue (.changeset/graduation.toml):"); let mut sorted = items.clone(); sorted.sort(); for crate_name in &sorted { - println!(" - {crate_name}"); + writer.list_item(crate_name); } } - println!(); + writer.blank(); } GraduationEvent::Added { crate_name } => { - println!("Added {crate_name} to graduation queue"); + writer.line(&format!("Added {crate_name} to graduation queue")); } GraduationEvent::Removed { crate_name } => { - println!("Removed {crate_name} from graduation queue"); + writer.line(&format!("Removed {crate_name} from graduation queue")); } GraduationEvent::NoEligibleForGraduation => { - println!( - "No eligible packages for graduation (must be 0.x stable version and not already queued)." + writer.line( + "No eligible packages for graduation (must be 0.x stable version and not already queued).", ); } GraduationEvent::NoGraduationPackages => { - println!("No packages are currently queued for graduation."); + writer.line("No packages are currently queued for graduation."); } } } } + +#[cfg(test)] +mod tests { + use changeset_operations::operations::{GraduationEvent, PrereleaseEvent}; + + use super::{print_graduation_events, print_prerelease_events}; + use crate::output::{BufferCliWriter, OutputEntry}; + + #[test] + fn prerelease_display_state_empty_shows_no_packages_message() { + let writer = BufferCliWriter::new(); + let events = vec![PrereleaseEvent::DisplayState(vec![])]; + + print_prerelease_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("(No packages in pre-release mode)")); + } + + #[test] + fn prerelease_display_state_with_entries_shows_sorted() { + let writer = BufferCliWriter::new(); + let events = vec![PrereleaseEvent::DisplayState(vec![ + ("z-crate".to_string(), "beta".to_string()), + ("a-crate".to_string(), "alpha".to_string()), + ])]; + + print_prerelease_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Pre-release configuration")); + let a_pos = text.find("a-crate").expect("a-crate present"); + let z_pos = text.find("z-crate").expect("z-crate present"); + assert!(a_pos < z_pos, "entries should be sorted"); + } + + #[test] + fn prerelease_added_shows_confirmation() { + let writer = BufferCliWriter::new(); + let events = vec![PrereleaseEvent::Added { + crate_name: "my-crate".to_string(), + tag: "rc".to_string(), + }]; + + print_prerelease_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Added my-crate to pre-release configuration with tag 'rc'")); + } + + #[test] + fn graduation_display_state_empty() { + let writer = BufferCliWriter::new(); + let events = vec![GraduationEvent::DisplayState(vec![])]; + + print_graduation_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("(No packages queued for graduation)")); + } + + #[test] + fn graduation_display_state_with_entries_shows_sorted_list_items() { + let writer = BufferCliWriter::new(); + let events = vec![GraduationEvent::DisplayState(vec![ + "z-pkg".to_string(), + "a-pkg".to_string(), + ])]; + + print_graduation_events(&events, &writer); + + let entries = writer.stdout_entries(); + assert!(entries.contains(&OutputEntry::ListItem("a-pkg".to_string()))); + assert!(entries.contains(&OutputEntry::ListItem("z-pkg".to_string()))); + } + + #[test] + fn graduation_added_shows_confirmation() { + let writer = BufferCliWriter::new(); + let events = vec![GraduationEvent::Added { + crate_name: "my-lib".to_string(), + }]; + + print_graduation_events(&events, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Added my-lib to graduation queue")); + } +} diff --git a/crates/cargo-changeset/src/commands/mod.rs b/crates/cargo-changeset/src/commands/mod.rs index 13d0099..2852c57 100644 --- a/crates/cargo-changeset/src/commands/mod.rs +++ b/crates/cargo-changeset/src/commands/mod.rs @@ -17,6 +17,7 @@ use changeset_manifest::{ use changeset_operations::OperationError; use crate::error::Result; +use crate::output::StdoutCliWriter; #[derive(Args)] pub(crate) struct AddArgs { @@ -361,24 +362,37 @@ Use 'cargo changeset manage' to configure these files." impl Commands { pub(crate) fn execute(self, start_path: &Path) -> (Result<()>, ExecuteResult) { + let writer = StdoutCliWriter; match self { - Self::Add(args) => (add::run(args, start_path), ExecuteResult { quiet: false }), + Self::Add(args) => ( + add::run(args, start_path, &writer), + ExecuteResult { quiet: false }, + ), Self::Verify(args) => { let quiet = args.quiet; - (verify::run(args, start_path), ExecuteResult { quiet }) + ( + verify::run(args, start_path, &writer), + ExecuteResult { quiet }, + ) } - Self::Status => (status::run(start_path), ExecuteResult { quiet: false }), + Self::Status => ( + status::run(start_path, &writer), + ExecuteResult { quiet: false }, + ), Self::Release(args) => ( - release::run(args, start_path), + release::run(args, start_path, &writer), + ExecuteResult { quiet: false }, + ), + Self::Init(args) => ( + init::run(args, start_path, &writer), ExecuteResult { quiet: false }, ), - Self::Init(args) => (init::run(args, start_path), ExecuteResult { quiet: false }), Self::Manage(args) => ( - manage::run(args, start_path), + manage::run(args, start_path, &writer), ExecuteResult { quiet: false }, ), Self::AdditionalPackages(args) => ( - additional_packages::run(args, start_path), + additional_packages::run(args, start_path, &writer), ExecuteResult { quiet: false }, ), } diff --git a/crates/cargo-changeset/src/commands/release.rs b/crates/cargo-changeset/src/commands/release.rs index ad695d6..1681847 100644 --- a/crates/cargo-changeset/src/commands/release.rs +++ b/crates/cargo-changeset/src/commands/release.rs @@ -16,6 +16,7 @@ use changeset_version::is_prerelease; use super::ReleaseArgs; use crate::error::Result; +use crate::output::CliWriter; #[derive(Debug, Clone)] pub(crate) struct ParsedPrereleaseArgs { @@ -29,7 +30,7 @@ struct ParsedGraduateArgs { all: bool, } -pub(crate) fn run(args: ReleaseArgs, start_path: &Path) -> Result<()> { +pub(super) fn run(args: ReleaseArgs, start_path: &Path, writer: &dyn CliWriter) -> Result<()> { let project_provider = FileSystemProjectProvider::new(); let project = project_provider.discover_project(start_path)?; let changeset_io = FileSystemChangesetIO::new(project.root()); @@ -79,7 +80,7 @@ pub(crate) fn run(args: ReleaseArgs, start_path: &Path) -> Result<()> { .expect("all fields have defaults"); let outcome = operation.execute(start_path, &input)?; - print_outcome(&outcome); + print_outcome(&outcome, writer); Ok(()) } @@ -152,94 +153,341 @@ fn parse_graduate_args(args: &[String]) -> ParsedGraduateArgs { ParsedGraduateArgs { packages, all } } -fn print_outcome(outcome: &ReleaseOutcome) { +fn print_outcome(outcome: &ReleaseOutcome, writer: &dyn CliWriter) { match outcome { ReleaseOutcome::NoChangesets => { - println!("No pending changesets to release."); + writer.line("No pending changesets to release."); } ReleaseOutcome::DryRun(output) => { - println!("Dry run - no changes will be made.\n"); - print_release_output(output); + writer.line("Dry run - no changes will be made."); + writer.blank(); + print_release_output(output, writer); } ReleaseOutcome::Executed(output) => { - print_release_output(output); - println!("\nRelease complete."); + print_release_output(output, writer); + writer.blank(); + writer.message(crate::output::MessageLevel::Success, "Release complete."); } } } -fn print_release_output(output: &ReleaseOutput) { +fn print_release_output(output: &ReleaseOutput, writer: &dyn CliWriter) { if output.planned_releases().is_empty() { - println!("No packages to release."); + writer.line("No packages to release."); return; } - println!("Releases:"); + writer.heading("Releases:"); for release in output.planned_releases() { let auto_label = if release.auto_bumped() { " (dependency update)" } else { "" }; - println!( - " - {} {} -> {}{}", + writer.list_item(&format!( + "{} {} -> {}{}", release.name(), release.current_version(), release.new_version(), auto_label - ); + )); } if !output.unchanged_packages().is_empty() { - println!("\nUnchanged packages:"); + writer.blank(); + writer.heading("Unchanged packages:"); for name in output.unchanged_packages() { - println!(" - {name}"); + writer.list_item(name); } } if !output.changelog_updates().is_empty() { - println!("\nChangelogs updated:"); + writer.blank(); + writer.heading("Changelogs updated:"); for update in output.changelog_updates() { let status = if update.created() { "created" } else { "updated" }; - println!(" - {} ({})", update.path().display(), status); + writer.list_item(&format!("{} ({})", update.path().display(), status)); } } if let Some(git_result) = output.git_result() { - print_git_result(git_result); + print_git_result(git_result, writer); } if !output.changesets_consumed().is_empty() { - println!( - "\nConsumed {} changeset file(s)", + writer.blank(); + writer.line(&format!( + "Consumed {} changeset file(s)", output.changesets_consumed().len() - ); + )); } } -fn print_git_result(git_result: &GitOperationResult) { +fn print_git_result(git_result: &GitOperationResult, writer: &dyn CliWriter) { if let Some(commit) = git_result.commit() { - println!( - "\nCommit created: {}", + writer.blank(); + writer.line(&format!( + "Commit created: {}", &commit.sha()[..7.min(commit.sha().len())] - ); + )); } if !git_result.tags_created().is_empty() { - println!("\nTags created:"); + writer.blank(); + writer.heading("Tags created:"); for tag in git_result.tags_created() { - println!(" - {}", tag.name()); + writer.list_item(tag.name()); } } if !git_result.changesets_deleted().is_empty() { - println!( - "\nDeleted {} changeset file(s)", + writer.blank(); + writer.line(&format!( + "Deleted {} changeset file(s)", git_result.changesets_deleted().len() + )); + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use changeset_core::BumpType; + use changeset_operations::operations::{ + ChangelogUpdate, CommitResult, GitOperationResult, PackageVersion, ReleaseOutcome, + ReleaseOutput, TagResult, + }; + use semver::Version; + + use super::{print_git_result, print_outcome, print_release_output}; + use crate::output::BufferCliWriter; + + fn make_package_version(name: &str, from: &str, to: &str, auto_bumped: bool) -> PackageVersion { + PackageVersion::new( + name.to_string(), + Version::parse(from).expect("valid semver"), + Version::parse(to).expect("valid semver"), + BumpType::Minor, + auto_bumped, + ) + } + + fn make_release_output_minimal() -> ReleaseOutput { + ReleaseOutput::new( + vec![make_package_version("my-crate", "1.0.0", "1.1.0", false)], + vec![], + vec![], + vec![], + None, + ) + } + + #[test] + fn print_outcome_no_changesets() { + let writer = BufferCliWriter::new(); + + print_outcome(&ReleaseOutcome::NoChangesets, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("No pending changesets to release.")); + } + + #[test] + fn print_outcome_dry_run_shows_prefix() { + let writer = BufferCliWriter::new(); + let output = make_release_output_minimal(); + + print_outcome(&ReleaseOutcome::DryRun(output), &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Dry run - no changes will be made.")); + assert!(text.contains("my-crate")); + } + + #[test] + fn print_outcome_executed_shows_complete_message() { + let writer = BufferCliWriter::new(); + let output = make_release_output_minimal(); + + print_outcome(&ReleaseOutcome::Executed(output), &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Release complete.")); + assert!(text.contains("my-crate")); + } + + #[test] + fn print_release_output_empty_planned_releases() { + let writer = BufferCliWriter::new(); + let output = ReleaseOutput::new(vec![], vec![], vec![], vec![], None); + + print_release_output(&output, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("No packages to release.")); + } + + #[test] + fn print_release_output_shows_version_transitions() { + let writer = BufferCliWriter::new(); + let output = ReleaseOutput::new( + vec![ + make_package_version("crate-a", "1.0.0", "1.1.0", false), + make_package_version("crate-b", "2.0.0", "2.1.0", true), + ], + vec![], + vec![], + vec![], + None, ); + + print_release_output(&output, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Releases:")); + assert!(text.contains("crate-a 1.0.0 -> 1.1.0")); + assert!(text.contains("crate-b 2.0.0 -> 2.1.0 (dependency update)")); + } + + #[test] + fn print_release_output_shows_unchanged_packages() { + let writer = BufferCliWriter::new(); + let output = ReleaseOutput::new( + vec![make_package_version("crate-a", "1.0.0", "1.1.0", false)], + vec!["unchanged-crate".to_string()], + vec![], + vec![], + None, + ); + + print_release_output(&output, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Unchanged packages:")); + assert!(text.contains("unchanged-crate")); + } + + #[test] + fn print_release_output_shows_changelog_updates() { + let writer = BufferCliWriter::new(); + let output = ReleaseOutput::new( + vec![make_package_version("crate-a", "1.0.0", "1.1.0", false)], + vec![], + vec![], + vec![ + ChangelogUpdate::new( + PathBuf::from("CHANGELOG.md"), + None, + Version::parse("1.1.0").expect("valid semver"), + false, + ), + ChangelogUpdate::new( + PathBuf::from("crates/b/CHANGELOG.md"), + Some("crate-b".to_string()), + Version::parse("2.0.0").expect("valid semver"), + true, + ), + ], + None, + ); + + print_release_output(&output, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Changelogs updated:")); + assert!(text.contains("CHANGELOG.md (updated)")); + assert!(text.contains("crates/b/CHANGELOG.md (created)")); + } + + #[test] + fn print_release_output_shows_consumed_changesets() { + let writer = BufferCliWriter::new(); + let output = ReleaseOutput::new( + vec![make_package_version("crate-a", "1.0.0", "1.1.0", false)], + vec![], + vec![ + PathBuf::from(".changeset/abc.md"), + PathBuf::from(".changeset/def.md"), + ], + vec![], + None, + ); + + print_release_output(&output, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Consumed 2 changeset file(s)")); + } + + #[test] + fn print_git_result_with_commit() { + let writer = BufferCliWriter::new(); + let git = GitOperationResult::new( + Some(CommitResult::new( + "abc1234def5678".to_string(), + "release v1.1.0".to_string(), + )), + vec![], + vec![], + ); + + print_git_result(&git, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Commit created: abc1234")); + } + + #[test] + fn print_git_result_with_tags() { + let writer = BufferCliWriter::new(); + let git = GitOperationResult::new( + None, + vec![ + TagResult::new("v1.1.0".to_string(), "abc1234".to_string()), + TagResult::new("crate-b@2.1.0".to_string(), "abc1234".to_string()), + ], + vec![], + ); + + print_git_result(&git, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Tags created:")); + assert!(text.contains("v1.1.0")); + assert!(text.contains("crate-b@2.1.0")); + } + + #[test] + fn print_git_result_with_deleted_changesets() { + let writer = BufferCliWriter::new(); + let git = GitOperationResult::new( + None, + vec![], + vec![ + PathBuf::from(".changeset/a.md"), + PathBuf::from(".changeset/b.md"), + PathBuf::from(".changeset/c.md"), + ], + ); + + print_git_result(&git, &writer); + + let text = writer.stdout_text(); + assert!(text.contains("Deleted 3 changeset file(s)")); + } + + #[test] + fn print_git_result_empty_is_silent() { + let writer = BufferCliWriter::new(); + let git = GitOperationResult::default(); + + print_git_result(&git, &writer); + + assert!(writer.stdout_entries().is_empty()); } } diff --git a/crates/cargo-changeset/src/commands/status.rs b/crates/cargo-changeset/src/commands/status.rs index c1187d9..936989c 100644 --- a/crates/cargo-changeset/src/commands/status.rs +++ b/crates/cargo-changeset/src/commands/status.rs @@ -7,9 +7,9 @@ use changeset_operations::providers::{ use changeset_operations::traits::ProjectProvider; use crate::error::Result; -use crate::output::{PlainTextStatusFormatter, StatusFormatter}; +use crate::output::{CliWriter, PlainTextStatusFormatter, StatusFormatter}; -pub(crate) fn run(start_path: &Path) -> Result<()> { +pub(super) fn run(start_path: &Path, writer: &dyn CliWriter) -> Result<()> { let project_provider = FileSystemProjectProvider::new(); let project = project_provider.discover_project(start_path)?; let changeset_reader = FileSystemChangesetIO::new(project.root()); @@ -19,7 +19,7 @@ pub(crate) fn run(start_path: &Path) -> Result<()> { let output = operation.execute(start_path)?; let formatter = PlainTextStatusFormatter; - print!("{}", formatter.format_status(&output)); + writer.raw(&formatter.format_status(&output)); Ok(()) } diff --git a/crates/cargo-changeset/src/commands/verify.rs b/crates/cargo-changeset/src/commands/verify.rs index 69cff4f..d8db2bd 100644 --- a/crates/cargo-changeset/src/commands/verify.rs +++ b/crates/cargo-changeset/src/commands/verify.rs @@ -8,9 +8,9 @@ use changeset_operations::traits::ProjectProvider; use super::VerifyArgs; use crate::error::{CliError, Result}; -use crate::output::{OutputFormatter, PlainTextFormatter}; +use crate::output::{CliWriter, OutputFormatter, PlainTextFormatter}; -pub(crate) fn run(args: VerifyArgs, start_path: &Path) -> Result<()> { +pub(super) fn run(args: VerifyArgs, start_path: &Path, writer: &dyn CliWriter) -> Result<()> { let project_provider = FileSystemProjectProvider::new(); let project = project_provider.discover_project(start_path)?; let (root_config, _) = project_provider.load_configs(&project)?; @@ -36,15 +36,21 @@ pub(crate) fn run(args: VerifyArgs, start_path: &Path) -> Result<()> { let result = operation.execute(start_path, &input)?; if result.is_dirty() && !args.quiet { - eprintln!("Dirty working directory detected, verifying uncommitted changes against HEAD"); + writer.warn_stderr( + "Dirty working directory detected, verifying uncommitted changes against HEAD", + ); } + print_outcome(result.outcome(), args.quiet, writer) +} + +fn print_outcome(outcome: &VerifyOutcome, quiet: bool, writer: &dyn CliWriter) -> Result<()> { let formatter = PlainTextFormatter; - match result.outcome() { + match outcome { VerifyOutcome::NoChanges => { - if !args.quiet { - println!("No files changed"); + if !quiet { + writer.line("No files changed"); } Ok(()) } @@ -52,26 +58,28 @@ pub(crate) fn run(args: VerifyArgs, start_path: &Path) -> Result<()> { project_file_count, ignored_file_count, } => { - if !args.quiet { - println!("No packages affected by changes"); + if !quiet { + writer.line("No packages affected by changes"); if *project_file_count > 0 { - println!(" {project_file_count} project-level file(s) changed"); + writer.indented(&format!( + "{project_file_count} project-level file(s) changed" + )); } if *ignored_file_count > 0 { - println!(" {ignored_file_count} file(s) ignored by patterns"); + writer.indented(&format!("{ignored_file_count} file(s) ignored by patterns")); } } Ok(()) } VerifyOutcome::Success(verification) => { - if !args.quiet { - print!("{}", formatter.format_success(verification)); + if !quiet { + writer.raw(&formatter.format_success(verification)); } Ok(()) } VerifyOutcome::Failed(verification) => { - if !args.quiet { - eprint!("{}", formatter.format_failure(verification)); + if !quiet { + writer.raw_stderr(&formatter.format_failure(verification)); } if !verification.deleted_changesets().is_empty() { Err(CliError::ChangesetDeleted { @@ -85,3 +93,160 @@ pub(crate) fn run(args: VerifyArgs, start_path: &Path) -> Result<()> { } } } + +#[cfg(test)] +mod tests { + use std::collections::HashSet; + use std::path::PathBuf; + + use changeset_core::PackageInfo; + use changeset_operations::operations::VerifyOutcome; + use changeset_operations::verification::VerificationResult; + use semver::Version; + + use super::print_outcome; + use crate::error::CliError; + use crate::output::BufferCliWriter; + + fn empty_verification() -> VerificationResult { + VerificationResult::new(vec![], HashSet::new(), vec![], vec![]) + } + + fn package_info(name: &str) -> PackageInfo { + PackageInfo::new( + name.to_string(), + Version::new(1, 0, 0), + PathBuf::from(format!("crates/{name}")), + ) + } + + #[test] + fn no_changes_prints_message() { + let writer = BufferCliWriter::new(); + + let result = print_outcome(&VerifyOutcome::NoChanges, false, &writer); + + assert!(result.is_ok()); + let text = writer.stdout_text(); + assert!(text.contains("No files changed")); + } + + #[test] + fn no_changes_quiet_suppresses_output() { + let writer = BufferCliWriter::new(); + + let result = print_outcome(&VerifyOutcome::NoChanges, true, &writer); + + assert!(result.is_ok()); + assert!(writer.stdout_entries().is_empty()); + } + + #[test] + fn no_packages_affected_shows_counts() { + let writer = BufferCliWriter::new(); + let outcome = VerifyOutcome::NoPackagesAffected { + project_file_count: 3, + ignored_file_count: 2, + }; + + let result = print_outcome(&outcome, false, &writer); + + assert!(result.is_ok()); + let text = writer.stdout_text(); + assert!(text.contains("No packages affected by changes")); + assert!(text.contains("3 project-level file(s) changed")); + assert!(text.contains("2 file(s) ignored by patterns")); + } + + #[test] + fn no_packages_affected_hides_zero_counts() { + let writer = BufferCliWriter::new(); + let outcome = VerifyOutcome::NoPackagesAffected { + project_file_count: 0, + ignored_file_count: 0, + }; + + let result = print_outcome(&outcome, false, &writer); + + assert!(result.is_ok()); + let text = writer.stdout_text(); + assert!(text.contains("No packages affected by changes")); + assert!(!text.contains("project-level")); + assert!(!text.contains("ignored")); + } + + #[test] + fn success_renders_via_formatter() { + let writer = BufferCliWriter::new(); + let verification = empty_verification(); + + let result = print_outcome(&VerifyOutcome::Success(verification), false, &writer); + + assert!(result.is_ok()); + let text = writer.stdout_text(); + assert!(text.contains("All changed packages have changeset coverage")); + } + + #[test] + fn success_quiet_suppresses_output() { + let writer = BufferCliWriter::new(); + let verification = empty_verification(); + + let result = print_outcome(&VerifyOutcome::Success(verification), true, &writer); + + assert!(result.is_ok()); + assert!(writer.stdout_entries().is_empty()); + } + + #[test] + fn failed_with_uncovered_returns_verification_error() { + let writer = BufferCliWriter::new(); + let mut verification = VerificationResult::new( + vec![package_info("my-crate")], + HashSet::new(), + vec![], + vec![], + ); + verification.set_uncovered_packages(vec![package_info("my-crate")]); + + let result = print_outcome(&VerifyOutcome::Failed(verification), false, &writer); + + assert!(result.is_err()); + match result.unwrap_err() { + CliError::VerificationFailed { uncovered_count } => { + assert_eq!(uncovered_count, 1); + } + other => panic!("expected VerificationFailed, got {other:?}"), + } + } + + #[test] + fn failed_with_deleted_changesets_returns_deleted_error() { + let writer = BufferCliWriter::new(); + let mut verification = empty_verification(); + verification.set_deleted_changesets(vec![PathBuf::from(".changeset/old.md")]); + + let result = print_outcome(&VerifyOutcome::Failed(verification), false, &writer); + + assert!(result.is_err()); + match result.unwrap_err() { + CliError::ChangesetDeleted { paths } => { + assert_eq!(paths.len(), 1); + } + other => panic!("expected ChangesetDeleted, got {other:?}"), + } + } + + #[test] + fn failed_quiet_suppresses_output_but_still_errors() { + let writer = BufferCliWriter::new(); + let mut verification = empty_verification(); + verification.set_uncovered_packages(vec![package_info("my-crate")]); + + let result = print_outcome(&VerifyOutcome::Failed(verification), true, &writer); + + assert!(result.is_err()); + assert!(writer.stdout_entries().is_empty()); + assert!(writer.stderr_entries().is_empty()); + } +} diff --git a/crates/cargo-changeset/src/output/mod.rs b/crates/cargo-changeset/src/output/mod.rs index 5ee5367..b2d9dd6 100644 --- a/crates/cargo-changeset/src/output/mod.rs +++ b/crates/cargo-changeset/src/output/mod.rs @@ -2,7 +2,12 @@ pub(crate) mod error_format; mod formatter; mod plain; mod status; +pub(crate) mod writer; pub(crate) use formatter::OutputFormatter; pub(crate) use plain::PlainTextFormatter; pub(crate) use status::{PlainTextStatusFormatter, StatusFormatter}; +pub(crate) use writer::{CliWriter, MessageLevel, StdoutCliWriter}; + +#[cfg(test)] +pub(crate) use writer::test_support::{BufferCliWriter, OutputEntry}; diff --git a/crates/cargo-changeset/src/output/writer.rs b/crates/cargo-changeset/src/output/writer.rs new file mode 100644 index 0000000..4b62cd3 --- /dev/null +++ b/crates/cargo-changeset/src/output/writer.rs @@ -0,0 +1,335 @@ +pub(crate) trait CliWriter { + fn blank(&self); + fn message(&self, level: MessageLevel, text: &str); + fn heading(&self, text: &str); + fn line(&self, text: &str); + fn indented(&self, text: &str); + fn detail(&self, key: &str, value: &str); + fn list_item(&self, text: &str); + fn warn_stderr(&self, text: &str); + fn raw(&self, text: &str); + fn raw_stderr(&self, text: &str); +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum MessageLevel { + Info, + Success, + Hint, +} + +pub(crate) struct StdoutCliWriter; + +impl CliWriter for StdoutCliWriter { + fn blank(&self) { + println!(); + } + + fn message(&self, _level: MessageLevel, text: &str) { + println!("{text}"); + } + + fn heading(&self, text: &str) { + println!("{text}"); + } + + fn line(&self, text: &str) { + println!("{text}"); + } + + fn indented(&self, text: &str) { + println!(" {text}"); + } + + fn detail(&self, key: &str, value: &str) { + println!(" {key} = {value}"); + } + + fn list_item(&self, text: &str) { + println!(" - {text}"); + } + + fn warn_stderr(&self, text: &str) { + eprintln!("{text}"); + } + + fn raw(&self, text: &str) { + print!("{text}"); + } + + fn raw_stderr(&self, text: &str) { + eprint!("{text}"); + } +} + +#[cfg(test)] +pub(crate) mod test_support { + use std::cell::RefCell; + + use super::{CliWriter, MessageLevel}; + + #[derive(Debug, Clone, PartialEq, Eq)] + pub(crate) enum OutputEntry { + Blank, + Message { level: MessageLevel, text: String }, + Heading(String), + Line(String), + Indented(String), + Detail { key: String, value: String }, + ListItem(String), + Raw(String), + } + + pub(crate) struct BufferCliWriter { + stdout: RefCell>, + stderr: RefCell>, + } + + impl BufferCliWriter { + pub(crate) fn new() -> Self { + Self { + stdout: RefCell::new(Vec::new()), + stderr: RefCell::new(Vec::new()), + } + } + + pub(crate) fn stdout_entries(&self) -> Vec { + self.stdout.borrow().clone() + } + + pub(crate) fn stderr_entries(&self) -> Vec { + self.stderr.borrow().clone() + } + + pub(crate) fn stdout_text(&self) -> String { + let entries = self.stdout.borrow(); + let mut output = String::new(); + for entry in entries.iter() { + match entry { + OutputEntry::Blank => output.push('\n'), + OutputEntry::Message { text, .. } => { + output.push_str(text); + output.push('\n'); + } + OutputEntry::Heading(text) | OutputEntry::Line(text) => { + output.push_str(text); + output.push('\n'); + } + OutputEntry::Indented(text) => { + output.push_str(" "); + output.push_str(text); + output.push('\n'); + } + OutputEntry::Detail { key, value } => { + output.push_str(" "); + output.push_str(key); + output.push_str(" = "); + output.push_str(value); + output.push('\n'); + } + OutputEntry::ListItem(text) => { + output.push_str(" - "); + output.push_str(text); + output.push('\n'); + } + OutputEntry::Raw(text) => { + output.push_str(text); + } + } + } + output + } + } + + impl CliWriter for BufferCliWriter { + fn blank(&self) { + self.stdout.borrow_mut().push(OutputEntry::Blank); + } + + fn message(&self, level: MessageLevel, text: &str) { + self.stdout.borrow_mut().push(OutputEntry::Message { + level, + text: text.to_string(), + }); + } + + fn heading(&self, text: &str) { + self.stdout + .borrow_mut() + .push(OutputEntry::Heading(text.to_string())); + } + + fn line(&self, text: &str) { + self.stdout + .borrow_mut() + .push(OutputEntry::Line(text.to_string())); + } + + fn indented(&self, text: &str) { + self.stdout + .borrow_mut() + .push(OutputEntry::Indented(text.to_string())); + } + + fn detail(&self, key: &str, value: &str) { + self.stdout.borrow_mut().push(OutputEntry::Detail { + key: key.to_string(), + value: value.to_string(), + }); + } + + fn list_item(&self, text: &str) { + self.stdout + .borrow_mut() + .push(OutputEntry::ListItem(text.to_string())); + } + + fn warn_stderr(&self, text: &str) { + self.stderr.borrow_mut().push(text.to_string()); + } + + fn raw(&self, text: &str) { + self.stdout + .borrow_mut() + .push(OutputEntry::Raw(text.to_string())); + } + + fn raw_stderr(&self, text: &str) { + self.stderr.borrow_mut().push(text.to_string()); + } + } +} + +#[cfg(test)] +mod tests { + use super::test_support::{BufferCliWriter, OutputEntry}; + use super::{CliWriter, MessageLevel}; + + #[test] + fn blank_records_blank_entry() { + let writer = BufferCliWriter::new(); + writer.blank(); + + assert_eq!(writer.stdout_entries(), vec![OutputEntry::Blank]); + } + + #[test] + fn message_records_level_and_text() { + let writer = BufferCliWriter::new(); + writer.message(MessageLevel::Success, "done"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Message { + level: MessageLevel::Success, + text: "done".to_string() + }] + ); + } + + #[test] + fn heading_records_text() { + let writer = BufferCliWriter::new(); + writer.heading("Title"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Heading("Title".to_string())] + ); + } + + #[test] + fn line_records_text() { + let writer = BufferCliWriter::new(); + writer.line("hello"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Line("hello".to_string())] + ); + } + + #[test] + fn indented_records_text() { + let writer = BufferCliWriter::new(); + writer.indented("item"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Indented("item".to_string())] + ); + } + + #[test] + fn detail_records_key_value() { + let writer = BufferCliWriter::new(); + writer.detail("commit", "true"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Detail { + key: "commit".to_string(), + value: "true".to_string() + }] + ); + } + + #[test] + fn list_item_records_text() { + let writer = BufferCliWriter::new(); + writer.list_item("first"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::ListItem("first".to_string())] + ); + } + + #[test] + fn warn_stderr_records_to_stderr_buffer() { + let writer = BufferCliWriter::new(); + writer.warn_stderr("warning!"); + + assert!(writer.stdout_entries().is_empty()); + assert_eq!(writer.stderr_entries(), vec!["warning!".to_string()]); + } + + #[test] + fn raw_stderr_records_to_stderr_buffer() { + let writer = BufferCliWriter::new(); + writer.raw_stderr("partial error"); + + assert!(writer.stdout_entries().is_empty()); + assert_eq!(writer.stderr_entries(), vec!["partial error".to_string()]); + } + + #[test] + fn raw_records_text_without_newline() { + let writer = BufferCliWriter::new(); + writer.raw("partial"); + + assert_eq!( + writer.stdout_entries(), + vec![OutputEntry::Raw("partial".to_string())] + ); + } + + #[test] + fn stdout_text_renders_all_entry_types() { + let writer = BufferCliWriter::new(); + writer.heading("Header"); + writer.blank(); + writer.line("a line"); + writer.indented("indented"); + writer.detail("key", "val"); + writer.list_item("item"); + writer.message(MessageLevel::Info, "info msg"); + writer.raw("raw"); + + let text = writer.stdout_text(); + assert_eq!( + text, + "Header\n\na line\n indented\n key = val\n - item\ninfo msg\nraw" + ); + } +} diff --git a/crates/changeset-operations/src/operations/release/types.rs b/crates/changeset-operations/src/operations/release/types.rs index 1cae573..e400322 100644 --- a/crates/changeset-operations/src/operations/release/types.rs +++ b/crates/changeset-operations/src/operations/release/types.rs @@ -174,12 +174,8 @@ pub struct ChangelogUpdate { } impl ChangelogUpdate { - pub(crate) fn new( - path: PathBuf, - package: Option, - version: Version, - created: bool, - ) -> Self { + #[must_use] + pub fn new(path: PathBuf, package: Option, version: Version, created: bool) -> Self { Self { path, package, @@ -198,7 +194,8 @@ pub struct CommitResult { } impl CommitResult { - pub(crate) fn new(sha: String, message: String) -> Self { + #[must_use] + pub fn new(sha: String, message: String) -> Self { Self { sha, message } } } @@ -212,7 +209,8 @@ pub struct TagResult { } impl TagResult { - pub(crate) fn new(name: String, target_sha: String) -> Self { + #[must_use] + pub fn new(name: String, target_sha: String) -> Self { Self { name, target_sha } } } @@ -228,7 +226,8 @@ pub struct GitOperationResult { } impl GitOperationResult { - pub(crate) fn new( + #[must_use] + pub fn new( commit: Option, tags_created: Vec, changesets_deleted: Vec, @@ -257,7 +256,7 @@ pub struct ReleaseOutput { } impl ReleaseOutput { - pub(crate) fn new( + pub fn new( planned_releases: Vec, unchanged_packages: Vec, changesets_consumed: Vec, diff --git a/crates/changeset-operations/src/verification/result.rs b/crates/changeset-operations/src/verification/result.rs index b1ad53b..a1eeda6 100644 --- a/crates/changeset-operations/src/verification/result.rs +++ b/crates/changeset-operations/src/verification/result.rs @@ -25,7 +25,8 @@ pub struct VerificationResult { } impl VerificationResult { - pub(crate) fn new( + #[must_use] + pub fn new( affected_packages: Vec, transitive_dependents: HashSet, project_files: Vec, @@ -43,19 +44,19 @@ impl VerificationResult { } } - pub(crate) fn insert_covered_package(&mut self, name: String) { + pub fn insert_covered_package(&mut self, name: String) { self.covered_packages.insert(name); } - pub(crate) fn set_uncovered_packages(&mut self, uncovered: Vec) { + pub fn set_uncovered_packages(&mut self, uncovered: Vec) { self.uncovered_packages = uncovered; } - pub(crate) fn set_deleted_changesets(&mut self, deleted: Vec) { + pub fn set_deleted_changesets(&mut self, deleted: Vec) { self.deleted_changesets = deleted; } - pub(crate) fn set_none_bump_violations(&mut self, violations: Vec) { + pub fn set_none_bump_violations(&mut self, violations: Vec) { self.none_bump_violations = violations; } From 8bee0272b924fa982ea20cabe579bc507e08b040 Mon Sep 17 00:00:00 2001 From: Lukas Wagner Date: Sat, 2 May 2026 22:13:21 +0200 Subject: [PATCH 2/2] add changesets --- .changeset/changesets/daringly-dependable-sheathbill.md | 4 ++++ .changeset/changesets/reflexively-peerless-guineapig.md | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 .changeset/changesets/daringly-dependable-sheathbill.md create mode 100644 .changeset/changesets/reflexively-peerless-guineapig.md diff --git a/.changeset/changesets/daringly-dependable-sheathbill.md b/.changeset/changesets/daringly-dependable-sheathbill.md new file mode 100644 index 0000000..7dff23d --- /dev/null +++ b/.changeset/changesets/daringly-dependable-sheathbill.md @@ -0,0 +1,4 @@ +--- +cargo-changeset: none +--- +Introduce `CliWriter` trait abstraction for all CLI output, enabling unit-testable output rendering diff --git a/.changeset/changesets/reflexively-peerless-guineapig.md b/.changeset/changesets/reflexively-peerless-guineapig.md new file mode 100644 index 0000000..12eec57 --- /dev/null +++ b/.changeset/changesets/reflexively-peerless-guineapig.md @@ -0,0 +1,4 @@ +--- +changeset-operations: minor +--- +Expose public constructors for result types (`VerificationResult`, `ChangelogUpdate`, `CommitResult`, `TagResult`, `GitOperationResult`, `ReleaseOutput`)