diff --git a/cli/tests/test_operations.rs b/cli/tests/test_operations.rs index 84a1ea7a2..fb6b64374 100644 --- a/cli/tests/test_operations.rs +++ b/cli/tests/test_operations.rs @@ -68,35 +68,15 @@ fn test_op_log() { @ 230dd059e1b059aefc0da06a2e5a7dbf22362f22 ◉ 0000000000000000000000000000000000000000 "###); - insta::assert_snapshot!(get_log_output(&test_env, &repo_path, "@--"), @r###" - ◉ 0000000000000000000000000000000000000000 - "###); insta::assert_snapshot!( test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "@---"]), @r###" Error: The "@---" expression resolved to no operations "###); - // "ID-" also resolves to the parent. - insta::assert_snapshot!( - get_log_output(&test_env, &repo_path, &format!("{add_workspace_id}-")), @r###" - ◉ 0000000000000000000000000000000000000000 - "###); // We get a reasonable message if an invalid operation ID is specified insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "foo"]), @r###" Error: Operation ID "foo" is not a valid hexadecimal prefix "###); - // Odd length - insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "123456789"]), @r###" - Error: No operation ID matching "123456789" - "###); - // Even length - insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "0123456789"]), @r###" - Error: No operation ID matching "0123456789" - "###); - // Empty ID - insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", ""]), @r###" - Error: Operation ID "" is not a valid hexadecimal prefix - "###); test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "description 1"]); test_env.jj_cmd_ok( @@ -112,10 +92,6 @@ fn test_op_log() { insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "@-"]), @r###" Error: The "@" expression resolved to more than one operation "###); - test_env.jj_cmd_ok(&repo_path, &["st"]); - insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["log", "--at-op", "@-"]), @r###" - Error: The "@-" expression resolved to more than one operation - "###); } #[test] diff --git a/lib/tests/test_operations.rs b/lib/tests/test_operations.rs index 3e41b0a81..b85dcfc6a 100644 --- a/lib/tests/test_operations.rs +++ b/lib/tests/test_operations.rs @@ -14,8 +14,12 @@ use std::path::Path; +use assert_matches::assert_matches; +use itertools::Itertools as _; use jj_lib::backend::{CommitId, ObjectId}; +use jj_lib::op_walk::{self, OpsetEvaluationError, OpsetResolutionError}; use jj_lib::repo::Repo; +use jj_lib::settings::UserSettings; use testutils::{create_random_commit, write_random_commit, TestRepo}; fn list_dir(dir: &Path) -> Vec { @@ -177,3 +181,125 @@ fn test_isolation() { let repo = repo.reload_at_head(&settings).unwrap(); assert_heads(repo.as_ref(), vec![rewrite1.id(), rewrite2.id()]); } + +fn stable_op_id_settings() -> UserSettings { + UserSettings::from_config( + testutils::base_config() + .add_source(config::File::from_str( + "debug.operation-timestamp = '2001-02-03T04:05:06+07:00'", + config::FileFormat::Toml, + )) + .build() + .unwrap(), + ) +} + +#[test] +fn test_resolve_op_id() { + let settings = stable_op_id_settings(); + let test_repo = TestRepo::init_with_settings(&settings); + let mut repo = test_repo.repo; + + let mut operations = Vec::new(); + for i in 0..6 { + let tx = repo.start_transaction(&settings); + repo = tx.commit(format!("transaction {i}")); + operations.push(repo.operation().clone()); + } + // "2" is ambiguous + insta::assert_debug_snapshot!(operations.iter().map(|op| op.id().hex()).collect_vec(), @r###" + [ + "27f8c802c8378c5c85825365e83928936ae84d7ae3b5bd26d1cd046aa9a2f791dd7b272338d0d2da8a4359523f25daf217f99128a155ba4bc728d279fc3d8f7f", + "8a6b19ed474dfad1efa49d64d265f80f74c1d10bf77439900d92d8e6a29fdb64ad1137a92928bfd409096bf84b6fbfb50ebdcc6a28323f9f8e5893548f21b7fb", + "65198b538e0f6558d875c49712b0b3570e3a0eb697fd22f5817e39139937b4498e9e9080df1353e116880e36c683f5dddc39d048007ef50da83690a94502bc68", + "59da2544953d8d5851e8f64ed5949c8c26f676b87ab84e9fe153bca76912de3753dee8c9cb641f53f57c51a0e876cd43f08c28ca651ad312e5bc09354e9ec40f", + "f40d12f62b921bdf96c2d191a4d04845fa26043d131ea1e69eb06fa7a4bbfed6668ab48bed7ec728f7e2c9e675d394b382a332c68399d7f4c446450610479ecf", + "2b45a4f90854dd3d4833d998f4fa2e4d4c4eda5212edd3845e8ccb3618d9d538d7a98c173791995898e68d272697ffed1b69838cf839d96cb770856cf499eea8", + ] + "###); + + // Full id + assert_eq!( + op_walk::resolve_op_with_repo(&repo, &operations[0].id().hex()).unwrap(), + operations[0] + ); + // Short id, odd length + assert_eq!( + op_walk::resolve_op_with_repo(&repo, &operations[0].id().hex()[..3]).unwrap(), + operations[0] + ); + // Short id, even length + assert_eq!( + op_walk::resolve_op_with_repo(&repo, &operations[1].id().hex()[..2]).unwrap(), + operations[1] + ); + // Ambiguous id + assert_matches!( + op_walk::resolve_op_with_repo(&repo, "2"), + Err(OpsetEvaluationError::OpsetResolution( + OpsetResolutionError::AmbiguousIdPrefix(_) + )) + ); + // Empty id + assert_matches!( + op_walk::resolve_op_with_repo(&repo, ""), + Err(OpsetEvaluationError::OpsetResolution( + OpsetResolutionError::InvalidIdPrefix(_) + )) + ); + // Unknown id + assert_matches!( + op_walk::resolve_op_with_repo(&repo, "deadbee"), + Err(OpsetEvaluationError::OpsetResolution( + OpsetResolutionError::NoSuchOperation(_) + )) + ); + // Current op + assert_eq!( + op_walk::resolve_op_with_repo(&repo, "@").unwrap(), + *repo.operation() + ); +} + +#[test] +fn test_resolve_op_parents() { + // Use monotonic timestamp to stabilize merge order of transactions + let settings = testutils::user_settings(); + let test_repo = TestRepo::init_with_settings(&settings); + let mut repo = test_repo.repo; + + let mut operations = Vec::new(); + for _ in 0..3 { + let tx = repo.start_transaction(&settings); + repo = tx.commit("test"); + operations.push(repo.operation().clone()); + } + + let op2_id_hex = operations[2].id().hex(); + assert_eq!( + op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}-")).unwrap(), + operations[1] + ); + assert_eq!( + op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}--")).unwrap(), + operations[0] + ); + // "{op2_id_hex}---" is the operation to initialize the repo. + assert_matches!( + op_walk::resolve_op_with_repo(&repo, &format!("{op2_id_hex}----")), + Err(OpsetEvaluationError::OpsetResolution( + OpsetResolutionError::EmptyOperations(_) + )) + ); + + let tx1 = repo.start_transaction(&settings); + let tx2 = repo.start_transaction(&settings); + repo = testutils::commit_transactions(&settings, vec![tx1, tx2]); + let op5_id_hex = repo.operation().id().hex(); + assert_matches!( + op_walk::resolve_op_with_repo(&repo, &format!("{op5_id_hex}-")), + Err(OpsetEvaluationError::OpsetResolution( + OpsetResolutionError::MultipleOperations(_) + )) + ); +} diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 2995ec2aa..7fd1f1fd0 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -131,17 +131,27 @@ impl TestRepo { } pub fn init_with_backend(backend: TestRepoBackend) -> Self { - let settings = user_settings(); + Self::init_with_backend_and_settings(backend, &user_settings()) + } + + pub fn init_with_settings(settings: &UserSettings) -> Self { + Self::init_with_backend_and_settings(TestRepoBackend::Test, settings) + } + + pub fn init_with_backend_and_settings( + backend: TestRepoBackend, + settings: &UserSettings, + ) -> Self { let temp_dir = new_temp_dir(); let repo_dir = temp_dir.path().join("repo"); fs::create_dir(&repo_dir).unwrap(); let repo = ReadonlyRepo::init( - &settings, + settings, &repo_dir, &move |settings, store_path| backend.init_backend(settings, store_path), - Signer::from_settings(&settings).unwrap(), + Signer::from_settings(settings).unwrap(), ReadonlyRepo::default_op_store_initializer(), ReadonlyRepo::default_op_heads_store_initializer(), ReadonlyRepo::default_index_store_initializer(),