Hi,

René Scharfe wrote:

> [Subject: t5004: resurrect original empty tar archive test]
[...]
> The different approaches test different things: The existing one is
> for empty trees, for which we know the exact expected output and thus
> we can simply check it without extracting; the new one is for commits
> with empty trees, whose archives include stamps and so the more
> "natural" check by extraction is a better fit because it focuses on
> the interesting aspect, namely the absence of any archive entries.
>
> Signed-off-by: Rene Scharfe <rene.scha...@lsrfire.ath.cx>

When first reading I was a little confused: does this patch resurrect
the original, existing test for empty tree handling in the form it had
before patch 2/3, or is it adding a new, distinct test that
complements the existing one that patch 2/3 modified?

A quick glance back at v1.8.2.2~7^2 (t5004: fix issue with empty
archive test and bsdtar, 2013-04-10) cleared matters up.  The original
test that is being resurrected is the one from before that commit.

Maybe a reminder in the commit message would help.  E.g.,

        The earlier version of the same check (before 24676f02, "t5004: fix
        issue with empty archive test and bsdtar") revived by this patch tests
        a different thing: The modified check is for empty trees, for which we
        know the exact expected output and thus we can simply check it without
        extracting; the original one is for commits with empty trees, whose
        archives include stamps and so the more "natural" check by extraction
        is a better fit because it focuses on the interesting aspect, namely
        the absence of any archive entries.

With or without such a change,
Reviewed-by: Jonathan Nieder <jrnie...@gmail.com>

Would it make sense to define HEADER_ONLY_TAR_OK as a lazy prereq in
the same file (even though it is only used once), so the code that
checks "tar" is not run if this test is being skipped (e.g.,
using GIT_TEST_SKIP) for some other reason?

Thanks,
Jonathan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to