Re: [PATCH] gitweb tests: Skip tests when we don't have Time::HiRes

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Feb 27, 2017, 18:25 UTC
Message-ID: <CACBZZX6gz8rs-Tf3DvG=J1WA5X+=ObfU-kGoDqVeeW0_ALQYXw@mail.gmail.com>
In-Reply-To: <4b34e3a0-3da7-d821-2a7f-9a420ac1d3f6@gmail.com>

On Mon, Feb 27, 2017 at 6:48 PM, Jakub Narębski <jnareb@gmail.com> wrote:

Show 14 quoted lines

> W dniu 27.02.2017 o 13:37, Ævar Arnfjörð Bjarmason pisze:
>> Change the gitweb tests to skip when we can't load the Time::HiRes
>> module.
>
> Could you tell us in the commit message why this module is needed?
> Is it because gitweb loads it unconditionally, or does that at least
> in the default configuration, or is it used in tests, or...?
>
> [I see it is somewhat addressed below]
>
>>
>> This module has bee in perl core since v5.8, which is the oldest
>
> s/bee/been/

I'll clarify that in a re-roll & fix the typo, pending any other comments. Thanks!

Show 13 quoted lines

>> version we support, however CentOS (and perhaps some other
>> distributions) carve it into its own non-core-perl package that's not
>> installed along with /usr/bin/perl by default. Without this we'll hard
>> fail the gitweb tests when trying to load the module.
>
> I see that it because gitweb.perl as the following at line 20:
>
>         use Time::HiRes qw(gettimeofday tv_interval);
>
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
>
> Good catch (if a strange one...).

This and the associated cvs tests failing as root patch I submitted were discovered when trying to build git in the standard mock build environment on CentOS. It creates a chroot and rpm installs just the packages you declare, so issues like these crop up.

Show 16 quoted lines

>> ---
>>  t/gitweb-lib.sh | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/t/gitweb-lib.sh b/t/gitweb-lib.sh
>> index d5dab5a94f..116c3890e6 100644
>> --- a/t/gitweb-lib.sh
>> +++ b/t/gitweb-lib.sh
>> @@ -114,4 +114,9 @@ perl -MCGI -MCGI::Util -MCGI::Carp -e 0 >/dev/null 2>&1 || {
>>       test_done
>>  }
>>
>> +perl -mTime::HiRes -e 0  >/dev/null 2>&1 || {
>> +     skip_all='skipping gitweb tests, Time::HiRes module unusable'
>
> Is "unusable" a good description, instead of "not found"?

Yeah it's odd, but I just copied the several lines above that use that phrasing.

Show 6 quoted lines

>> +     test_done
>> +}
>> +
>>  gitweb_init
>>
>