From 02a04d0d37764f0fceb3e610df72ef1ce3eff45c Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Fri, 22 Mar 2024 19:34:28 -0700 Subject: [PATCH] test_conflicts and test_resolve_command: use `indoc!` to indent conflict markers in tests Apart from (IMO) looking nicer, this will also sidestep the potential problem that if the file contains actual jj conflict markers (`>>>>>>>` in the beginning of a line, for example), jj would currently have trouble materializing and subsequently parsing conflicts in the file if it actually became conflicted. I'll demo this bug in either this or a subsequent PR. It's the kind of bug that sounds serious in theory but might never cause a problem in practice. After this PR, only `docs/tutorial.md` has a conflict marker that's not indented. There's only one there, so hopefully it won't be too much of a pain to deal with. I also indented other strings in `test_conflicts.rs`. IMO, this looks nice and more consistent with the `insta::assert_snapshot` output. I didn't spend the time to do the same for `test_resolve_command`. --- Cargo.lock | 2 + Cargo.toml | 1 + cli/Cargo.toml | 1 + cli/tests/test_resolve_command.rs | 40 ++-- lib/Cargo.toml | 1 + lib/tests/test_conflicts.rs | 301 ++++++++++++++++-------------- 6 files changed, 187 insertions(+), 159 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f7204ca9f..494dbbed0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1688,6 +1688,7 @@ dependencies = [ "gix", "hex", "indexmap", + "indoc", "insta", "itertools 0.12.1", "jj-cli", @@ -1739,6 +1740,7 @@ dependencies = [ "glob", "hex", "ignore", + "indoc", "insta", "itertools 0.12.1", "jj-lib-proc-macros", diff --git a/Cargo.toml b/Cargo.toml index 845c28fda..f3cc70b05 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,6 +56,7 @@ glob = "0.3.1" hex = "0.4.3" ignore = "0.4.20" indexmap = "2.2.5" +indoc = "2.0.4" insta = { version = "1.35.1", features = ["filters"] } itertools = "0.12.1" libc = { version = "0.2.153" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 43b4ec237..1444902a5 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -82,6 +82,7 @@ anyhow = { workspace = true } assert_cmd = { workspace = true } assert_matches = { workspace = true } async-trait = { workspace = true } +indoc = { workspace = true } insta = { workspace = true } test-case = { workspace = true } testutils = { workspace = true } diff --git a/cli/tests/test_resolve_command.rs b/cli/tests/test_resolve_command.rs index a1ff2954c..f25d6f14a 100644 --- a/cli/tests/test_resolve_command.rs +++ b/cli/tests/test_resolve_command.rs @@ -14,6 +14,8 @@ use std::path::Path; +use indoc::indoc; + use crate::common::TestEnvironment; fn create_commit( @@ -195,15 +197,16 @@ fn test_resolution() { &editor_script, [ "dump editor2", - "write -<<<<<<< -%%%%%%% --some -+fake -+++++++ -conflict ->>>>>>> -", + indoc! {" + write + <<<<<<< + %%%%%%% + -some + +fake + +++++++ + conflict + >>>>>>> + "}, ] .join("\0"), ) @@ -270,15 +273,16 @@ conflict &editor_script, [ "dump editor3", - "write -<<<<<<< -%%%%%%% --some -+fake -+++++++ -conflict ->>>>>>> -", + indoc! {" + write + <<<<<<< + %%%%%%% + -some + +fake + +++++++ + conflict + >>>>>>> + "}, ] .join("\0"), ) diff --git a/lib/Cargo.toml b/lib/Cargo.toml index 2df34e7a3..760578f8a 100644 --- a/lib/Cargo.toml +++ b/lib/Cargo.toml @@ -72,6 +72,7 @@ winreg = { workspace = true } assert_matches = { workspace = true } criterion = { workspace = true } esl01-renderdag = { workspace = true } +indoc = { workspace = true } insta = { workspace = true } num_cpus = { workspace = true } pretty_assertions = { workspace = true } diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 8dbe47a62..ad8563c3d 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use indoc::indoc; use jj_lib::backend::FileId; use jj_lib::conflicts::{ extract_as_single_hunk, materialize_merge_result, parse_conflict, update_from_content, @@ -32,34 +33,37 @@ fn test_materialize_conflict_basic() { let base_id = testutils::write_file( store, path, - "line 1 -line 2 -line 3 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + line 3 + line 4 + line 5 + "}, ); let left_id = testutils::write_file( store, path, - "line 1 -line 2 -left 3.1 -left 3.2 -left 3.3 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + left 3.1 + left 3.2 + left 3.3 + line 4 + line 5 + "}, ); let right_id = testutils::write_file( store, path, - "line 1 -line 2 -right 3.1 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + right 3.1 + line 4 + line 5 + "}, ); // The left side should come first. The diff should be use the smaller (right) @@ -122,37 +126,41 @@ fn test_materialize_conflict_multi_rebase_conflicts() { let base_id = testutils::write_file( store, path, - "line 1 -line 2 base -line 3 -", + indoc! {" + line 1 + line 2 base + line 3 + "}, ); let a_id = testutils::write_file( store, path, - "line 1 -line 2 a.1 -line 2 a.2 -line 2 a.3 -line 3 -", + indoc! {" + line 1 + line 2 a.1 + line 2 a.2 + line 2 a.3 + line 3 + "}, ); let b_id = testutils::write_file( store, path, - "line 1 -line 2 b.1 -line 2 b.2 -line 3 -", + indoc! {" + line 1 + line 2 b.1 + line 2 b.2 + line 3 + "}, ); let c_id = testutils::write_file( store, path, - "line 1 -line 2 c.1 -line 3 -", + indoc! {" + line 1 + line 2 c.1 + line 3 + "}, ); // The order of (a, b, c) should be preserved. For all cases, the "a" side @@ -240,32 +248,35 @@ fn test_materialize_parse_roundtrip() { let base_id = testutils::write_file( store, path, - "line 1 -line 2 -line 3 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + line 3 + line 4 + line 5 + "}, ); let left_id = testutils::write_file( store, path, - "line 1 left -line 2 left -line 3 -line 4 -line 5 left -", + indoc! {" + line 1 left + line 2 left + line 3 + line 4 + line 5 left + "}, ); let right_id = testutils::write_file( store, path, - "line 1 right -line 2 -line 3 -line 4 right -line 5 right -", + indoc! {" + line 1 right + line 2 + line 3 + line 4 right + line 5 right + "}, ); let conflict = Merge::from_removes_adds( @@ -335,31 +346,34 @@ fn test_materialize_conflict_modify_delete() { let base_id = testutils::write_file( store, path, - "line 1 -line 2 -line 3 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + line 3 + line 4 + line 5 + "}, ); let modified_id = testutils::write_file( store, path, - "line 1 -line 2 -modified -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + modified + line 4 + line 5 + "}, ); let deleted_id = testutils::write_file( store, path, - "line 1 -line 2 -line 4 -line 5 -", + indoc! {" + line 1 + line 2 + line 4 + line 5 + "}, ); // left modifies a line, right deletes the same line. @@ -479,12 +493,13 @@ fn test_materialize_conflict_two_forward_diffs() { fn test_parse_conflict_resolved() { assert_eq!( parse_conflict( - b"line 1 + indoc! {b" + line 1 line 2 line 3 line 4 line 5 -", +"}, 2 ), None @@ -494,19 +509,19 @@ line 5 #[test] fn test_parse_conflict_simple() { insta::assert_debug_snapshot!( - parse_conflict( - b"line 1 -<<<<<<< -%%%%%%% - line 2 --line 3 -+left - line 4 -+++++++ -right ->>>>>>> -line 5 -", + parse_conflict(indoc! {b" + line 1 + <<<<<<< + %%%%%%% + line 2 + -line 3 + +left + line 4 + +++++++ + right + >>>>>>> + line 5 + "}, 2 ), @r###" @@ -535,23 +550,24 @@ line 5 fn test_parse_conflict_multi_way() { insta::assert_debug_snapshot!( parse_conflict( - b"line 1 -<<<<<<< -%%%%%%% - line 2 --line 3 -+left - line 4 -+++++++ -right -%%%%%%% - line 2 -+forward - line 3 - line 4 ->>>>>>> -line 5 -", + indoc! {b" + line 1 + <<<<<<< + %%%%%%% + line 2 + -line 3 + +left + line 4 + +++++++ + right + %%%%%%% + line 2 + +forward + line 3 + line 4 + >>>>>>> + line 5 + "}, 3 ), @r###" @@ -582,18 +598,19 @@ line 5 fn test_parse_conflict_different_wrong_arity() { assert_eq!( parse_conflict( - b"line 1 -<<<<<<< -%%%%%%% - line 2 --line 3 -+left - line 4 -+++++++ -right ->>>>>>> -line 5 -", + indoc! {b" + line 1 + <<<<<<< + %%%%%%% + line 2 + -line 3 + +left + line 4 + +++++++ + right + >>>>>>> + line 5 + "}, 3 ), None @@ -605,17 +622,18 @@ fn test_parse_conflict_malformed_marker() { // The conflict marker is missing `%%%%%%%` assert_eq!( parse_conflict( - b"line 1 -<<<<<<< - line 2 --line 3 -+left - line 4 -+++++++ -right ->>>>>>> -line 5 -", + indoc! {b" + line 1 + <<<<<<< + line 2 + -line 3 + +left + line 4 + +++++++ + right + >>>>>>> + line 5 + "}, 2 ), None @@ -627,18 +645,19 @@ fn test_parse_conflict_malformed_diff() { // The diff part is invalid (missing space before "line 4") assert_eq!( parse_conflict( - b"line 1 -<<<<<<< -%%%%%%% - line 2 --line 3 -+left -line 4 -+++++++ -right ->>>>>>> -line 5 -", + indoc! {b" + line 1 + <<<<<<< + %%%%%%% + line 2 + -line 3 + +left + line 4 + +++++++ + right + >>>>>>> + line 5 + "}, 2 ), None