Re: [PATCH v2 1/8] t1092: add tests for status/add and sparse files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Apr 23, 2021 at 6:34 PM Derrick Stolee via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> Before moving to update 'git status' and 'git add' to work with sparse
> indexes, add an explicit test that ensures the sparse-index works the
> same as a normal sparse-checkout when the worktree contains directories
> and files outside of the sparse cone.
>
> Specifically, 'folder1/a' is a file in our test repo, but 'folder1' is
> not in the sparse cone. When 'folder1/a' is modified, the file
> 'folder1/a' is shown as modified, but adding it fails.

Hmm, I might be doing something wrong, but I think `folder1/a` is not
shown as modified.

$ git init test
$ mkdir test/folder1
$ echo original >test/folder1/a
$ echo original >test/b
$ git -C test add . && git -C test commit -m files
$ git -C test sparse-checkout init --cone --sparse-index
$ ls test
b
$ mkdir test/folder1 && echo modified >test/folder1/a
$ git -C test status
On branch master
You are in a sparse checkout with 50% of tracked files present.
nothing to commit, working tree clean

> This is new
> behavior as of a20f704 (add: warn when asked to update SKIP_WORKTREE
> entries, 2021-04-08). Before that change, these adds would be silently
> ignored.
>
> Untracked files are fine: adding new files both with 'git add .' and
> 'git add folder1/' works just as in a full checkout. This may not be
> entirely desirable, but we are not intending to change behavior at the
> moment, only document it. A future change could alter the behavior to
> be more sensible, and this test could be modified to satisfy the new
> expected behavior.
>
> Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
> ---
>  t/t1092-sparse-checkout-compatibility.sh | 40 ++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh
> index 12e6c453024f..0ec487acd283 100755
> --- a/t/t1092-sparse-checkout-compatibility.sh
> +++ b/t/t1092-sparse-checkout-compatibility.sh
> @@ -232,6 +232,46 @@ test_expect_success 'add, commit, checkout' '
>         test_all_match git checkout -
>  '
>
> +test_expect_success 'status/add: outside sparse cone' '
> +       init_repos &&

A minor suggestion: before recreating folder1/a, we could also test
that `git add folder1/a` will not remove the sparse entry from the
index and will properly warn about it on both sparse repos. I.e.
adding a:

        test_sparse_match test_must_fail git add folder1/a

> +       # folder1 is at HEAD, but outside the sparse cone
> +       run_on_sparse mkdir folder1 &&
> +       cp initial-repo/folder1/a sparse-checkout/folder1/a &&
> +       cp initial-repo/folder1/a sparse-index/folder1/a &&
> +
> +       test_sparse_match git status &&
> +
> +       write_script edit-contents <<-\EOF &&
> +       echo text >>$1
> +       EOF
> +       run_on_all ../edit-contents folder1/a &&

Hmm, we modify `folder1/a` in all repos, but we only try adding it on
the sparse repos, and then we immediately restore it on the full repo.
As we won't use the modified version on the full repo, could this
perhaps be `run_on_sparse` instead? If so, we could also save the
later `git -C full-checkout checkout HEAD -- folder1/a`.

> +       run_on_all ../edit-contents folder1/new &&
> +
> +       test_sparse_match git status --porcelain=v2 &&
> +
> +       # This "git add folder1/a" is completely ignored
> +       # by the sparse-checkout repos. It causes the
> +       # full repo to have a different staged environment.
> +       #
> +       # This is not a desirable behavior, but this test
> +       # ensures that the sparse-index is not the cause
> +       # of a behavior change.

I'm not sure I understand what the undesirable behavior is in this
sentence. Is it "git add folder1/a" erroring out and not updating
`folder1/a`? Or the full repo having a different staged environment?

> +       test_sparse_match test_must_fail git add folder1/a &&
> +       test_sparse_match test_must_fail git add --refresh folder1/a &&
> +       git -C full-checkout checkout HEAD -- folder1/a &&
> +       test_all_match git status --porcelain=v2 &&
> +
> +       test_all_match git add . &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m folder1/new &&
> +
> +       run_on_all ../edit-contents folder1/newer &&
> +       test_all_match git add folder1/ &&
> +       test_all_match git status --porcelain=v2 &&
> +       test_all_match git commit -m folder1/newer
> +'
> +
>  test_expect_success 'checkout and reset --hard' '
>         init_repos &&
>
> --
> gitgitgadget
>



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux