Re: [PATCH v2 1/9] t7002: add tests for moving from in-cone to out-of-cone
- From
Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
- Date
- Aug 9, 2022, 02:55 UTC
- Message-ID
- <651d89e2-5282-2cf8-ffc3-8650a023c80a@gmail.com>
- In-Reply-To
- <bd80881d-b2a3-c220-8f2d-a07a46e14207@github.com>
On 8/9/2022 8:51 AM, Victoria Dye wrote:
Show 17 quoted lines
> Shaoxuan Yuan wrote: >> Add corresponding tests to test that user can move an in-cone <source> >> to out-of-cone <destination> when --sparse is supplied. >> >> Such <source> can be either clean or dirty, and moving it results in >> different behaviors: >> >> A clean move should move the <source> to the <destination>, both in the >> working tree and the index, then remove the resulted path from the >> working tree, and turn on its CE_SKIP_WORKTREE bit. > Looking ahead to patch 6, I think the part about "move it in the working > tree, then delete from the working tree" doesn't quite match your > implementation. Instead, if I'm not mistaken, what happens is: > > 1. Move <source> to <destination> in the index (do *not* create > <destination> in the worktree) > 2. Delete <source> from the working tree
You are right. The description here seems to be my first in-mind implementation, which I wrote down for record. Later I changed the implementation to the one you mentioned here, but somehow forgot to change the description here.
Thanks for the catch!
Show 159 quoted lines
>> A dirty move should move the <source> to the <destination>, both in the
>> working tree and the index, but should *not* remove the resulted path
>> from the working tree and should *not* turn on its CE_SKIP_WORKTREE bit.
> Makes sense.
>
>> Also make sure that if <destination> exists in the index (existing
>> check for if <destination> is in the worktree is not enough in
>> in-to-out moves), warn user against the overwrite. And Git should force
>> the overwrite when supplied with -f or --force.
> Also makes sense.
>
>> Helped-by: Derrick Stolee <derrickstolee@github.com>
>> Helped-by: Victoria Dye <vdye@github.com>
>> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
>> ---
>> t/t7002-mv-sparse-checkout.sh | 122 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 122 insertions(+)
>>
>> diff --git a/t/t7002-mv-sparse-checkout.sh b/t/t7002-mv-sparse-checkout.sh
>> index 71fe29690f..9b3a9ab4c3 100755
>> --- a/t/t7002-mv-sparse-checkout.sh
>> +++ b/t/t7002-mv-sparse-checkout.sh
>> @@ -290,4 +290,126 @@ test_expect_success 'move sparse file to existing destination with --force and -
>> test_cmp expect sub/file1
>> '
>>
>> +test_expect_failure 'move clean path from in-cone to out-of-cone' '
>> + test_when_finished "cleanup_sparse_checkout" &&
>> + setup_sparse_checkout &&
>> +
>> + test_must_fail git mv sub/d folder1 2>stderr &&
>> + cat sparse_error_header >expect &&
>> + echo "folder1/d" >>expect &&
>> + cat sparse_hint >>expect &&
>> + test_cmp expect stderr &&
>> +
>> + git mv --sparse sub/d folder1 2>stderr &&
>> + test_must_be_empty stderr &&
>> +
>> + test_path_is_missing sub/d &&
>> + test_path_is_missing folder1/d &&
>> + git ls-files -t >actual &&
>> + ! grep "^H sub/d\$" actual &&
>> + grep "S folder1/d" actual
>> +'
>> +
>> +test_expect_failure 'move clean path from in-cone to out-of-cone overwrite' '
>> + test_when_finished "cleanup_sparse_checkout" &&
>> + setup_sparse_checkout &&
>> + echo "sub/file1 overwrite" >sub/file1 &&
>> + git add sub/file1 &&
>> +
>> + test_must_fail git mv sub/file1 folder1 2>stderr &&
>> + cat sparse_error_header >expect &&
>> + echo "folder1/file1" >>expect &&
>> + cat sparse_hint >>expect &&
>> + test_cmp expect stderr &&
>> +
>> + test_must_fail git mv --sparse sub/file1 folder1 2>stderr &&
>> + echo "fatal: destination exists in the index, source=sub/file1, destination=folder1/file1" \
>> + >expect &&
>> + test_cmp expect stderr &&
>> +
>> + git mv --sparse -f sub/file1 folder1 2>stderr &&
>> + test_must_be_empty stderr &&
>> +
>> + test_path_is_missing sub/file1 &&
>> + test_path_is_missing folder1/file1 &&
>> + git ls-files -t >actual &&
>> + ! grep "H sub/file1" actual &&
>> + grep "S folder1/file1" actual &&
>> +
>> + # compare file content before move and after move
>> + echo "sub/file1 overwrite" >expect &&
>> + git ls-files -s -- folder1/file1 | awk "{print \$2}" >oid &&
>> + git cat-file blob $(cat oid) >actual &&
>> + test_cmp expect actual
>> +'
>> +
>> +test_expect_failure 'move dirty path from in-cone to out-of-cone' '
>> + test_when_finished "cleanup_sparse_checkout" &&
>> + setup_sparse_checkout &&
>> + echo "modified" >>sub/d &&
>> +
>> + test_must_fail git mv sub/d folder1 2>stderr &&
>> + cat sparse_error_header >expect &&
>> + echo "folder1/d" >>expect &&
>> + cat sparse_hint >>expect &&
>> + test_cmp expect stderr &&
>> +
>> + git mv --sparse sub/d folder1 2>stderr &&
>> +
>> + test_path_is_missing sub/d &&
>> + test_path_is_file folder1/d &&
>> + git ls-files -t >actual &&
>> + ! grep "^H sub/d\$" actual &&
>> + grep "H folder1/d" actual
>> +'
>> +
>> +test_expect_failure 'move dir from in-cone to out-of-cone' '
>> + test_when_finished "cleanup_sparse_checkout" &&
>> + setup_sparse_checkout &&
>> +
>> + test_must_fail git mv sub/dir folder1 2>stderr &&
>> + cat sparse_error_header >expect &&
>> + echo "folder1/dir/e" >>expect &&
>> + cat sparse_hint >>expect &&
>> + test_cmp expect stderr &&
>> +
>> + git mv --sparse sub/dir folder1 2>stderr &&
>> + test_must_be_empty stderr &&
>> +
>> + test_path_is_missing folder1 &&
>> + git ls-files -t >actual &&
>> + ! grep "H sub/dir/e" actual &&
>> + grep "S folder1/dir/e" actual
>> +'
>> +
>> +test_expect_failure 'move partially-dirty dir from in-cone to out-of-cone' '
>> + test_when_finished "cleanup_sparse_checkout" &&
>> + setup_sparse_checkout &&
>> + touch sub/dir/e2 sub/dir/e3 &&
>> + git add sub/dir/e2 sub/dir/e3 &&
>> + echo "modified" >>sub/dir/e2 &&
>> + echo "modified" >>sub/dir/e3 &&
>> +
>> + test_must_fail git mv sub/dir folder1 2>stderr &&
>> + cat sparse_error_header >expect &&
>> + echo "folder1/dir/e" >>expect &&
>> + echo "folder1/dir/e2" >>expect &&
>> + echo "folder1/dir/e3" >>expect &&
>> + cat sparse_hint >>expect &&
>> + test_cmp expect stderr &&
>> +
>> + git mv --sparse sub/dir folder1 2>stderr &&
>> +
>> + test_path_is_missing folder1/dir/e &&
>> + test_path_is_file folder1/dir/e2 &&
>> + test_path_is_file folder1/dir/e3 &&
>> + git ls-files -t >actual &&
>> + ! grep "H sub/dir/e" actual &&
>> + ! grep "H sub/dir/e2" actual &&
>> + ! grep "H sub/dir/e3" actual &&
>> + grep "S folder1/dir/e" actual &&
>> + grep "H folder1/dir/e2" actual &&
>> + grep "H folder1/dir/e3" actual
>> +'
>> +
> There are two other test cases I'd be interested in seeing:
>
> 1. Move a (clean or dirty) in-cone source file to an out-of-cone destination
> *file*. For example:
>
> echo test >sub/dir/file1 &&
> git add sub/dir/file1 &&
> git mv --sparse sub/dir/file1 folder1/file1
>
> I'm assuming this should behave the same way as show in 'move clean path
> from in-cone to out-of-cone overwrite'.OK.
Show 12 quoted lines
> > 2. Move a (clean or dirty) in-cone source directory to an out-of-cone > destination where one or more files in <src> overwrite files in <dst>. > For example, something like: > > echo test >sub/dir/file1 && > git add sub/dir/file1 && > git mv --sparse sub/dir folder1 > > I don't have a strong opinion on the behavior (does it fail the whole > 'mv' operation? move everything except the files that overwrite > something?), but it would help to have it documented via test here.
OK. I think it will fail the whole `mv` operation. The program will report unspecified overwrite during the early checking phase. The actual "moving" phase won't be touched at all because Git complains early.
-- Thanks, Shaoxuan