Re: [PATCH v1 1/7] t1005: assert output of ls-files

From: Michael J Gruber <git@grubix.eu>
Date: Mar 14, 2023, 08:51 UTC
Message-ID: <167878391659.3913.5697119728018854973.git@grubix.eu>
In-Reply-To: <20230312201520.370234-2-rybak.a.v@gmail.com>

Andrei Rybak venit, vidit, dixit 2023-03-12 21:15:13:

Show 70 quoted lines

> Test 'reset should work' in t1005-read-tree-reset.sh compares two files
> "expect" and "actual" to assert the expected output of "git ls-files".
> Several other tests in the same file also create files "expect" and
> "actual", but don't use them in assertions.
> 
> Assert output of "git ls-files" in t1005-read-tree-reset.sh to improve
> test coverage.
> 
> Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com>
> ---
>  t/t1005-read-tree-reset.sh | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/t/t1005-read-tree-reset.sh b/t/t1005-read-tree-reset.sh
> index 12e30d77d0..26be4a2b5a 100755
> --- a/t/t1005-read-tree-reset.sh
> +++ b/t/t1005-read-tree-reset.sh
> @@ -41,7 +41,8 @@ test_expect_success 'reset should remove remnants from a failed merge' '
>         git ls-files -s &&
>         read_tree_u_must_succeed --reset -u HEAD &&
>         git ls-files -s >actual &&
> -       ! test -f old
> +       ! test -f old &&
> +       test_cmp expect actual
>  '
>  
>  test_expect_success 'two-way reset should remove remnants too' '
> @@ -56,7 +57,8 @@ test_expect_success 'two-way reset should remove remnants too' '
>         git ls-files -s &&
>         read_tree_u_must_succeed --reset -u HEAD HEAD &&
>         git ls-files -s >actual &&
> -       ! test -f old
> +       ! test -f old &&
> +       test_cmp expect actual
>  '
>  
>  test_expect_success 'Porcelain reset should remove remnants too' '
> @@ -71,7 +73,8 @@ test_expect_success 'Porcelain reset should remove remnants too' '
>         git ls-files -s &&
>         git reset --hard &&
>         git ls-files -s >actual &&
> -       ! test -f old
> +       ! test -f old &&
> +       test_cmp expect actual
>  '
>  
>  test_expect_success 'Porcelain checkout -f should remove remnants too' '
> @@ -86,7 +89,8 @@ test_expect_success 'Porcelain checkout -f should remove remnants too' '
>         git ls-files -s &&
>         git checkout -f &&
>         git ls-files -s >actual &&
> -       ! test -f old
> +       ! test -f old &&
> +       test_cmp expect actual
>  '
>  
>  test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '
> @@ -101,7 +105,8 @@ test_expect_success 'Porcelain checkout -f HEAD should remove remnants too' '
>         git ls-files -s &&
>         git checkout -f HEAD &&
>         git ls-files -s >actual &&
> -       ! test -f old
> +       ! test -f old &&
> +       test_cmp expect actual
>  '
>  
>  test_done
> -- 
> 2.39.2
>

Just in case someone else was wondering, too:

All these subtests write to `expect` just before the provided context lines, so there indeed is something to compare to, and it is the output of `git ls-files -s` before any changes and resets. As a consequence, these subtests checked only removal of remnants in the woring tree before the patch, and they check removal of index bits after the patch (additionally).

Looks fine to me - though one could probably use `git ls-files -s -o` or such instead, the suggested version is more "explicit".

Michael