Web lists-archives.com

Re: [PATCH] t5562: do not reuse output files




Max Kirillov <max@xxxxxxxxxx> writes:

> diff --git a/t/t5562-http-backend-content-length.sh b/t/t5562-http-backend-content-length.sh
> index 90d890d02f..bb53f82c0c 100755
> --- a/t/t5562-http-backend-content-length.sh
> +++ b/t/t5562-http-backend-content-length.sh
> @@ -25,6 +25,8 @@ test_http_env() {
>  	handler_type="$1"
>  	request_body="$2"
>  	shift
> +	(rm -f act.out || true) &&
> +	(rm -f act.err || true) &&

Why "||true"?  If the named file doesn't exist, "rm -f" would
succeed, and if it does exist but somehow we fail to remove, then
these added lines are not preveting the next part from reusing,
i.e. they are not doing what they are supposed to be doing, so we
should detect such a failure (if happens) as an error, no?

IOW, shouldn't it just be more like

	+	rm -f act.out act.err &&

The same comment applies to the other hunk.


>  	env \
>  		CONTENT_TYPE="application/x-git-$handler_type-pack-request" \
>  		QUERY_STRING="/repo.git/git-$handler_type-pack" \
> @@ -155,6 +157,8 @@ test_expect_success 'CONTENT_LENGTH overflow ssite_t' '
>  '
>  
>  test_expect_success 'empty CONTENT_LENGTH' '
> +	(rm -f act.out || true) &&
> +	(rm -f act.err || true) &&
>  	env \
>  		QUERY_STRING="service=git-receive-pack" \
>  		PATH_TRANSLATED="$PWD"/.git/info/refs \