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

From: Martin Ågren <martin.agren@gmail.com>
Date: May 7, 2018, 10:30 UTC
Message-ID: <CAN0heSrbG+Axs=xq=MdnA=m98Habi_uHS3hj6xweCHqufA7Vag@mail.gmail.com>
In-Reply-To: <20180506231752.975110-11-sandals@crustytoothpaste.net>

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

Show 23 quoted lines

> 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