Re: [PATCH v2 1/9] t7002: add tests for moving from in-cone to out-of-cone

[Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>]

On 8/9/2022 8:51 AM, Victoria Dye wrote:
> 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!
>> 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.
>
> 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