Re: [PATCH] t5561: get rid of racy appending to logfile

From: Jeff King <peff@peff.net>
Date: Sep 24, 2015, 01:45 UTC
Message-ID: <20150924014541.GB6442@sigill.intra.peff.net>
In-Reply-To: <1443054017-8312-1-git-send-email-s-beyer@gmx.net>

On Thu, Sep 24, 2015 at 02:20:17AM +0200, Stephan Beyer wrote:

Show 5 quoted lines

> The definition of log_div() appended information to the web server's
> logfile to make the test more readable. However, it could happen that
> this information is written before the web server writes its log line
> (this consistently happens with a PROFILE=GEN build), and hence the
> test failed.

I don't know if you want to add more detail here or not, but I believe the race is based on the amount of time between git-http-backend finishes serving the request, and when the process exits. We run log_div() as soon as the first is done, but Apache waits for the latter to flush out the logfile. And PROFILE=GEN lengthens that time.

Show 11 quoted lines

> To get rid of this behavior, the logfile is not touched at all. This
> commit removes log_div() and its calls. The readability-improving
> information is kept in the test but filtered out before comparing
> it to the actual logfile.
> 
> Signed-off-by: Stephan Beyer <s-beyer@gmx.net>
> ---
>  t/t5560-http-backend-noserver.sh |  4 ----
>  t/t5561-http-backend.sh          |  8 +-------
>  t/t556x_common                   | 12 ------------
>  3 files changed, 1 insertion(+), 23 deletions(-)

This looks good to me.

I'd have written the grep as:

> -cat >exp <<EOF
> +grep -e '^[GP]' >exp <<EOF

grep '^[^#]' >exp <<EOF

to exclude blank lines and comments, but I doubt it matters in practice (I cannot imagine any line except GET or POST here).

-Peff