Junio C Hamano <gits...@pobox.com> writes:
> Max Kirillov <m...@max630.net> 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,...
Another thing. The analysis in your log message talks about a stray
process holding open filehandles to these files. An attempt to remove
them in such a stat would fail on some systems, so "||true" would not
help, would it? It just hides the failure to remove, and when ||true
is useful in hiding th failure _is_ when such a stray process is still
there, waiting to corrupt the output of the next request and breaking
the test, no?
I do agree that forcing the parent to wait, like you described in
the comment, would be far more preferrable, but until that happens,
a better workaround might be to write into unique output filenames
(act1.out, act2.out, etc.); that way, you do not have to worry about
the output file for the next request getting clobbered by a stale
process handling the previous request. But at the same time,
wouldn't this suggest that the test or the previous request may see
an incomplete output, as the analysed problem is that such a process
is writing to the output file very late, while we are preparing to
test the enxt request, which meas we have already checked the output
file for the previous request, right? So even without ||true, I am
not sure how true a "solution" this change is to the issue.
