Subject: Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI

From: Jakub Narebski <[email-masked]>
Date: Mon, 30 Jun 2008 01:56:27 +0200

On Mon, 30 June 2008, Lea Wiemann wrote:
> Jakub Narebski wrote:
>> On Thu, 26 June 2008, Lea Wiemann wrote:
>>>
>>> - Follow redirects rather than failing.
>> 
>> Nice. Where it is?
> 
> test_page doesn't use $mech->get_ok anymore, but rather calls $mech->get
> and checks that the status is [23][0-9][0-9].  If it's 3xx, it also
> follows the redirect.

Thanks for the info.

By the way, some time ago I have send a patch (dropped, perhaps because
of it being feature freeze, or just lost) which converted support for
links with hash and without action (introduced in 7f9778b by Gerrit
Pape) to use redirect (like for 'object' action) instead of silently
filling correct action based on type of object (given by hash).  IMHO
it is better as it should prevent bookmarking "expensive" URL.

So this is useful, and could/would be even more useful.

>>> - Do not test correctness of line number fragments (#l[0-9]+); they're
>>>   broken too often right now.
>> 
>> What do you mean by broken?
> 
> This is only visible with --long-tests -- there are links to line
> numbers that don't exist (IOW the fragment doesn't exist in a name or id
> attribute on the target page).  I've even seen links to #l0.  Bug fixes
> welcome. ;-)

I wonder if those are intentional (or at least known) breaking, to form
approximate blame file history browsing; there was discussion about it
on git mailing list some time ago (it resulted in adding "parent" line
to blame porcelain/incremental output... or was it only in 'pu'?).
 
>> Good work!
> 
> Thanks! :-)  I'll send v9 in (hopefully) 2-3 days, together with the
> first working version of the caching code.

One issue of note, after brief peek at Git::Repo code: you should not
error out on unknown header in commit object, but either save its value
under its name, or just skip it.

Unless this has changed...

-- 
Jakub Narebski
Poland