Re: [PATCH 06/28] t0000: annotate with SHA1 prerequisite

From: Martin Ågren <martin.agren@gmail.com>
Date: May 7, 2018, 10:24 UTC
Message-ID: <CAN0heSqu9gwsrgV0rnPe8=vANhBA+Z7fZ3UNTeG9S1QiJBsksw@mail.gmail.com>
In-Reply-To: <20180506231752.975110-7-sandals@crustytoothpaste.net>

On 7 May 2018 at 01:17, brian m. carlson <sandals@crustytoothpaste.net> wrote:

Show 52 quoted lines

> Since this is a core test that tests basic functionality, annotate the
> assertions that have dependencies on SHA-1 with the appropriate
> prerequisite.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  t/t0000-basic.sh | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index 7fd87dd544..af61d083b4 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -839,7 +839,7 @@ test_expect_success 'writing tree out with git write-tree' '
>  '
>
>  # we know the shape and contents of the tree and know the object ID for it.
> -test_expect_success 'validate object ID of a known tree' '
> +test_expect_success SHA1 'validate object ID of a known tree' '
>         test "$tree" = 7bb943559a305bdd6bdee2cef6e5df2413c3d30a
>      '
>
> @@ -882,7 +882,7 @@ test_expect_success 'showing stage with git ls-files --stage' '
>         git ls-files --stage >current
>  '
>
> -test_expect_success 'validate git ls-files output for a known tree' '
> +test_expect_success SHA1 'validate git ls-files output for a known tree' '
>         cat >expected <<-\EOF &&
>         100644 f87290f8eb2cbbea7857214459a0739927eab154 0       path0
>         120000 15a98433ae33114b085f3eb3bb03b832b3180a01 0       path0sym
> @@ -900,7 +900,7 @@ test_expect_success 'writing tree out with git write-tree' '
>         tree=$(git write-tree)
>  '
>
> -test_expect_success 'validate object ID for a known tree' '
> +test_expect_success SHA1 'validate object ID for a known tree' '
>         test "$tree" = 087704a96baf1c2d1c869a8b084481e121c88b5b
>  '
>
> @@ -908,7 +908,7 @@ test_expect_success 'showing tree with git ls-tree' '
>      git ls-tree $tree >current
>  '
>
> -test_expect_success 'git ls-tree output for a known tree' '
> +test_expect_success SHA1 'git ls-tree output for a known tree' '
>         cat >expected <<-\EOF &&
>         100644 blob f87290f8eb2cbbea7857214459a0739927eab154    path0
>         120000 blob 15a98433ae33114b085f3eb3bb03b832b3180a01    path0sym
> @@ -924,7 +924,7 @@ test_expect_success 'showing tree with git ls-tree -r' '
>         git ls-tree -r $tree >current
>  '

[snip]

This could be more centralized at the top of the file, more likely to be noticed during a `make test` and easier to adapt in the future. Maybe something like this at the top of the file:

if hash for storage is SHA-1 then
        knowntree=7bb943559a305bdd6bdee2cef6e5df2413c3d30a
        path0=f87290f8eb2cbbea7857214459a0739927eab154
        ....
else
        skip_all='unknown hash, but you really need to expand this test'
        # or even error out!
fi

When we add NewHash, we get to add an "else if". Otherwise, we'd be plugging in NewHash without testing some very basic stuff.

Ok, so `skip_all` is quite hard, but skipping certain basic tests also feels really shaky. Adding a new hash algo without adapting this test should be a no-go, IMHO.

Martin