Re: [PATCH v2] sha1-file: test the error behavior of alt_odb_usable()

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 9, 2019, 09:43 UTC
Message-ID: <xmqq5zrn7bua.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <8736mra7nq.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> I'll leave it up to you if you want to queue just the test patch or drop

As I said in a separate message, I think it is good to make sure that fsck does not crash. I do not think it is good to grep in its output.

> it. I figured I'd re-send just that since I figured just fixing the
> blindspot of the current behavior would be a good thing on its own, not
> as an endorsement of the current behavior, just a "this is the current
> known behavior" regression test.

If the behaviour is undesirable one, we could document the current "breakage" with "test_expect_failure", whether we plan to fix it immediately. It is OK if readers cannot tell between a bug that is expected to stay forever with us, or a bug that somebody is actively working on.

But unfortunately, there is no separate "test_merely_documenting", that is different from "test_expect_success", so even if we claim "this is not an endorsement, but is merely documenting the current behaviour" when we add such a test, there is no way for future readers to tell between the two, short of going back to "git blame" and seeing the log message.

For that reason, I do not think it is a good practice to document the "current behaviour that happens to be" the same way as "the behaviour we desire" in test_expect_success.