On 12-sep-2023 04:35:28, Jeff King wrote:

On Sun, Sep 10, 2023 at 01:08:11AM +0200, Rubén Justo wrote:

quoted

GIT_TEST_PASSING_SANITIZE_LEAK=true and GIT_TEST_SANITIZE_LEAK_LOG=true
use internnlly the --invert-exit-code machinery.  Therefore if the user
wants to use --invert-exit-code in combination with them, the result
will be confusing.

For the same reason, we are already using BAIL_OUT if the user tries to
combine GIT_TEST_PASSING_SANITIZE_LEAK=check with --invert-exit-code.

Let's do the same for GIT_TEST_PASSING_SANITIZE_LEAK=true and
GIT_TEST_SANITIZE_LEAK_LOG=true.

OK, so we are trying to find a case where the user is triggering
--invert-exit-code themselves and complaining. But in the code...

quoted

@@ -1557,15 +1557,25 @@ then
 			say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
 			invert_exit_code=t
 		fi
-	elif test -z "$passes_sanitize_leak" &&
-	     test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
+	elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
 	then
-		skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
-		test_done
+		if test -n "$invert_exit_code"
+		then
+			BAIL_OUT "cannot use --invert-exit-code under GIT_TEST_PASSING_SANITIZE_LEAK=true"
+		elif test -z "$passes_sanitize_leak"
+		then
+			skip_all="skipping $this_test under GIT_TEST_PASSING_SANITIZE_LEAK=true"
+			test_done
+		fi
 	fi

You can see at the top of the context that we will set
invert_exit_code=t ourselves, which will then complain here:

quoted

 	if test_bool_env GIT_TEST_SANITIZE_LEAK_LOG false
 	then
+		if test -n "$invert_exit_code"
+		then
+			BAIL_OUT "cannot use --invert-exit-code and GIT_TEST_SANITIZE_LEAK_LOG=true"
+		fi
+
 		if ! mkdir -p "$TEST_RESULTS_SAN_DIR"
 		then
 			BAIL_OUT "cannot create $TEST_RESULTS_SAN_DIR"

That varible-set in the earlier context is from running in "check" mode.
So:

  make GIT_TEST_PASSING_SANITIZE_LEAK=check GIT_TEST_SANITIZE_LEAK_LOG=true

will now always fail. But this is the main way you'd want to run it
(enabling the leak log catches more stuff, and the log-check function
you touch in patch 2 already covers check mode).

So I think you'd have to hoist your check above the if/else for setting
up PASSING_SANITIZE_LEAK modes.

Arrg, sorry.  You're right.

This was part of the series by mistake.  Please, discard it.

In my tree, I have a previous commit with:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 87cfea9e9a..46b8a76e9c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -1556,7 +1556,6 @@ then
                if test -z "$passes_sanitize_leak"
                then
                        say "in GIT_TEST_PASSING_SANITIZE_LEAK=check mode, setting --invert-exit-code for TEST_PASSES_SANITIZE_LEAK != true"
-                       invert_exit_code=t
                fi
        elif test_bool_env GIT_TEST_PASSING_SANITIZE_LEAK false
        then

that is part of an unsent attempt to make:

  $ make SANITIZE=leak T=t7510-signed-commit.sh GIT_TEST_PASSING_SANITIZE_LEAK=check test
 
not to suggest, when GPG is missing, that t7510-signed-commit.sh is a
candidate to be marked as leak-free.  Which is distracting to me.

However I was not satisfied with the solution and discarded it.  But
unfortunately not entirely.  Sorry. 

-Peff