git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 07/16] t5312: be more assertive about command failure

From
Jeff King <peff@peff.net>
Date
Sep 24, 2021, 18:37 UTC
Message-ID
<YU4a19JXV67wsV+o@coredump.intra.peff.net>
In-Reply-To
<YU4ZOF9+ubmoItmK@coredump.intra.peff.net>

When repacking or pruning in a corrupted repository, our tests in t5312 argue that it is OK to complete the operation or bail, as long as we don't actually delete the objects pointed to by the corruption.

This isn't a wrong line of reasoning, but the tests are a bit permissive by using test_might_fail. The fact is that we _do_ bail currently, and if we ever stopped doing so, that would be worthy of a human investigating. So let's switch these to test_must_fail.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5312-prune-corruption.sh | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/t/t5312-prune-corruption.sh b/t/t5312-prune-corruption.sh
index c7010fb754..1522a4ba8e 100755
--- a/t/t5312-prune-corruption.sh
+++ b/t/t5312-prune-corruption.sh
@@ -7,6 +7,9 @@ if we see, for example, a ref with a bogus name, it is OK either to
 bail out or to proceed using it as a reachable tip, but it is _not_
 OK to proceed as if it did not exist. Otherwise we might silently
 delete objects that cannot be recovered.
+
+Note that we do assert command failure in these cases, because that is
+what currently happens. If that changes, these tests should be revisited.
 '
 GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
 export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
@@ -35,7 +38,7 @@ test_expect_success 'create history reachable only from a bogus-named ref' '
 test_expect_success 'pruning does not drop bogus object' '
 	test_when_finished "git hash-object -w -t commit saved" &&
 	create_bogus_ref &&
-	test_might_fail git prune --expire=now &&
+	test_must_fail git prune --expire=now &&
 	git cat-file -e $bogus
 '
 
@@ -53,9 +56,9 @@ test_expect_success 'non-destructive repack ignores bogus name' '
 
 test_expect_success 'destructive repack keeps packed object' '
 	create_bogus_ref &&
-	test_might_fail git repack -Ad --unpack-unreachable=now &&
+	test_must_fail git repack -Ad --unpack-unreachable=now &&
 	git cat-file -e $bogus &&
-	test_might_fail git repack -ad &&
+	test_must_fail git repack -ad &&
 	git cat-file -e $bogus
 '
 
@@ -83,7 +86,7 @@ test_expect_success 'create history with missing tip commit' '
 
 test_expect_success 'pruning with a corrupted tip does not drop history' '
 	test_when_finished "git hash-object -w -t commit saved" &&
-	test_might_fail git prune --expire=now &&
+	test_must_fail git prune --expire=now &&
 	git cat-file -e $recoverable
 '
 
-- 
2.33.0.1071.gb37e412355
Previous: Jeff KingNext: Jeff King
Message 10 of 22 in “enabling GIT_REF_PARANOIA by default”
  1. 0/16 enabling GIT_REF_PARANOIA by defaultJeff King, Sep 24, 2021
  2. 01/16 t7900: clean up some more broken refsJeff King, Sep 24, 2021
  3. Jonathan TanSep 27, 2021
  4. Jeff KingSep 27, 2021
  5. 02/16 t5516: don't use HEAD ref for invalid ref-deletion testsJeff King, Sep 24, 2021
  6. 03/16 t5600: provide detached HEAD for corruption failuresJeff King, Sep 24, 2021
  7. 04/16 t5312: drop "verbose" helperJeff King, Sep 24, 2021
  8. 05/16 t5312: create bogus ref as necessaryJeff King, Sep 24, 2021
  9. 06/16 t5312: test non-destructive repackJeff King, Sep 24, 2021
  10. 07/16 t5312: be more assertive about command failureJeff King, Sep 24, 2021
  11. 08/16 refs-internal.h: move DO_FOR_EACH_* flags next to each otherJeff King, Sep 24, 2021
  12. 09/16 refs-internal.h: reorganize DO_FOR_EACH_* flag documentationJeff King, Sep 24, 2021
  13. 10/16 refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flagJeff King, Sep 24, 2021
  14. 11/16 refs: omit dangling symrefs when using GIT_REF_PARANOIAJeff King, Sep 24, 2021
  15. 12/16 refs: turn on GIT_REF_PARANOIA by defaultJeff King, Sep 24, 2021
  16. Jonathan TanSep 27, 2021
  17. 13/16 repack, prune: drop GIT_REF_PARANOIA settingsJeff King, Sep 24, 2021
  18. 14/16 ref-filter: stop setting FILTER_REFS_INCLUDE_BROKENJeff King, Sep 24, 2021
  19. 15/16 ref-filter: drop broken-ref code entirelyJeff King, Sep 24, 2021
  20. 16/16 refs: drop "broken" flag from for_each_fullref_in()Jeff King, Sep 24, 2021
  21. Jonathan TanSep 27, 2021
  22. Junio C HamanoSep 24, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.