On 12-sep-2023 04:35:28, Jeff King wrote:
> On Sun, Sep 10, 2023 at 01:08:11AM +0200, Rubén Justo wrote:
>
> > 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...
>
> > @@ -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:
>
> > 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
