Re: [PATCH 3/7] worktree move: new command

On Mon, Feb 12, 2018 at 11:15:06PM +0100, Martin Ågren wrote:

On 12 February 2018 at 10:56, Duy Nguyen [off-list ref] wrote:

quoted

On Wed, Feb 7, 2018 at 3:05 AM, Martin Ågren [off-list ref] wrote:

quoted

On 6 February 2018 at 03:13, Jeff King [off-list ref] wrote:

quoted

On Mon, Feb 05, 2018 at 08:28:10PM +0700, Duy Nguyen wrote:

quoted

I learned SANITIZE=leak today! It not only catches this but also "dst".

Jeff is there any ongoing effort to make the test suite pass with
SANITIZE=leak? My t2038 passed, so I went ahead with the full test
suite and saw so many failures. I did see in your original mails that
you focused on t0000 and t0001 only..

Yeah, I did those two scripts to try to prove to myself that the
approach was good. But I haven't really pushed it any further.

Martin Ågren (cc'd) did some follow-up work, but I think we still have a
long way to go.

Agreed. :-)

Should we mark the test files that pass SANITIZE=leak and run as a sub
testsuite? This way we can start growing it until it covers everything
(unlikely) and make sure people don't break what's already passed.

Of course I don't expect everybody to run this new make target with
SANITIZE=leak. Travis can do that for us and hopefully have some way
to tell git@vger about new breakages.

Makes sense to try to make sure that we don't regress leak-free tests. I
don't know what our Travis-budget looks like, but I would volunteer to
run something like this periodically using my own cycles.

My experience with the innards of our Makefiles and test-lib.sh is
non-existant, but from a very cursory look it seems like something as
simple as loading GIT_SKIP_TESTS from a blacklist-file might do the
trick.

Yeah my first throught was something along that line too. But maybe
it's nicer to mark a test file leak-free like this:

-- 8< --

diff --git a/t/t2028-worktree-move.sh b/t/t2028-worktree-move.sh
index 459f676683..1446f075b7 100755
--- a/t/t2028-worktree-move.sh
+++ b/t/t2028-worktree-move.sh

@@ -2,6 +2,8 @@
 
 test_description='test git worktree move, remove, lock and unlock'
 
+test_leak_free=yes
+
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-- 8< --

And we can run these test files with a new option --leak-check (or
something like that, we already support a similar option --valgrind).

Most of the work will be in test-lib.sh. If we detect --leak-check and
test_leak_free is not set, we skip the whole file. In the far far far
future when all files have test_leak_free=yes, we can flip the default
and delete these lines.

It would be nice if test-lib.sh can determine if 'git' binary is built
with SANITIZE=yes, but I think we can live without it.

I could try to look into it in the next couple of days.

Have fun :) Let me know if you give up though, I'll give it a shot.
--
Duy