diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 7439026..7489785 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -18,6 +18,10 @@ pub struct Cli { /// The revision(s) whose description to edit (default: @) #[arg(value_name = "REVSETS")] revsets: Vec, + + /// Create a new child revision after editing the description + #[arg(short, long)] + new: bool, } impl Cli { @@ -29,4 +33,95 @@ impl Cli { self.revsets.iter().map(|s| s.as_str()).collect() } } + + pub fn create_new(&self) -> bool { + self.new + } + + pub fn validate(&self) -> Result<(), jj_cz::Error> { + if self.new && self.revsets().len() > 1 { + Err(jj_cz::Error::NewFlagWithMultipleRevisions) + } else { + Ok(()) + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use clap::Parser; + + #[test] + fn revsets_defaults_to_at() { + let cli = Cli::parse_from(["jj-cz"]); + assert_eq!(cli.revsets(), vec!["@"]); + } + + #[test] + fn revsets_returns_provided_values() { + let cli = Cli::parse_from(["jj-cz", "abc", "def"]); + let revsets = cli.revsets(); + assert_eq!(revsets, vec!["abc", "def"]); + } + + #[test] + fn revsets_single_revset() { + let cli = Cli::parse_from(["jj-cz", "xyz"]); + assert_eq!(cli.revsets(), vec!["xyz"]); + } + + #[test] + fn create_new_returns_false_by_default() { + let cli = Cli::parse_from(["jj-cz"]); + assert!(!cli.create_new()); + } + + #[test] + fn create_new_returns_true_with_flag() { + let cli = Cli::parse_from(["jj-cz", "--new"]); + assert!(cli.create_new()); + } + + #[test] + fn create_new_returns_true_with_short_flag() { + let cli = Cli::parse_from(["jj-cz", "-n"]); + assert!(cli.create_new()); + } + + #[test] + fn validate_ok_with_no_args() { + let cli = Cli::parse_from(["jj-cz"]); + assert!(cli.validate().is_ok()); + } + + #[test] + fn validate_ok_with_new_and_single_revset() { + let cli = Cli::parse_from(["jj-cz", "--new", "@"]); + assert!(cli.validate().is_ok()); + } + + #[test] + fn validate_ok_with_multiple_revsets_no_new() { + let cli = Cli::parse_from(["jj-cz", "abc", "def"]); + assert!(cli.validate().is_ok()); + } + + #[test] + fn validate_err_with_new_and_multiple_revsets() { + let cli = Cli::parse_from(["jj-cz", "--new", "abc", "def"]); + let result = cli.validate(); + assert!(result.is_err()); + assert!(matches!( + result.unwrap_err(), + jj_cz::Error::NewFlagWithMultipleRevisions + )); + } + + #[test] + fn cli_derives_debug() { + let cli = Cli::parse_from(["jj-cz", "--new", "@"]); + let debug = format!("{:?}", cli); + assert!(debug.contains("Cli")); + } } diff --git a/src/error.rs b/src/error.rs index a287312..03ee320 100644 --- a/src/error.rs +++ b/src/error.rs @@ -31,6 +31,8 @@ pub enum Error { RevsetResolutionError { revset: String, context: String }, #[error("Revision set '{revset}' resolves to multiple commits; specify a single revision")] MultipleRevisions { revset: String }, + #[error("--new cannot be used with multiple revisions")] + NewFlagWithMultipleRevisions, } impl From for Error { diff --git a/src/jj/lib_executor.rs b/src/jj/lib_executor.rs index 6025688..feb46a7 100644 --- a/src/jj/lib_executor.rs +++ b/src/jj/lib_executor.rs @@ -263,6 +263,33 @@ impl JjExecutor for JjLib { })?; Ok(commit.description().trim_end().to_string()) } + + async fn new_revision(&self, revset: &str) -> Result<(), Error> { + let commit_id = self.get_commit_id(revset).await?; + let repo = self.repo.lock()?.clone(); + let mut tx = repo.start_transaction(); + let parent_commit = + tx.repo() + .store() + .get_commit(&commit_id) + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; + tx.repo_mut() + .check_out(self.workspace_name.clone(), &parent_commit) + .await + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; + let new_repo = + tx.commit("jj-cz: create new revision") + .await + .map_err(|e| Error::JjOperation { + context: e.to_string(), + })?; + *self.repo.lock()? = new_repo; + Ok(()) + } } #[cfg(test)] diff --git a/src/jj/mock.rs b/src/jj/mock.rs index a293995..6fd9c60 100644 --- a/src/jj/mock.rs +++ b/src/jj/mock.rs @@ -11,18 +11,22 @@ use std::sync::{Mutex, atomic::AtomicBool}; /// Mock implementation of JjExecutor for testing #[derive(Debug)] pub struct MockJjExecutor { - /// Response to return from is_repository() + /// Response to return from `is_repository()` is_repo_response: Result, - /// Response to return from describe() + /// Response to return from `describe()` describe_response: Result<(), Error>, /// Track described revsets described_revsets: Mutex>, - /// Track response to return from get_description() + /// Track response to return from `get_description()` get_description_response: Result, - /// Track calls to is_repository() + /// Track calls to `is_repository()` is_repo_called: AtomicBool, - /// Track calls to describe() with the message passed + /// Track calls to `describe()` with the message passed describe_calls: Mutex>, + /// Track response to return from `new_revision()` + new_revision_response: Result<(), Error>, + /// Track calls to `new_revision()` + new_revision_calls: Mutex>, } impl Default for MockJjExecutor { @@ -34,6 +38,8 @@ impl Default for MockJjExecutor { get_description_response: Ok(String::new()), is_repo_called: AtomicBool::new(false), describe_calls: Mutex::new(Vec::new()), + new_revision_response: Ok(()), + new_revision_calls: Mutex::new(Vec::new()), } } } @@ -66,6 +72,15 @@ impl MockJjExecutor { pub fn describe_messages(&self) -> Vec { self.describe_calls.lock().unwrap().clone() } + + pub fn with_new_revision_response(mut self, response: Result<(), Error>) -> Self { + self.new_revision_response = response; + self + } + + pub fn new_revision_calls(&self) -> Vec { + self.new_revision_calls.lock().unwrap().clone() + } } #[async_trait(?Send)] @@ -97,6 +112,17 @@ impl JjExecutor for MockJjExecutor { async fn get_description(&self, _revset: &str) -> Result { self.get_description_response.clone() } + + async fn new_revision(&self, revset: &str) -> Result<(), Error> { + self.new_revision_calls + .lock() + .unwrap() + .push(revset.to_string()); + match &self.new_revision_response { + Ok(()) => Ok(()), + Err(e) => Err(e.clone()), + } + } } #[cfg(test)] @@ -245,4 +271,63 @@ mod tests { mock.is_repository().await.unwrap(); assert!(mock.was_is_repo_called()); } + + /// Test mock new_revision() records the revset + #[tokio::test] + async fn mock_new_revision_records_revset() { + let mock = MockJjExecutor::new(); + let result = mock.new_revision("@").await; + assert!(result.is_ok()); + let calls = mock.new_revision_calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0], "@"); + } + + /// Test mock new_revision() records multiple calls + #[tokio::test] + async fn mock_new_revision_records_multiple_calls() { + let mock = MockJjExecutor::new(); + mock.new_revision("@").await.unwrap(); + mock.new_revision("abc").await.unwrap(); + mock.new_revision("xyz").await.unwrap(); + let calls = mock.new_revision_calls(); + assert_eq!(calls.len(), 3); + assert_eq!(calls[0], "@"); + assert_eq!(calls[1], "abc"); + assert_eq!(calls[2], "xyz"); + } + + /// Test mock new_revision() returns configured error + #[tokio::test] + async fn mock_new_revision_returns_error() { + let mock = + MockJjExecutor::new().with_new_revision_response(Err(Error::RepositoryLocked)); + let result = mock.new_revision("@").await; + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::RepositoryLocked)); + } + + /// Test mock new_revision() records revset even on error + #[tokio::test] + async fn mock_new_revision_records_revset_on_error() { + let mock = + MockJjExecutor::new().with_new_revision_response(Err(Error::JjOperation { + context: "failed".to_string(), + })); + let result = mock.new_revision("abc").await; + assert!(result.is_err()); + let calls = mock.new_revision_calls(); + assert_eq!(calls.len(), 1); + assert_eq!(calls[0], "abc"); + } + + /// Test mock new_revision() can be inspected after success + #[tokio::test] + async fn mock_new_revision_returns_ok_and_tracks_revset() { + let mock = MockJjExecutor::new(); + let result = mock.new_revision("my-feature").await; + assert!(result.is_ok()); + let calls = mock.new_revision_calls(); + assert_eq!(calls, vec!["my-feature"]); + } } diff --git a/src/jj/mod.rs b/src/jj/mod.rs index 636fe7f..0003b89 100644 --- a/src/jj/mod.rs +++ b/src/jj/mod.rs @@ -21,6 +21,11 @@ pub trait JjExecutor: Send + Sync { /// Get the current description of a specific revision async fn get_description(&self, revset: &str) -> Result; + + /// Create a new empty child revision parented on `revset`. + /// + /// Equivalent to `jj new ` + async fn new_revision(&self, revset: &str) -> Result<(), Error>; } #[cfg(test)] diff --git a/src/lib.rs b/src/lib.rs index 09835e6..18d12cb 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,5 +1,5 @@ mod commit; -mod error; +pub mod error; mod jj; mod prompts; diff --git a/src/main.rs b/src/main.rs index 0e19867..2bdad5e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,7 +10,7 @@ const EXIT_CANCELLED: i32 = 130; // Same as SIGINT (Ctrl+C) const EXIT_ERROR: i32 = 1; /// Map application errors to appropriate exit codes -fn error_to_exit_code(error: &Error) -> i32 { +fn error_to_exit_code(error: Error) -> i32 { match error { Error::Cancelled => EXIT_CANCELLED, Error::NotARepository => EXIT_ERROR, @@ -24,6 +24,7 @@ fn error_to_exit_code(error: &Error) -> i32 { Error::FailedReadingConfig { .. } => EXIT_ERROR, Error::RevsetResolutionError { .. } => EXIT_ERROR, Error::MultipleRevisions { .. } => EXIT_ERROR, + Error::NewFlagWithMultipleRevisions => EXIT_ERROR, } } @@ -34,25 +35,26 @@ fn is_interactive_terminal() -> bool { } #[tokio::main] -async fn main() { +async fn main() -> Result<(), ()> { let cli = cli::Cli::parse(); + + cli.validate().map_err(exit_on_error)?; + if !is_interactive_terminal() { eprintln!("❌ Error: jj-cz requires an interactive terminal (TTY)"); eprintln!(" This tool cannot be used in non-interactive mode or when piping input."); eprintln!(" Use --help for usage information."); process::exit(EXIT_ERROR); } - let executor = match JjLib::new().await { - Ok(e) => e, - Err(e) => { - eprintln!("❌ Error: {}", e); - process::exit(EXIT_ERROR); - } - }; + let executor = JjLib::new().await.map_err(exit_on_error)?; let workflow = CommitWorkflow::new(executor); for revset in cli.revsets() { let result = workflow.run_for_revset(revset).await; handle_result(result); + if cli.create_new() { + println!("Creating a new revision after {revset}"); + workflow.new_revision(revset).await.map_err(exit_on_error)?; + } } fn handle_result(result: Result<(), Error>) { @@ -62,11 +64,13 @@ async fn main() { println!("🟡 Operation cancelled by user."); process::exit(EXIT_CANCELLED); } - Err(e) => { - eprintln!("❌ Error: {}", e); - process::exit(error_to_exit_code(&e)); - } + Err(e) => exit_on_error(e), } } process::exit(EXIT_SUCCESS); } + +fn exit_on_error(e: Error) { + eprintln!("❌ Error: {}", e); + process::exit(error_to_exit_code(e)); +} diff --git a/src/prompts/workflow.rs b/src/prompts/workflow.rs index 787f9c1..63d97ab 100644 --- a/src/prompts/workflow.rs +++ b/src/prompts/workflow.rs @@ -184,6 +184,10 @@ impl CommitWorkflow { Err(Error::Cancelled) } } + + pub async fn new_revision(&self, revset: &str) -> Result<(), Error> { + self.executor.new_revision(revset).await + } } #[cfg(test)] @@ -762,4 +766,56 @@ mod tests { assert_eq!(messages.len(), 1); assert_eq!(messages[0], "fix: fix crash"); } + + /// Test workflow new_revision() records the revset + #[tokio::test] + async fn workflow_new_revision_records_revset() { + let mock_executor = MockJjExecutor::new(); + let workflow = CommitWorkflow::new(mock_executor); + + let result = workflow.new_revision("@").await; + assert!(result.is_ok()); + + let calls = workflow.executor.new_revision_calls(); + assert_eq!(calls, vec!["@"]); + } + + /// Test workflow new_revision() propagates executor errors + #[tokio::test] + async fn workflow_new_revision_propagates_error() { + let mock_executor = MockJjExecutor::new() + .with_new_revision_response(Err(Error::RepositoryLocked)); + let workflow = CommitWorkflow::new(mock_executor); + + let result = workflow.new_revision("@").await; + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::RepositoryLocked)); + } + + /// Test workflow run_for_revset() followed by new_revision() records both + /// + /// This mirrors the actual usage pattern in main.rs. + #[tokio::test] + async fn workflow_describe_then_new_revision() { + let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true)); + let mock_prompts = MockPrompts::new() + .with_commit_type(CommitType::Feat) + .with_scope(Scope::empty()) + .with_description(Description::parse("add feature").unwrap()) + .with_breaking_change(BreakingChange::No) + .with_body(Body::default()) + .with_confirm(true); + + let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); + + workflow.run_for_revset("@").await.expect("describe failed"); + workflow.new_revision("@").await.expect("new_revision failed"); + + let messages = workflow.executor.describe_messages(); + assert_eq!(messages.len(), 1); + assert!(messages[0].contains("feat:")); + + let calls = workflow.executor.new_revision_calls(); + assert_eq!(calls, vec!["@"]); + } } diff --git a/tests/error_tests.rs b/tests/error_tests.rs index 773fbc4..0e9dc3a 100644 --- a/tests/error_tests.rs +++ b/tests/error_tests.rs @@ -135,3 +135,155 @@ fn test_jj_operation_context() { panic!("Expected JjOperation variant"); } } + +/// Test conversion from std::io::Error +#[test] +fn test_from_io_error() { + let io_err = std::io::Error::new(std::io::ErrorKind::NotFound, "file not found"); + let error: Error = io_err.into(); + assert!(matches!(error, Error::FailedGettingCurrentDir)); +} + +/// Test conversion from std::sync::PoisonError +#[test] +fn test_from_poison_error() { + let mutex = std::sync::Mutex::new(()); + // Poison the mutex by panicking while holding the lock + let poison_err = std::panic::catch_unwind(|| { + let _guard = mutex.lock().unwrap(); + panic!("deliberate panic"); + }); + assert!(poison_err.is_err()); + + // Now lock should fail with PoisonError + let result = mutex.lock(); + assert!(result.is_err()); + + let error: Error = result.unwrap_err().into(); + assert!(matches!(error, Error::JjOperation { .. })); + assert_eq!( + format!("{}", error), + "Repository operation failed: internal lock poisoned" + ); +} + +/// Test from_revset_evaluation_error constructs RevsetResolutionError +#[test] +fn test_from_revset_evaluation_error() { + let underlying = std::io::Error::new(std::io::ErrorKind::Other, "store failure"); + let eval_err = jj_lib::revset::RevsetEvaluationError::Other(Box::new(underlying)); + let error = Error::from_revset_evaluation_error("@", eval_err); + assert!(matches!(error, Error::RevsetResolutionError { .. })); + let description = format!("{}", error); + assert!(description.contains("@")); + assert!(description.contains("store failure")); +} + +/// Test from_revset_resolution_error constructs RevsetResolutionError +#[test] +fn test_from_revset_resolution_error() { + let resolution_err = jj_lib::revset::RevsetResolutionError::NoSuchRevision { + name: "nonexistent".to_string(), + candidates: Vec::new(), + }; + let error = Error::from_revset_resolution_error("@", resolution_err); + assert!(matches!(error, Error::RevsetResolutionError { .. })); + let description = format!("{}", error); + assert!(description.contains("@")); + assert!(description.contains("nonexistent")); +} + +/// Test NewFlagWithMultipleRevisions error display +#[test] +fn test_new_flag_with_multiple_revisions() { + let error = Error::NewFlagWithMultipleRevisions; + assert_eq!( + format!("{}", error), + "--new cannot be used with multiple revisions" + ); +} + +/// Test NonInteractive error display +#[test] +fn test_non_interactive() { + let error = Error::NonInteractive; + assert_eq!(format!("{}", error), "Non-interactive terminal detected"); +} + +/// Test FailedReadingConfig error display +#[test] +fn test_failed_reading_config() { + let error = Error::FailedReadingConfig { + context: "config parse error".to_string(), + }; + let description = format!("{}", error); + assert!(description.contains("config parse error")); +} + +/// Test MultipleRevisions error display +#[test] +fn test_multiple_revisions() { + let error = Error::MultipleRevisions { + revset: "abc | def".to_string(), + }; + let description = format!("{}", error); + assert!(description.contains("abc | def")); + assert!(description.contains("multiple commits")); +} + +/// Test RepositoryLocked error display +#[test] +fn test_repository_locked() { + let error = Error::RepositoryLocked; + assert_eq!( + format!("{}", error), + "Repository is locked by another process" + ); +} + +/// Test FailedGettingCurrentDir error display +#[test] +fn test_failed_getting_current_dir() { + let error = Error::FailedGettingCurrentDir; + assert_eq!(format!("{}", error), "Could not get current directory"); +} + +/// Test error matching on all variants +#[test] +fn test_error_matching_all_variants() { + let variants: Vec = vec![ + Error::InvalidScope("s".into()), + Error::InvalidDescription("d".into()), + Error::InvalidCommitMessage("m".into()), + Error::NotARepository, + Error::JjOperation { + context: "c".into(), + }, + Error::RepositoryLocked, + Error::FailedGettingCurrentDir, + Error::FailedReadingConfig { + context: "c".into(), + }, + Error::Cancelled, + Error::NonInteractive, + Error::RevsetResolutionError { + revset: "@".into(), + context: "c".into(), + }, + Error::MultipleRevisions { revset: "@".into() }, + Error::NewFlagWithMultipleRevisions, + ]; + + // All variants should be displayable without panicking + for variant in &variants { + let _ = format!("{}", variant); + let _ = format!("{:?}", variant); + } + + // Verify all variants can be cloned + let cloned: Vec = variants.iter().map(|e| e.clone()).collect(); + assert_eq!(variants.len(), cloned.len()); + for (original, clone) in variants.iter().zip(cloned.iter()) { + assert_eq!(original, clone); + } +}