Re: [PATCH v4 2/4] git-p4: add p4d timeout in tests

[Lars Schneider <larsxschneider@gmail.com>]

> On 06 Nov 2015, at 10:23, Eric Sunshine <sunshine@sunshineco.com> wrote:
> 
> On Fri, Nov 6, 2015 at 3:58 AM,  <larsxschneider@gmail.com> wrote:
>> In rare cases p4d seems to hang. This watchdog will kill the p4d
>> process after 300s in any case. That means each individual git p4 test
>> needs to finish before 300s or it will fail.
>> 
>> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
>> ---
>> diff --git a/t/lib-git-p4.sh b/t/lib-git-p4.sh
>> @@ -81,6 +85,19 @@ start_p4d() {
>>        # will be caught with the "kill -0" check below.
>>        i=${P4D_START_PATIENCE:-300}
>>        pid=$(cat "$pidfile")
>> +
>> +       timeout=$(($(date +%s) + $P4D_TIMEOUT))
>> +    while true
> 
> The 'while' line is incorrectly indented with spaces rather than tabs.'

Oh, you're right. One more thing that I will add to my pre mailing list submit check script :-)

> 
>> +       do
>> +               if test $(date +%s) -gt $timeout
> 
> I don't know how portable you intend this to be, but 'date +%s' is not
> universally supported (it's missing on Solaris, for instance, and
> perhaps AIX). For 6a9d16a (filter-branch: add passed/remaining seconds
> on progress, 2015-09-07), we ultimately settled upon detecting +%s
> support dynamically:
> 
>    if date '+%s' 2>/dev/null | grep -q '^[0-9][0-9]*$'
>        # it's supported
>    fi
> 
> Perhaps you'd want to detect this via a lazy prerequisite and skip
> this if not supported?

AFAIK Perforce does not run on Solaris and AIX anyways (see supported platforms [1]). Therefore these tests should not be executed on these platforms. A lazy prerequisite sounds like a good idea to make this explicit!

Thanks,
Lars

[1] https://www.perforce.com/perforce/doc.current/user/relnotes.txt