Re: [PATCH] t9501: Skip testing load if we can't detect it

From: Jakub Narebski <jnareb@gmail.com>
Date: Feb 6, 2010, 14:05 UTC
Message-ID: <201002061505.13886.jnareb@gmail.com>
In-Reply-To: <0CD6B283-3181-4FAB-A6B2-13AFF9E5071C@gernhardtsoftware.com>

Brian Gernhardt wrote:

Show 15 quoted lines

> On Feb 6, 2010, at 6:22 AM, Jakub Narebski wrote:
>> Brian Gernhardt <brian@gernhardtsoftware.com> writes:
>> 
>>> Currently gitweb only knows how to check for load using /proc/loadavg,
>>> which isn't available on all systems.  We shouldn't fail the test just
>>> because we don't know how to check the system load.
>>> 
>>> Signed-off-by: Brian Gernhardt <brian@gernhardtsoftware.com>
>> 
>> NAK.  It is not necessary, and it would be hindrance (one more place
>> to update) if we are to extend get_loadavg() in gitweb to work without
>> /proc/loadavg, e.g. via BSD::loadavg module.
> 
> Without this patch, the test fails on my OS X machine (which doesn't
> have /proc).  So _something_ is necessary.

Right.

> Skipping the test because we can't use the feature on the host machine
> seemed more in line with what the other tests do with things like
> symlinks and file modes.

This is more test that gitweb returns correct HTTP response in 'load too high' situation, than testing of it finding average system load.

> And if get_loadavg() is updated to use BSD::loadavg, the test should
> still be skipped if the module isn't installed.

This test should succeed both when get_loadavg() returns true system load and when it can't read load average and it returns 0.

Show 20 quoted lines

>> Third, the test (as you can see below in context line in quoted diff
>> below) forces gitweb to go over maximum load by setting $maxload to 0.
>> This means that regardless of true load, and regardless whether gitweb
>> can detect system load (remember that if it cant get system load it
>> returns 0 instead) gitweb would be in "load too high" situation.
> 
> I did check the code.  Skipping the test seemed more in line with other tests.
> 
> sub get_loadavg {
>     if( -e '/proc/loadavg' ){}
>     return 0;
> }
> 
> if (defined $maxload && get_loadavg() > $maxload) {
> }
> 
> Setting $maxload to 0 does _not_ trigger failure because zero is not
> greater than 0.  Setting $maxload to -1 might work though.  I'll try
> it and test it in a little bit.  While I disagree that it's a good
> way to handle the situation, I will see if it works.

Oops.

Either gitweb should be modified to read

  if (defined $maxload && get_loadavg() >= $maxload) {

or the test should use

  $maxload = -1;

to artificially force 'load too high' situation.

-- 
Jakub Narebski
Poland