| Message ID | patch-1.4-315da7c2df0-20220421T200733Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 58407e041e62a221d2ac823a42c41b23b2aa984b |
| Headers | show |
| Series | Fix issues and a regression noted by valgrind | expand |
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > Subject: Re: [PATCH 1/4] tests: make RUNTIME_PREFIX compatible with --valgrind The patch text looks more like tests: --valgrind does not work with RUNTIME_PREFIX to me. Is it true that a test that only works with RUNTIME_PREFIX does not work under --valgrind at all? I am wondering if we can do this without introducing a new prerequisite, and if it makes sense to do so if it can be done. The output from $ git grep 'test_expect_success .*RUNTIME_PREFIX' t/ tells me that there are two test pieces that must be run under RUNTIME_PREFIX, and this patch touches both of them. I guess it is not RUNTIME_PREFIX build itself per-se, but it is the way we test RUNTIME_PREFIX is incompatible with how we run our tests under --valgrind, so neither the title on the Subject: header of this message or "looks more like" above is a good one. It is more like tests: using custom GIT_EXEC_PATH breaks --valgrind tests I think. And after having looked at the patch text and thought about the issues enough to come up with the above updated title, I can say that the change looks quite reasonable, both the body of the proposed log message and the solution. Nicely done. It would have been even nicer if I didn't have to think about the issues myself, though ;-) Thanks. > Fix a regression in b7d11a0f5d2 (tests: exercise the RUNTIME_PREFIX > feature, 2021-07-24) where tests that want to set up and test a "git" > wrapper in $PATH conflicted with the t/bin/valgrind wrapper(s) doing > the same. > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > t/t0060-path-utils.sh | 4 ++-- > t/test-lib.sh | 1 + > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh > index 2fe6ae6a4e5..aa35350b6f3 100755 > --- a/t/t0060-path-utils.sh > +++ b/t/t0060-path-utils.sh > @@ -542,7 +542,7 @@ test_lazy_prereq CAN_EXEC_IN_PWD ' > ./git rev-parse > ' > > -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' > +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' > mkdir -p pretend/bin pretend/libexec/git-core && > echo "echo HERE" | write_script pretend/libexec/git-core/git-here && > cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && > @@ -550,7 +550,7 @@ test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' > echo HERE >expect && > test_cmp expect actual' > > -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' > +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' > mkdir -p pretend/bin && > cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && > git config yes.path "%(prefix)/yes" && > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 531cef097db..7f3d323e937 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -1666,6 +1666,7 @@ test -n "$USE_LIBPCRE2" && test_set_prereq PCRE > test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2 > test -z "$NO_GETTEXT" && test_set_prereq GETTEXT > test -n "$SANITIZE_LEAK" && test_set_prereq SANITIZE_LEAK > +test -n "$GIT_VALGRIND_ENABLED" && test_set_prereq VALGRIND > > if test -z "$GIT_TEST_CHECK_CACHE_TREE" > then
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 2fe6ae6a4e5..aa35350b6f3 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -542,7 +542,7 @@ test_lazy_prereq CAN_EXEC_IN_PWD ' ./git rev-parse ' -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' mkdir -p pretend/bin pretend/libexec/git-core && echo "echo HERE" | write_script pretend/libexec/git-core/git-here && cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && @@ -550,7 +550,7 @@ test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' echo HERE >expect && test_cmp expect actual' -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' mkdir -p pretend/bin && cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && git config yes.path "%(prefix)/yes" && diff --git a/t/test-lib.sh b/t/test-lib.sh index 531cef097db..7f3d323e937 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1666,6 +1666,7 @@ test -n "$USE_LIBPCRE2" && test_set_prereq PCRE test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT test -n "$SANITIZE_LEAK" && test_set_prereq SANITIZE_LEAK +test -n "$GIT_VALGRIND_ENABLED" && test_set_prereq VALGRIND if test -z "$GIT_TEST_CHECK_CACHE_TREE" then
Fix a regression in b7d11a0f5d2 (tests: exercise the RUNTIME_PREFIX feature, 2021-07-24) where tests that want to set up and test a "git" wrapper in $PATH conflicted with the t/bin/valgrind wrapper(s) doing the same. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- t/t0060-path-utils.sh | 4 ++-- t/test-lib.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)