Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> OK. sleep.pid is a reasonable easy-to-access side effect we can
>> observe to make sure that the sleep-one-second merge driver was
>> indeed invoked, which was missing from the earlier round.
>
> No, this is incorrect. The condition that we need to know applies is that
> the script is still running, and blocking if the bug reappears.
OK, I see what you are saying, and I see a few things wrong in here:
* First, the test is titled in a misleading way. In the context of
a patch that was titled ad65f7e3b7 ("t6026-merge-attr: child
processes must not inherit index.lock handles", 2016-08-18), it
might have been clear enough to say "does not lock index", but
the sleeping is to make sure that we would notice if the fd to
the index.lock leaked to the child process by mistake, and the
way to do so is that the child arranges the leaked fd to be kept
open after it exits (by spawning "sleep"). The test was never
about "does not lock index" (the driver does not take any lock by
itself in the first place).
* There are three possible outcome from this test:
- 'git merge' fails.
This is expected to happen only on Windows and if the code gets
broken and starts leaking the fd.
- 'git merge' finishes correctly, the sleep is still running when
test_when_finished goes to cull it.
In this case, we KNOW there wasn't any fd leak IF we are on
Windows where a leaked FD would not allow 'git merge' to
succeed. But on other platforms, fd leak that may cause
trouble for Windows friends will not be caught.
- 'git merge' finishes correctly, the sleep is no longer
running because the machine was heavily loaded; a workaround is
to tolerate failure of culling it.
In this case, we cannot tell anything from the test. Even if
the fd was leaked, 'git merge' may have succeeded even on
Windows.
As everybody knows there is no appropriate timeout value that is
good for everybody. I wonder if we can replace the sleep 1 with
something like
( while sleep 3600; do :; done ) &
so that leaked fd will be kept even in any heavily loaded
environment instead?