Koji Nakamaru <koji.nakamaru@xxxxxxxx> writes: >> > +test_expect_success "submodule implicitly starts daemon by pull" ' >> > + test_atexit "stop_watchdog" && >> > + test_when_finished "stop_git && rm -rf cloned super sub" && >> >> If stop_git ever returns with non-zero status, "rm -rf" will be >> skipped, which I am not sure is a good idea. >> >> The whole test_when_finished would fail in such a case, so you would >> notice the problem right away, which is a plus, though. > > t/README discusses that test_when_finished and test_atexit differ about > the "--immediate" option. As git and its subprocesses are the test > target, I moved stop_git to the current place. This might be however > confusing when someone later reads this test. Should we simply put > stop_git and stop_watchdong in test_atexit? That is not what I meant. I was merely questioning the &&-chaining that stops "rm -fr" from running if stop_git ever fails (and your earlier iteration you had multiple "rm -fr" ;-chained, not &&-chained---not using && is often more appropriate in a when_finished handler). >> > + set -m && >> >> I have to wonder how portable (and necessary) this is. >> >> POSIX says it shall be supported if the implementation supports the >> User Portability Utilities option. It also says that it was added >> to apply only to the UPE because it applies primarily to interactive >> use, not shell script applications. And our test scripts are of >> course not interactive. > > How about the following modification? It still utilizes $git_pgid to > filter processes, but avoids "set -m". Nah, your original reads much better, and the code is grabbing and using the process group information anyway (and my question about "-m" was more about "should we be relying on process group features in this test to kill them all?"). I am OK with the idea that we can assume, at least among the platforms that support fsmonitor, that sending a signal to a process group would cause the signal delivered to the member processes just as we expect. Thanks.
