Re: [PATCH 10/28] t: skip pack tests if not using SHA-1

["Martin Ågren" <martin.agren@gmail.com>]

On 7 May 2018 at 01:17, brian m. carlson <sandals@crustytoothpaste.net> wrote:
> These tests rely on creating packs with specially named objects which
> are necessarily dependent on the hash used.  Skip these tests if we're
> not using SHA-1.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  t/t5308-pack-detect-duplicates.sh | 6 ++++++
>  t/t5309-pack-delta-cycles.sh      | 6 ++++++
>  2 files changed, 12 insertions(+)
>
> diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
> index 156ae9e9d3..6845c1f3c3 100755
> --- a/t/t5308-pack-detect-duplicates.sh
> +++ b/t/t5308-pack-detect-duplicates.sh
> @@ -4,6 +4,12 @@ test_description='handling of duplicate objects in incoming packfiles'
>  . ./test-lib.sh
>  . "$TEST_DIRECTORY"/lib-pack.sh
>
> +if ! test_have_prereq SHA1
> +then
> +       skip_all='not using SHA-1 for objects'
> +       test_done
> +fi

Add something like "FIXME please expand this test", either as a comment
or inside the skip_all? That probably goes for all patches in this
series that skip tests.

As it is now, it feels to me like this is simply stuffing tests away
that are failing. When your primary focus is to run the test suite with
another hash function, I can see why you need to do this change. But if
the goal is to eventually have another hash function and test things at
least as well as before, I think leaving some sort of note here would
help someone who later wants to resurrect those tests that this series
suppressed.

I realize this is related to my comment on the previous series formally
changing the on-disk format [1] and that this comment is along the same
lines as my comment there.

[1] https://public-inbox.org/git/20180427210823.GB722934@genre.crustytoothpaste.net/

Martin