| Message ID | bc4c208f7814c3362fa5949af3cb8d2d2904f386.1573779465.git.liu.denton@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | t: general test cleanup + `set -o pipefail` | expand |
Denton Liu <liu.denton@gmail.com> writes: > In a future patch, we plan on running tests with `set -o pipefail`. > Since we intentionally induce SIGPIPE, before the return code > was being masked away. However, now `git rm` will cause an error code to > be returned because of the SIGPIPE. > > Mark the failing command with `test_must_fail ok=sigpipe` so that > failures induced by SIGPIPE don't propogate. Hmph, would this pipeline _always_ fail? I somehow thought that a process that writes into a pipe that is not being read would fail only if the downstream dies and closes before it write(2)s, and if the output from this "git rm -n" is small enough (say to fit within the pipe buffer) and downstream no-op is slow enough to die, the upstream may probably not notice and happily and successfully exit. : with slow downstream $ ( ( printf "%s\n" a b c d e); echo "my exit $?" 1>&2 ) | ( sleep 2; echo "downstream exit $?" ); echo "overall exit $?" my exit 0 downstream exit 0 overall exit 0 : with slow upstream $ ( (sleep 2; printf "%s\n" a b c d e); echo "my exit $?" 1>&2 ) | ( echo "downstream exit $?" ); echo "overall exit $?" downstream exit 0 my exit 141 overall exit 0 So, no, I do not think this is a good idea. It would be racy. > Signed-off-by: Denton Liu <liu.denton@gmail.com> > --- > t/t3600-rm.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh > index 0ea858d652..d1b3703edb 100755 > --- a/t/t3600-rm.sh > +++ b/t/t3600-rm.sh > @@ -252,7 +252,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft' ' > i=$(( $i + 1 )) > done | git update-index --index-info && > # git command is intentionally placed upstream of pipe to induce SIGPIPE > - git rm -n "some-file-*" | : && > + test_must_fail ok=sigpipe git rm -n "some-file-*" | : && > test_path_is_missing .git/index.lock > '
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh index 0ea858d652..d1b3703edb 100755 --- a/t/t3600-rm.sh +++ b/t/t3600-rm.sh @@ -252,7 +252,7 @@ test_expect_success 'choking "git rm" should not let it die with cruft' ' i=$(( $i + 1 )) done | git update-index --index-info && # git command is intentionally placed upstream of pipe to induce SIGPIPE - git rm -n "some-file-*" | : && + test_must_fail ok=sigpipe git rm -n "some-file-*" | : && test_path_is_missing .git/index.lock '
In a future patch, we plan on running tests with `set -o pipefail`. Since we intentionally induce SIGPIPE, before the return code was being masked away. However, now `git rm` will cause an error code to be returned because of the SIGPIPE. Mark the failing command with `test_must_fail ok=sigpipe` so that failures induced by SIGPIPE don't propogate. Signed-off-by: Denton Liu <liu.denton@gmail.com> --- t/t3600-rm.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)