Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output

[Jakub Narebski <jnareb@gmail.com>]

Lea Wiemann wrote:
> Jakub Narebski wrote:
>> Lea Wiemann wrote:
>>
>>> I think that gitweb should not be writing stuff to stderr unless an
>>> internal or serious error occurs
>>
>> There is no option to git-rev-list to not write any output to stderr
> 
> Okay, this one will go away with the new API I'm writing, which uses 
> cat-file --batch-check instead of rev-list.

Won't work.  Well, it would work for p=commit;h=non-existent, but it
would not work for p=log;h=non-existent, where git-rev-list
(or git-log) is really needed.  And I'd rather have parse_commit()
(used in 'commit' view) and parse_commits() (used in log-like views)
share the same commit parser, parse_commit_text(), so it's rev-list
also for git_commit, not cat-file.

But using git-cat-file --batch-check would improve other cases.

>                                             In the meantime (and in  
> other cases) I guess diverting stderr in the test code is fine.  (I 
> wouldn't want to ignore stderr in all cases, even where you're not 
> expecting any output on stderr, since that might actually indicate an 
> error.)

Well, I'd divert stderr in tests cases (or use simply 'test_external'),
or better filter stderr, only in those cases where there is spurious
but not dangerous thing on stderr.

But again, I think the solution would be either to add feature to
Git.pm, something like command_output_pipe_no_stderr, which would
redirect stderr to /dev/null, or modify git wrapper to redirect
stderr to /dev/null in scripts / when calling script, and redefine
die and warn for built-ins (or close stderr).

>> [snip]  It should work.  test-lib.sh sets up $PATH to have 'git' binary
>> (just compiled git binary) in it...
> 
> Since you're accessing http://localhost/ URLs, the web server's PATH is 
> in effect,...

It isn't.  http://localhost/ is just access convention, and
WWW::Mechanize::CGI actually calls system()[*1*] on provided path made
absolute (hence error when it contains whitespace), setting CGI
environmental variables before calling it.

[*1*] it would be nice to have perl_application in WWW::Mechanize::CGI,
which would simply setup %ENV and use do() instead of system() on
provided application.  Perhaps it would be better in meantime to
simply craft $mech->cgi( sub { ... } ), and do not use generic
$mech->cgi_application($path); we could do without all the checking,
too.

-- 
Jakub Narebski
Poland