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

From: Brian Gernhardt <brian@gernhardtsoftware.com>
Date: Feb 6, 2010, 13:46 UTC
Message-ID: <0CD6B283-3181-4FAB-A6B2-13AFF9E5071C@gernhardtsoftware.com>
In-Reply-To: <m3eikya9tn.fsf@localhost.localdomain>

On Feb 6, 2010, at 6:22 AM, Jakub Narebski wrote:

Show 11 quoted lines

> 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.  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.  And if get_loadavg() is updated to use BSD::loadavg, the test should still be skipped if the module isn't installed.

Furthermore, tests should always be updated when a feature is changed.

> Let me explain how it currently works without /proc/loadavg.

I did check the code.

Show 5 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.

~~ Brian