Re: [PATCH 1/2] t1015: demonstrate directory/file conflict recovery failures
- From
Elijah Newren <newren@gmail.com>
- Date
- Jul 11, 2018, 18:23 UTC
- Message-ID
- <CABPp-BHHkL0Uns9QoeG8LHJgZmfRJu=qAesdpTWJcNmmqhJF9A@mail.gmail.com>
- In-Reply-To
- <20180711092126.GA17315@flurp.local>
Hi Eric,
On Wed, Jul 11, 2018 at 2:21 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
Show 23 quoted lines
> On Tue, Jul 10, 2018 at 10:18:33PM -0700, Elijah Newren wrote: >> Several "recovery" commands outright fail or do not fully recover >> when directory-file conflicts are present. This includes: >> * git read-tree --reset HEAD >> * git am --skip >> * git am --abort >> * git merge --abort >> * git reset --hard >> >> Add testcases documenting these shortcomings. >> >> Signed-off-by: Elijah Newren <newren@gmail.com> >> --- >> diff --git a/t/t1015-read-index-unmerged.sh b/t/t1015-read-index-unmerged.sh >> @@ -0,0 +1,123 @@ >> +test_expect_success 'setup modify/delete + directory/file conflict' ' >> + test_create_repo df_plus_modify_delete && >> + ( >> + cd df_plus_modify_delete && >> + >> + printf "a\nb\nc\nd\ne\nf\ng\nh\n" >letters && > > test_write_lines a b c d e f g h >letters &&
Copying and modifying an existing testcase, while forgetting to check for anachronisms, strikes again. As always, thanks for reviewing and catching this; I'll fix it up.
Show 32 quoted lines
>> + git add letters &&
>> + git commit -m initial &&
>> +
>> + git checkout -b modify &&
>> + # Throw in letters.txt for sorting order fun
>> + # ("letters.txt" sorts between "letters" and "letters/file")
>> + echo i >>letters &&
>> + echo "version 2" >letters.txt &&
>> + git add letters letters.txt &&
>> + git commit -m modified &&
>> +
>> + git checkout -b delete HEAD^ &&
>> + git rm letters &&
>> + mkdir letters &&
>> + >letters/file &&
>> + echo "version 1" >letters.txt &&
>> + git add letters letters.txt &&
>> + git commit -m deleted
>> + )
>> +'
>> +
>> +test_expect_failure 'read-tree --reset cleans unmerged entries' '
>> + test_when_finished "git -C df_plus_modify_delete clean -f" &&
>> + test_when_finished "git -C df_plus_modify_delete reset --hard" &&
>> + (
>> + cd df_plus_modify_delete &&
>> + ...
>> + )
>> +'
>
> I wonder how much value these distinct repositories add over not using
> them:In my opinion, that'd be much worse. Personally, I think we should move in the opposite direction and try to migrate more of the testsuite elsewhere towards clearly independent tests. A huge pet peeve of mine is that trying to debug a test often requires working through dozens and dozens of unrelated tests and their setup just to understand which part of the repository state is related to the test at hand and which parts can be ignored. It's happened enough times that I just intentionally try to make it clear which tests of mine are independent by making sure they have their own separate repo (and I used to do a git reset --hard && git clean -fdqx && rm -rf .git && git init at the beginning of tests). If folks have a better suggestion for how to ensure test independence than using test_create_repo, I'm all ears, but I'm strongly against just adding more files into the repo than what previous tests did and continuing running from there. I feel that's especially important for future readers when dealing with weird edge and corner cases for merges, but I'd really like to see that clean separation spread throughout the test suite.
Show 103 quoted lines
> --- >8 ---
> #!/bin/sh
>
> test_description='Test various callers of read_index_unmerged'
> . ./test-lib.sh
>
> test_expect_success 'setup modify/delete + directory/file conflict' '
> test_write_lines a b c d e f g h >letters &&
> git add letters &&
> git commit -m initial &&
>
> git checkout -b modify &&
> # Throw in letters.txt for sorting order fun
> # ("letters.txt" sorts between "letters" and "letters/file")
> echo i >>letters &&
> echo "version 2" >letters.txt &&
> git add letters letters.txt &&
> git commit -m modified &&
>
> git checkout -b delete HEAD^ &&
> git rm letters &&
> mkdir letters &&
> >letters/file &&
> echo "version 1" >letters.txt &&
> git add letters letters.txt &&
> git commit -m deleted
> '
>
> test_expect_failure 'read-tree --reset cleans unmerged entries' '
> test_when_finished "git clean -f" &&
> test_when_finished "git reset --hard" &&
>
> git checkout delete^0 &&
> test_must_fail git merge modify &&
>
> git read-tree --reset HEAD &&
> git ls-files -u >conflicts &&
> test_must_be_empty conflicts
> '
>
> test_expect_failure 'One reset --hard cleans unmerged entries' '
> test_when_finished "git clean -f" &&
> test_when_finished "git reset --hard" &&
>
> git checkout delete^0 &&
> test_must_fail git merge modify &&
>
> git reset --hard &&
> test_path_is_missing .git/MERGE_HEAD &&
> git ls-files -u >conflicts &&
> test_must_be_empty conflicts
> '
>
> test_expect_success 'setup directory/file conflict + simple edit/edit' '
> test_seq 1 10 >numbers &&
> git add numbers &&
> git commit -m initial &&
>
> git checkout -b d-edit &&
> mkdir foo &&
> echo content >foo/bar &&
> git add foo &&
> echo 11 >>numbers &&
> git add numbers &&
> git commit -m "directory and edit" &&
>
> git checkout -b f-edit d-edit^1 &&
> echo content >foo &&
> git add foo &&
> echo eleven >>numbers &&
> git add numbers &&
> git commit -m "file and edit"
> '
>
> test_expect_failure 'git merge --abort succeeds despite D/F conflict' '
> test_when_finished "git clean -f" &&
> test_when_finished "git reset --hard" &&
>
> git checkout f-edit^0 &&
> test_must_fail git merge d-edit^0 &&
>
> git merge --abort &&
> test_path_is_missing .git/MERGE_HEAD &&
> git ls-files -u >conflicts &&
> test_must_be_empty conflicts
> '
>
> test_expect_failure 'git am --skip succeeds despite D/F conflict' '
> test_when_finished "git clean -f" &&
> test_when_finished "git reset --hard" &&
>
> git checkout f-edit^0 &&
> git format-patch -1 d-edit &&
> test_must_fail git am -3 0001*.patch &&
>
> git am --skip &&
> test_path_is_missing .git/rebase-apply &&
> git ls-files -u >conflicts &&
> test_must_be_empty conflicts
> '
>
> test_done
> --- >8 ---