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

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

[Jakub Narebski]

This test uses Perl module Test::WWW::Mechanize::CGI to check
gitweb output, using HTML::Lint (if present) to validate HTML.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Review, review, review!

This test requires test_external_without_stderr in t/test-lib.sh by
Lea Wiemann, send to git mailing list in
  [PATCH] t/test-lib.sh: add test_external and test_external_without_stderr
  http://thread.gmane.org/gmane.comp.version-control.git/83415
  Message-Id: <1212276975-27428-1-git-send-email-LeWiemann@gmail.com>
also present in git://repo.or.cz/git/gitweb-caching.git since commit
(currently) d28d31467db2ec5737948685ade281d5c0704a27.

This is very much work in progress; it uses Test::WWW::Mechanize::CGI 
Perl module (from CPAN), which makes writing tests checking form (but
not requiring exact details) of gitweb output.  The test driver, i.e.
t/t9503-gitweb-Mechanize.sh skips tests if they cannot be done; note
that it assumes that if one jas TWM::CGI module it also has all
modules it depends on (Test::WWW::Mechanize, HTTP::Request::AsCGI,...)
installed.

The alternatives would be to redo work of TWM::CGI using standard or
simply more common modules (LWP::* modules, HTTP::Request::AsCGI), or
(proposed during off-line discussion with Lea Wiemann) to create
expected output documents and diff (compare) literal output.  

The second solution has two disadvantages I can see.  First, it
freezes gitweb output format, making improvements more difficult.
Second, if there is change that invalidates some or all tests vectors,
you would have to regenerate those vectors from actual gitweb output
without having any tests to check this output; it would be quite easy
to put into test vectors errorneous output.

On the other hand current approach means much, much more tests.
It is also bit more challenging, because you have to find invariants
of the output and test that they are fullfilled.


NOTE: Currently test_external_without_stderr fails because when trying
to access URL for non-existent commit gitweb writes to STDERR; it is
not necessarily a bug because it is not written to web server logs (at
least in the case of Apache2).

NOTE!!!: RFC.  Work in progress.  YMMV.  Posted here to have Lea got
at least _some_ tests to check her GSoC work against, and have some
examples how such tests *might* be written (I have nearly next to no
experience in writing tests using Perl test infrastructure, and in
using Mechanize and Test::WWW::Mechanize::CGI).  I would really
appreciate comments from Perl experts here, or at least people with
more Perl experience than me.

 t/t9503-gitweb-Mechanize.sh |  127 +++++++++++++++++++++++++++++++++++++++++++
 t/t9503/test.pl             |   91 +++++++++++++++++++++++++++++++
 2 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..5df22c4
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+#
+
+test_description='gitweb as CGI (using WWW::Mechanize)
+
+This test uses Perl module Test::WWW::Mechanize::CGI to
+check gitweb output, using HTML::Lint to validate HTML.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+#!/usr/bin/perl
+
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external_without_stderr \
+	'test gitweb output' \
+	perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..7d081b9
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,91 @@
+#!/usr/bin/perl
+
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+eval { require HTML::Lint };
+my $lint_installed = !$@;
+diag('HTML::Lint is not installed; no HTML validation tests')
+	unless $lint_installed;
+
+my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.perl');
+# the followin two lines of code are workaround for bug in
+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3
+# (http://rt.cpan.org/Ticket/Display.html?id=36654)
+# for pathnames with spaces (because of "trash directory")
+$gitweb = File::Spec->rel2abs($gitweb);
+$gitweb = Cwd::abs_path($gitweb);
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi_application($gitweb);
+$mech->env(GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'});
+
+# import config, pedeclaring config variables
+our $site_name = '';
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+my $pagename = '';
+my $get_ok;
+SKIP: {
+	$pagename = 'project list (implicit)';
+	skip "Could not get $pagename", 2 + $lint_installed
+		unless $mech->get_ok('http://localhost/', "GET $pagename");
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->title_like(qr!$site_name!,
+		'title contains $site_name');
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+$mech->get_ok('http://localhost/?p=.git',
+	'GET test repository summary (implicit)');
+$mech->get_ok('http://localhost/.git',
+	'GET test repository summary (implicit, pathinfo)');
+$get_ok = 0;
+SKIP: {
+	$pagename = 'test repository summary (explicit)';
+	$get_ok = $mech->get_ok('http://localhost/?p=.git;a=summary',
+		"GET $pagename");
+	skip "Could not get $pagename", 1 + $lint_installed
+		unless $get_ok;
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->title_like(qr!$site_name.*\.git/summary!,
+		'title contains $site_name and ".git/summary"');
+}
+
+SKIP: {
+	skip "Could not get starting page $pagename", 2 + $lint_installed
+		unless $get_ok;
+	$pagename = 'search test repository (from search form)';
+	$get_ok = $mech->submit_form_ok(
+		{form_number=>1,
+		 fields=> {'s' => 'Initial commit'}
+		},
+		"submit search form (default)");
+	skip "Could not submit search form", 1 + $lint_installed
+		unless $get_ok;
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->content_contains('Initial commit',
+		'content contains searched text');
+}
+
+$pagename = 'non existent project';
+$mech->get('http://localhost/?p=non-existent.git');
+like($mech->status, qr/40[0-9]/, "40x status response for $pagename");
+$mech->html_lint_ok('page validates') if $lint_installed;
+
+$pagename = 'non existent commit';
+$mech->get('http://localhost/?p=.git;a=commit;h=non-existent');
+like($mech->status, qr/40[0-9]/, "40x status response for $pagename");
+$mech->html_lint_ok('page validates') if $lint_installed;
+
+1;
+__END__
-- 
1.5.5.3

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

[Lea Wiemann]

Jakub Narebski wrote:
> This test requires [PATCH] t/test-lib.sh: add test_external and test_external_without_stderr

For everyone's convenience, here's the raw patch: 
http://article.gmane.org/gmane.comp.version-control.git/83415/raw
(When repo.or.cz is up again you can also locate a version with fixed 
whitespace in my repo: <http://repo.or.cz/w/git/gitweb-caching.git>)

> This test uses Perl module Test::WWW::Mechanize::CGI to check
> gitweb output, using HTML::Lint (if present) to validate HTML.

Awesome, thank you so much for making a start here!  Here are some quick 
comments.

> NOTE: Currently test_external_without_stderr fails because when trying
> to access URL for non-existent commit gitweb writes to STDERR; it is
> not necessarily a bug because it is not written to web server logs

Without having looked at the cause of that, I think that gitweb should 
not be writing stuff to stderr unless an internal or serious error 
occurs; in particular trying to access invalid commits shouldn't cause 
messages on stderr, only to log files if at all.

That said, as long as it isn't fixed, here's my workaround to 
temporarily discard stderr (from my t/t9710/test.pl):

our $old_stderr;
sub discard_stderr {
         open our $old_stderr, ">&", STDERR or die "cannot save STDERR";
         close STDERR;
}
sub restore_stderr {
         open STDERR, ">&", $old_stderr or die "cannot restore STDERR";
}

It works on Unix, but I don't know about other platforms.

> +cat >gitweb_config.perl <<EOF
> [...]
> +our \$GIT = "git";

t9500 seems to be doing the same(?) thing, but this somehow doesn't work 
with your t9503 test:

$ git     # no git in PATH to make sure it picks up the right git binary
bash: git: command not found
$ ./t9500-gitweb-standalone-no-errors.sh | grep passed
* passed all 75 test(s)
$ ./t9503-gitweb-Mechanize.sh -v
[...]
	gitweb.perl: Can't exec "git": No such file or directory at 
/home/lea/source/git/fresh-git/gitweb/gitweb.perl line 380.

-- Lea

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

[Jakub Narebski]

Lea Wiemann wrote:
> Jakub Narebski wrote:
> >
> > NOTE: Currently test_external_without_stderr fails because when trying
> > to access URL for non-existent commit gitweb writes to STDERR; it is
> > not necessarily a bug because it is not written to web server logs
> 
> Without having looked at the cause of that, I think that gitweb should 
> not be writing stuff to stderr unless an internal or serious error 
> occurs; in particular trying to access invalid commits shouldn't cause 
> messages on stderr, only to log files if at all.

Actually it isn't gitweb (or Perl) writing to stderr, but git itself.
Somehow, at least for gitweb run as CGI script (and under legacy
mod_perl) with Apache 2 as web server this error message:
  fatal: bad revision 'non-existent'
doesn't land in web server logs (/var/log/httpd/error_log).  So it lands
in /dev/null when running gitweb as web script, so it was deemed not
important (also fixing this is not very easy, as you can read below).

t/t9500-gitweb-standalone-no-errors.sh considers as errors only those
error message which would make it into web server logs, see gitweb_run()
function there.  This catches compilation errors.


Fixing this is not that simple.  There is no option to git-rev-list
to not write any output to stderr (no, '--quiet' is about something
else), and I'd rather not lose all advantages of list (shell-less)
form of magical "|-" open only for "2>/dev/null" redirection as
in git_object subroutine in gitweb.

Perhaps it could be solved in Git.pm, and when gitweb is rewritten
to use "use Git" (and global $repo object instead of global $git_dir
variable) it would automatically fix it (using Git.pm would have
the advantage of making gitweb more portable, I think - to ActiveState
broken Perl implementation, with broken magic "|-" open).

Or git-rev-list, or even git wrapper itself, could acquire option to
redirect all stderr to dev null... I think adding it in git wrapper
would be even better; simply change "warning" and "die" to null
functions (I was even thinking about doing that...).

> That said, as long as it isn't fixed, here's my workaround to 
> temporarily discard stderr (from my t/t9710/test.pl):
> 
> our $old_stderr;
> sub discard_stderr {
>          open our $old_stderr, ">&", STDERR or die "cannot save STDERR";
>          close STDERR;
> }
> sub restore_stderr {
>          open STDERR, ">&", $old_stderr or die "cannot restore STDERR";
> }
> 
> It works on Unix, but I don't know about other platforms.

Thanks.

> > +cat >gitweb_config.perl <<EOF
> > [...]
> > +our \$GIT = "git";
> 
> t9500 seems to be doing the same(?) thing, but this somehow doesn't work 
> with your t9503 test:

It should work.  test-lib.sh sets up $PATH to have 'git' binary (just
compiled git binary) in it...

> $ git     # no git in PATH to make sure it picks up the right git binary
> bash: git: command not found
> $ ./t9500-gitweb-standalone-no-errors.sh | grep passed
> * passed all 75 test(s)
> $ ./t9503-gitweb-Mechanize.sh -v
> [...]
> 	gitweb.perl: Can't exec "git": No such file or directory at 
> /home/lea/source/git/fresh-git/gitweb/gitweb.perl line 380.

...and it would be very strange for t9500 to pass, but t9503 do not
pass.  (Of course both tests passes at my computer, otherwise
I wouldn't send this patch in current form).

Hmmm... perhaps $PATH doesn't get passed down... strange.


But thanks to your report I have found bug in gitweb.  I have changed
t/t9503-gitweb-Mechanize.sh... 

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
index 5df22c4..a5be275 100755
--- a/t/t9503-gitweb-Mechanize.sh
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -92,7 +92,7 @@ cat >gitweb_config.perl <<EOF
 # gitweb configuration for tests
 
 our \$version = "current";
-our \$GIT = "git";
+our \$GIT = "$safe_pwd/../../git";
 our \$projectroot = "$safe_pwd";
 our \$project_maxdepth = 8;
 our \$home_link_str = "projects";

...and found out that gitweb doesn't like when $GIT contains spaces
in _one_ place: finding git version.  It should be
 
 our $git_version = qx("$GIT" --version) =~ m/git version (.*)$/ ? $1 : "unknown";

with $GIT quoted.  (Patch will be send shortly).


So better solution would be

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
index 5df22c4..a5be275 100755
--- a/t/t9503-gitweb-Mechanize.sh
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -92,7 +92,7 @@ cat >gitweb_config.perl <<EOF
 # gitweb configuration for tests
 
 our \$version = "current";
-our \$GIT = "git";
+our \$GIT = "$GIT_EXEC_PATH/git";
 our \$projectroot = "$safe_pwd";
 our \$project_maxdepth = 8;
 our \$home_link_str = "projects";


Does it works for you?
-- 
Jakub Narebski
Poland

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

[Lea Wiemann]

Jakub Narebski wrote:
> +	skip "Could not get $pagename", 2 + $lint_installed
> +		unless $mech->get_ok('http://localhost/', "GET $pagename");

Just noticed this -- here's the cause of the problem I encountered in my 
previous post.

You can't really probe for a running gitweb installation on localhost, 
because even if there's one, you don't know if you're testing against 
your tree and your test repo.

So unless it's (easily) possible to force Mechanize to eat gitweb.cgi's 
output in lieu of making actual HTTP requests, I'd suggest that we 
simply use an environment variable (e.g. 
GITWEB_TEST_BASE_URL=http://localhost/cgi-bin/gitweb.cgi) -- if it's 
unset, the tests get skipped, otherwise they're run against that base URL.

*wanders-off-to-write-a-patch*

-- Lea

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

[Lea Wiemann]

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.  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.)

> [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, which doesn't get overridden by the tests.  But anyways, 
using an environment variable (see my other email) will move the 
responsibility for this to the developer running the tests.

> I have changed t/t9503-gitweb-Mechanize.sh... 
> 
> -our \$GIT = "git";
> +our \$GIT = "$safe_pwd/../../git";

For t9503 this'll go away if the GITWEB_TEST_BASE_URL thing is 
implemented.  For t9500 (which contains $GIT = "git" as well), it should 
  be fine as is since test-lib.sh sets PATH and thus gitweb picks up the 
right binary.

-- Lea

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

[Jakub Narebski]

Lea Wiemann wrote:
> Jakub Narebski wrote:
> >
> > +	skip "Could not get $pagename", 2 + $lint_installed
> > +		unless $mech->get_ok('http://localhost/', "GET $pagename");
> 
> Just noticed this -- here's the cause of the problem I encountered in my 
> previous post.
> 
> You can't really probe for a running gitweb installation on localhost, 
> because even if there's one, you don't know if you're testing against 
> your tree and your test repo.

Errr... WWW::Mechanize::CGI does not access web server, but runs CGI
script (or CGI subroutine) as if it was installed in the root directory
of localhost.  'http://localhost' simply means installed CGI script.

For example tests works for me, even though I have gitweb installed
somewhere deeper than directly at locahost:
  http://localhost/cgi-bin/gitweb/gitweb.cgi
Besides you can check output and see that it is consistent with set
and used gitweb config in t9503* test.

Test::WWW::Mechanize::CGI has the following description:
http://search.cpan.org/~mramberg/Test-WWW-Mechanize-CGI-0.1/lib/Test/WWW/Mechanize/CGI.pm

  Provides a convenient way of testing CGI applications without a external
  daemon.

The documentation of module (0.1 in the case of TWM::CGI, 0.3 in the
case of WM::CGI) leaves something to be desired, so I mainly used
examples provided as guideline.

-- 
Jakub Narebski
Poland

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

[Jakub Narebski]

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

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

[Lea Wiemann]

Jakub Narebski wrote:
> 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.

Since you can only run whole perl scripts with test_external, we'll have 
to resort to using those discard_/restore_stderr functions.  I doubt 
filtering for specific messages is worth the effort.

> 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,

(FYI, Git.pm has such a feature in the command method, though it it 
simply closes STDERR and doesn't properly re-open it, I believe.)  The 
Git::Repo API I'm writing doesn't currently generate any spurious output 
on stderr, but if it ever does I'll make sure it gets discarded.

>> Since you're accessing http://localhost/ URLs, the web server's PATH is 
>> in effect,...
> 
> It isn't.  http://localhost/ is just access convention,

Yup, I should've read TFM. :)  On my system, $PATH is empty in 
gitweb.cgi for some strange reason, but since using an absolute path for 
$GIT works, I won't track it down for now.

> [*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.

Gitweb and probably CGI::Carp qw(fatalsToBrowser) use 'exit', so we'd 
have to use Test::Trap (or so) to catch those.  I think we should defer 
this until performance actually becomes an issue.

-- Lea

[RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

From: Jakub Narebski <jnareb@gmail.com>

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint (if present) to validate the HTML.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
This requires the following two patches:
[PATCH] gitweb: Make it work with $GIT containing spaces
    http://article.gmane.org/gmane.comp.version-control.git/85036/raw
[PATCH] t/test-lib.sh: add test_external and test_external_without_stderr
    http://article.gmane.org/gmane.comp.version-control.git/83415/raw

Changed since v1:

- Use "$GIT_EXEC_PATH/git" instead of "git" for the git binary path,
  since on my system $PATH gets lost mysteriously.

- Use test_external instead of test_external_without_stderr so that
  the test doesn't fail if there is output on stderr.

  Discarding and restoring STDERR (as I mentioned in a recent message)
  doesn't work with Mechanize, as Mechanize somehow seems to save the
  STDERR handle and reuse it, so subsequent calls to Mechanize keep
  using the discarded STDERR.  (The discard/restore functions work
  fine in other test modules, it's really just Mechanize here.)

  Using test_external means that in verbose mode we get some spurious
  error messages in the test output, but they're not harmful (and it
  should be clear to anyone running the test suite that they're OK).

- Reworded commit message a little for clarity.

The test works now on my system.  If you run this successfully (or
unsuccessfully), please post a note on the list.
Comments/patches/additions, in particular from people who've used
Mechanize before, is still much appreciated!

This patch is WIP and not for inclusion.  At some point we'll take
further revisions off the list and use a public repository instead
(once repo.or.cz is up again); we'll post a notice when that happens.
When the test suite is finished we'll post it here again.

 t/t9503-gitweb-Mechanize.sh |  127 +++++++++++++++++++++++++++++++++++++++++++
 t/t9503/test.pl             |   91 +++++++++++++++++++++++++++++++
 2 files changed, 218 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..8720aea
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,127 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+#
+
+test_description='gitweb as CGI (using WWW::Mechanize)
+
+This test uses Perl module Test::WWW::Mechanize::CGI to
+check gitweb output, using HTML::Lint to validate HTML.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+#!/usr/bin/perl
+
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..7d081b9
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,91 @@
+#!/usr/bin/perl
+
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+eval { require HTML::Lint };
+my $lint_installed = !$@;
+diag('HTML::Lint is not installed; no HTML validation tests')
+	unless $lint_installed;
+
+my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.perl');
+# the followin two lines of code are workaround for bug in
+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3
+# (http://rt.cpan.org/Ticket/Display.html?id=36654)
+# for pathnames with spaces (because of "trash directory")
+$gitweb = File::Spec->rel2abs($gitweb);
+$gitweb = Cwd::abs_path($gitweb);
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi_application($gitweb);
+$mech->env(GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'});
+
+# import config, pedeclaring config variables
+our $site_name = '';
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+my $pagename = '';
+my $get_ok;
+SKIP: {
+	$pagename = 'project list (implicit)';
+	skip "Could not get $pagename", 2 + $lint_installed
+		unless $mech->get_ok('http://localhost/', "GET $pagename");
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->title_like(qr!$site_name!,
+		'title contains $site_name');
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+$mech->get_ok('http://localhost/?p=.git',
+	'GET test repository summary (implicit)');
+$mech->get_ok('http://localhost/.git',
+	'GET test repository summary (implicit, pathinfo)');
+$get_ok = 0;
+SKIP: {
+	$pagename = 'test repository summary (explicit)';
+	$get_ok = $mech->get_ok('http://localhost/?p=.git;a=summary',
+		"GET $pagename");
+	skip "Could not get $pagename", 1 + $lint_installed
+		unless $get_ok;
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->title_like(qr!$site_name.*\.git/summary!,
+		'title contains $site_name and ".git/summary"');
+}
+
+SKIP: {
+	skip "Could not get starting page $pagename", 2 + $lint_installed
+		unless $get_ok;
+	$pagename = 'search test repository (from search form)';
+	$get_ok = $mech->submit_form_ok(
+		{form_number=>1,
+		 fields=> {'s' => 'Initial commit'}
+		},
+		"submit search form (default)");
+	skip "Could not submit search form", 1 + $lint_installed
+		unless $get_ok;
+	$mech->html_lint_ok('page validates') if $lint_installed;
+	$mech->content_contains('Initial commit',
+		'content contains searched text');
+}
+
+$pagename = 'non existent project';
+$mech->get('http://localhost/?p=non-existent.git');
+like($mech->status, qr/40[0-9]/, "40x status response for $pagename");
+$mech->html_lint_ok('page validates') if $lint_installed;
+
+$pagename = 'non existent commit';
+$mech->get('http://localhost/?p=.git;a=commit;h=non-existent');
+like($mech->status, qr/40[0-9]/, "40x status response for $pagename");
+$mech->html_lint_ok('page validates') if $lint_installed;
+
+1;
+__END__
-- 
1.5.6.rc2.51.g4d03a.dirty

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

[Jakub Narebski]

Lea Wiemann wrote:
> Jakub Narebski wrote:
>>
>> [*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.
> 
> Gitweb and probably CGI::Carp qw(fatalsToBrowser) use 'exit', so we'd 
> have to use Test::Trap (or so) to catch those.  I think we should defer 
> this until performance actually becomes an issue.

The idea was more to avoid bug in cgi_application() method of WM::CGI,
which fails on pathname containing embedded spaces (that is why
workaround in setting $gitweb path, to avoid 'trash directory' in it),
than for performance reasons.

Nevertheless naive $mech->cgi( sub { do "$gitweb"; } ); doesn't work;
and after thinking about it a little it simply couldn't work...

-- 
Jakub Narebski
Poland

Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Jakub Narebski]

Lea Wiemann wrote:

> Changed since v1:

> - Use test_external instead of test_external_without_stderr so that
>   the test doesn't fail if there is output on stderr.
> 
>   Discarding and restoring STDERR (proposed in a recent message)
>   doesn't work with Mechanize, as Mechanize somehow seems to save the
>   STDERR handle and reuse it, so subsequent calls to Mechanize keep
>   using the discarded STDERR.  (The discard/restore functions work
>   fine in other test modules, it's really just Mechanize here.)

Actually I think it is CGI module itself catching and redirecting
STDERR from Perl to logfile, and WWW::Mechanize::CGI having to catch
and redirect all stderr of invoked application, but I'm not sure.


What should we think about now, I guess, is
1.) Should we put all tests in one file, or should they be split
    according to what do they test?  For example separate tests
    for correct 4xx and 5xx responses for requests for non-exiting
    objects, and for not permitted views.

2.) What invariants should we test, beside obvious HTML validation
    using HTML::Lint (by the way, is there some Perl module for RSS,
    Atom and OPML validation?).  Checking for example if all items
    are listed in a 'tree' view, or if all inner links (#link) are
    valid would be a good start... pity that Mechanize modules don't
    have very good documetation.

3.) What invariants you want to test for your caching efforts, e.g.
    checking (not necessary with TWM::CGI) if cached output matches
    non-cached, with exception of marking output as cached?

-- 
Jakub Narebski
Poland

Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

Jakub Narebski wrote:
> 1.) Should we put all tests in one file, or should they be split

I'd suggest we leave it in a single file until test execution time 
becomes an issue.  Then (when it has become too large) we'll be able to 
figure out good boundaries along which to split the test suite.

> 2.) What invariants should we test [...]  Checking for example if all items
>     are listed in a 'tree' view, or if all inner links (#link) are
>     valid would be a good start... 

Yup; completeness of item lists is especially relevant for paginated 
output.  Also check for the presence and validity of links (like 
"parent" links, etc.), and for the presence of certain elements (like 
the file modes in the tree view).

Also, with a $ENV{LONG_GIT_TEST} variable or so, we could automatically 
validate all links for each page we're checking -- it takes a long time, 
but it's still way more efficient than exhaustive spidering of the whole 
site.

> (by the way, is there some Perl module for RSS, Atom and OPML validation?)

I can't find anything on Google right now, but piping them into external 
validators might be just as fine.  Also, since those formats are 
generated using print statements (which is really error-prone for XML 
formats), I'd say that a good start would be to check for XML validity.

> 3.) What invariants you want to test for your caching efforts, e.g.
>     checking if cached output matches non-cached

How about this:

1. Run the Mechanize tests (and possibly also the existing t9500 tests) 
*without* caching, recording the URL's and contents of all pages the 
test suite accesses.

2. Get all those URL's again *with* caching (from a cold cache), and 
assert that the output is identical.

3. Get all those URL's again *with* caching (from a warm cache), and 
assert that the output is identical.  Perhaps also assert that no call 
to the git binary is made (i.e. everything has actually been cached). 
(Of course we might need options for the production site to not cache 
certain things, but let's defer this discussion.)

-- Lea

Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Jakub Narebski]

Lea Wiemann wrote:
> Jakub Narebski wrote:
> >
> > 1.) Should we put all tests in one file, or should they be split
> 
> I'd suggest we leave it in a single file until test execution time 
> becomes an issue.  Then (when it has become too large) we'll be able to 
> figure out good boundaries along which to split the test suite.

I wanted to split tests mainly not because of performance, but because
of making it easier to maintain.  Although perhaps single driver-test,
and do()-ing or require()-ing sub-files would be enough.

> > 2.) What invariants should we test [...]  Checking for example if all items
> >     are listed in a 'tree' view, or if all inner links (#link) are
> >     valid would be a good start... 
> 
> Yup; completeness of item lists is especially relevant for paginated 
> output.  Also check for the presence and validity of links (like 
> "parent" links, etc.), and for the presence of certain elements (like 
> the file modes in the tree view).

For example if "next" (and like) views really lead to next page.

> Also, with a $ENV{LONG_GIT_TEST} variable or so, we could automatically 
> validate all links for each page we're checking -- it takes a long time, 
> but it's still way more efficient than exhaustive spidering of the whole 
> site.

Good idea.  I would examine how it is done in other tests.

> > (by the way, is there some Perl module for RSS, Atom and OPML validation?)
> 
> I can't find anything on Google right now, 

I usually search CPAN first, not Google...

> but piping them into external  
> validators might be just as fine.  Also, since those formats are 
> generated using print statements (which is really error-prone for XML 
> formats), I'd say that a good start would be to check for XML validity.

We can use Test::XML / Test::XML::Valid / Test::XML::Simple for being
well-formed XML.  If RSS / Atom / OPML have good DTD / XML Schema /
/ Relax-NG schema / Sablotron rules, they could be checked using that
from Perl.

> > 3.) What invariants you want to test for your caching efforts, e.g.
> >     checking if cached output matches non-cached
> 
> How about this:
> 
> 1. Run the Mechanize tests (and possibly also the existing t9500 tests) 
> *without* caching, recording the URL's and contents of all pages the 
> test suite accesses.
> 
> 2. Get all those URL's again *with* caching (from a cold cache), and 
> assert that the output is identical.

How would you ensure cold cache?

> 3. Get all those URL's again *with* caching (from a warm cache), and 
> assert that the output is identical.

Well, it might be identical, but it also might have "cached output"
somewhere in the output.

> Perhaps also assert that no call  
> to the git binary is made (i.e. everything has actually been cached). 
> (Of course we might need options for the production site to not cache 
> certain things, but let's defer this discussion.)

Or at least (if we don't cache everything, and that could be good idea)
to check if there are less git binary calls.

-- 
Jakub Narebski
Poland

Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

Jakub Narebski wrote:
> How would you ensure cold cache?

Use a unique key prefix, like "gw:test-$PID:<key>"

>> 3. Get all those URL's again *with* caching (from a warm cache), and 
>> assert that the output is identical.
> 
> it might have "cached output" somewhere in the output.

We can make that deactivatable so that the output is actually identical.

-- Lea

Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Jakub Narebski]

Lea Wiemann wrote:
> Jakub Narebski wrote:
>> Lea Wiemann wrote:
>>
>> How would you ensure cold cache?
> 
> Use a unique key prefix, like "gw:test-$PID:<key>"

I was thinking rather about dropping cache somewhat.
 
>>> 3. Get all those URL's again *with* caching (from a warm cache), and 
>>> assert that the output is identical.
>> 
>> it might have "cached output" somewhere in the output.
> 
> We can make that deactivatable so that the output is actually identical.

Good idea.  Or we can just do diff and check the difference.

BTW. another thing worth considering: generating "Generating page..."
or something like that like kernel.org's gitweb does when it has to
regenerate cache.  Nice feature, that is.

-- 
Jakub Narebski
Poland

[WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

From: Jakub Narebski <jnareb@gmail.com>

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint (if present) to validate the HTML.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
I haven't gotten around to merging Jakub's recent XML validation patch
yet, but I think it'd be good to have some review now; I'll merge it
tomorrow and send another patch.

Changes since v2:

t9503-gitweb-Mechanize.sh has stayed the same, but t9503/test.pl has
been overhauled: extracted common code into helper functions, only
validate if --long-tests if given, added link-checking with
--long-tests, added some minor tests for page contents, and added
tests for the summary page at the bottom.

This runs on top of the *next* branch plus the following patches:

[PATCH] gitweb: respect $GITPERLLIB
    http://article.gmane.org/gmane.comp.version-control.git/85586/raw
[PATCH 1/2 v3] t/test-lib.sh: add test_external and test_external_without_stderr
    http://article.gmane.org/gmane.comp.version-control.git/85504/raw
[PATCH v3] gitweb: standarize HTTP status codes
    http://article.gmane.org/gmane.comp.version-control.git/85522/raw

(The latter two patches might be in next already by the time you're
reading this.)


 t/t9503-gitweb-Mechanize.sh |  128 +++++++++++++++++++++++++++++++++++++++++++
 t/t9503/test.pl             |  128 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 256 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..abcf987
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+test_description='gitweb as CGI (using WWW::Mechanize)
+
+This test uses Perl module Test::WWW::Mechanize::CGI to
+check gitweb output, using HTML::Lint to validate HTML.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+#!/usr/bin/perl
+
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..3d72575
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,128 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+my $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require HTML::Lint };
+my $use_lint = !$@;
+diag('HTML::Lint is not installed; no HTML validation tests')
+    unless $use_lint;
+
+my @revisions = map { substr $_, 0, 10 } split /\s/, `git-rev-list --first-parent HEAD`;
+my $head = $revisions[-1];
+
+my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.cgi');
+# the followin two lines of code are workaround for bug in
+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3
+# (http://rt.cpan.org/Ticket/Display.html?id=36654)
+# for pathnames with spaces (because of "trash directory")
+$gitweb = File::Spec->rel2abs($gitweb);
+$gitweb = Cwd::abs_path($gitweb);
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi_application($gitweb);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV, GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'}, $mech->env);
+
+# import config, predeclaring config variables
+our $site_name = '';
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+our %checked_pages;
+# Validate and spider the current page, if --long-tests (-l) is given.
+sub check_page {
+	my $uri = $mech->uri;
+	if (not $checked_pages{$uri}) {
+		$mech->html_lint_ok('validate') or return 0 if $long_tests && $use_lint;
+		$mech->page_links_ok("check links on $uri") if $long_tests;
+		$checked_pages{$uri} = 1;
+	}
+	return 1
+}
+
+our $baseurl = "http://localhost";
+our($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page '?p=.git;a=summary', 'repository summary') {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened (and if --long-tests is given that
+# it validates and has valid links), and return true if it does.  Also
+# set the global variables $params, $pagedesc, and $url for use in the
+# if block.  Optionally pass a third parameter $status to test the
+# HTTP status code of the page (useful for error pages).
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	$url = "$baseurl$params";
+	if($status) {
+		$mech->get($url);
+	} else {
+		$mech->get_ok($url, "GET \"$params\" -- $pagedesc") or return 0;
+	}
+	check_page or return 0;
+	if($status) {
+		return is($mech->status, $status, "GET \"$params\" -- $pagedesc -- yields $status");
+	} else {
+		return 1;
+	}
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+			     fields => { 's' => 'Initial' }
+			   }, "submit search form (default)")) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+# Summary page.
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[-3..-1]) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		ok( $mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision" );
+	}
+}
+
+
+1;
+__END__
-- 
1.5.6.32.gad63a.dirty

Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Jakub Narebski]

On Fri, 20 Jun 2008, Lea Wiemann wrote:

> From: Jakub Narebski <jnareb@gmail.com>

If you have rewritten the patch I won't mind if you change authorship
to you (remove this From: line); signoff is IMHO enough.

> ---
> I haven't gotten around to merging Jakub's recent XML validation patch
> yet, but I think it'd be good to have some review now; I'll merge it
> tomorrow and send another patch.

First, it is not XML validation[*1*], but check if XML is well-formed.
Second, I think it would be better if adding XML checks (RSS, Atom,
OPML) would be left as separate commit.
 
[*1*] There is W3C feed validation service, http://validator.w3.org/feed/
which provides also standalone Python script for feed validation, and
also SOAP interface with WebService::Validator::Feed::W3C wrapper (but
it would require having Internet connection), or we can try validate
against XML Schema Definition (there exists one for RSS, and there is
one for Atom derived from Relax-NG schema).

> diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
[...]
> +# set up test repository
> +test_expect_success 'set up test repository' '
[cut]

I have created this part as a copy of older t9500 gitweb test, thinking
about what we might want to test, but the WIP of Mechanize based t9503
doesn't have yet tests for those specific features.

> diff --git a/t/t9503/test.pl b/t/t9503/test.pl
[...]
> +# We don't count properly when skipping, so no_plan is necessary.
> +use Test::More qw(no_plan);

Actually it is not that we cannot could properly when skipping, because
there are two ways to have skipped tests and test count upfront, both
implemented in my initial version (v1) of t/t9503/test.pl.

You can set number of tests upfront depending if some feature is
available, e.g. "plan tests => <n> - $use_lint*<m2> - $check_xml*<m2>"
or just "if ($use_lint) { plan tests => <m> } else ...".

Or you can explicitly skip tests using SKIP: BLOCK (see Test::More(3pm)).


What we have here is that we don't know number of tests upfront,
because it is complicated, and that's what I'd like to see in the
comment instead (but I don't have idea for exact wording).

> +my @revisions = map { substr $_, 0, 10 } split /\s/, `git-rev-list --first-parent HEAD`;

Why do you use shortened SHA1 identifier, instead of full SHA1? Links
in gitweb use full SHA1.

> +my $head = $revisions[-1];

Errr, HEAD would be $revisions[0], $revisions[-1] would be $root.

> +my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.cgi');
> +# the followin two lines of code are workaround for bug in
> +# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3
> +# (http://rt.cpan.org/Ticket/Display.html?id=36654)
> +# for pathnames with spaces (because of "trash directory")
> +$gitweb = File::Spec->rel2abs($gitweb);
> +$gitweb = Cwd::abs_path($gitweb);
> +
> +my $mech = new Test::WWW::Mechanize::CGI;
> +$mech->cgi_application($gitweb);

Another solution would be to copy relevant parts of cgi_application
(without all the checks for example), and use $mech->cgi( sub { ... } );
here (without the cgi_application bug).

> +our $baseurl = "http://localhost";
> +our($params, $url, $pagedesc, $status);

I think we can use 'my' here; gitweb uses 'our' only to be able to run
it correctly as "legacy CGI" mod_perl script.

> +our($params, $url, $pagedesc, $status);

Style: I would write "our (", with space between keyword and opening
parenthesis.

> +# test_page ( <params>, <page_description>, <expected_status> )
> +# Example:
> +# if (test_page '?p=.git;a=summary', 'repository summary') {
> +#     $mech->...;
> +#     $mech->...;
> +# }

Style: I would use function call form, i.e. "test_page(...)", not
command-like (or script-like) form.


As to the rest of the test: I think as preliminary test it is a good
thing to have.  We can think about what tests to add later (unless you
rather have exhaustive test suite now...).

-- 
Jakub Narebski
Poland

Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

Jakub Narebski wrote:
> First, it is not XML validation[*1*], but check if XML is well-formed.
> Second, I think it would be better if adding XML checks (RSS, Atom,
> OPML) would be left as separate commit.

Sure; FWIW I'm generally in favor of having a large initial commit for 
new independent files (and since we don't need to worry about conflicts 
I'm not fervent about getting this into the next branch ASAP), but we 
can definitely add XML checking separately.

>> +# set up test repository
> 
> I have created this part as a copy of older t9500 gitweb test, thinking
> about what we might want to test, but the WIP of Mechanize based t9503
> doesn't have yet tests for those specific features.

I was thinking about that.  Right now the tests are so generic that you 
can replace the test repository with anything else as long as it has 
some commits (and later some tags, etc.).  That's kinda nice.

>> +# We don't count properly when skipping, so no_plan is necessary.
>> +use Test::More qw(no_plan);
> 
> Actually it is not that we cannot could properly when skipping, because
> there are two ways to have skipped tests and test count upfront,

We're skipping tests if a page-load fails to prevent a slew of failure, 
using constructs like if(test_page '?...') { ... inspect the page ...}. 
It's my impression that we shouldn't end up with a wrong test count even 
if one test fails; but then we'd have to replace those ifs with 
cumbersome skip blocks.

I'm generally not in favor of maintaining any test count plans; they're 
an unnecessary failure source, and I don't think they buy us much, if 
anything -- correct me if I'm missing something Perl-specific here.

> Why do you use shortened SHA1 identifier, instead of full SHA1?
> Links in gitweb use full SHA1.

That's for readability in the test output; links get checked anyway, 
don't they?  If you think we should be testing against full SHA1s, 
that's fine too.

> Errr, HEAD would be $revisions[0], $revisions[-1] would be $root.

Fixed, thanks.

> Another solution would be to copy relevant parts of cgi_application
> (without all the checks for example), and use $mech->cgi( sub { ... } );
> here (without the cgi_application bug).

ISTR that using cgi(sub ...) gives us problems with untrappable exits in 
gitweb.cgi (and possibly more things), right?  I'm fine with the 
workarounds we have in place, they don't seem brittle.

>> +our $baseurl = "http://localhost";
>> +our($params, $url, $pagedesc, $status);
> 
> I think we can use 'my' here;

They're used in subroutines, so I believe 'our' is correct here.

> Style: I would write "our (", with space [...]

>> +# if (test_page '?p=.git;a=summary', 'repository summary') {
> 
> Style: I would use function call form, i.e. "test_page(...)", not
> command-like (or script-like) form.

Both fixed, thanks.

> As to the rest of the test: I think as preliminary test it is a good
> thing to have.  We can think about what tests to add later (unless you
> rather have exhaustive test suite now...).

I'll be writing tests as I go and change parts of gitweb.  I won't be 
able to write exhaustive tests, but I at least want to make sure that 
the code I'm changing is covered somehow.

-- Lea

Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Jakub Narebski]

On Fri, 20 Jun 2008, Lea Wiemann wrote:
> Jakub Narebski wrote:
>>
>> Second, I think it would be better if adding XML checks (RSS, Atom,
>> OPML) would be left as separate commit.
> 
> Sure; FWIW I'm generally in favor of having a large initial commit for 
> new independent files [...]

I just think that having this separate could help bisectability in
the case of errors.

>>> +# set up test repository
>> 
>> I have created this part as a copy of older t9500 gitweb test, thinking
>> about what we might want to test, but the WIP of Mechanize based t9503
>> doesn't have yet tests for those specific features.
> 
> I was thinking about that.  Right now the tests are so generic that you 
> can replace the test repository with anything else as long as it has 
> some commits (and later some tags, etc.).  That's kinda nice.

Well, I've tried to put the cases where something can go wrong, and to
cover wide range of possibilities.  Rename, copy, typechange, merge
commit for testing *diff views, symlink to test 'tree' view, different
types of tags and tagged objects...

What could be added is different types of stage output: filenames with
'*', '+', '=', ':', '?', whitespace, etc.  Checking if submodules
doesn't trip gitweb would be good idea too.
 
> I'm generally not in favor of maintaining any test count plans; they're 
> an unnecessary failure source, and I don't think they buy us much, if 
> anything -- correct me if I'm missing something Perl-specific here.

While they can detect otherwise unnoticed errors, I think most of time
they are error in test counting... so I can agree with that.
 
>> Why do you use shortened SHA1 identifier, instead of full SHA1?
>> Links in gitweb use full SHA1.
> 
> That's for readability in the test output; links get checked anyway, 
> don't they?  If you think we should be testing against full SHA1s, 
> that's fine too.

This would reduce number of operations when crawling gitweb output.

 
[about workaround bug in TWM::CGI when path to application contains
 embedded space].

> ISTR that using cgi(sub ...) gives us problems with untrappable exits in 
> gitweb.cgi (and possibly more things), right?

Actually ->cgi_application(<path>) is implemented using ->cgi(<sub>)
in TWM::CGI.  The bug is that it uses straight "system($application)",
without any quoting, after ensuring that $application is absolute path
(so quoting it beforehand won't work).

> I'm fine with the workarounds we have in place, they don't seem brittle.

They work around the fact that we use 'trash directory', but they
would fail if you run test from the directory which contains spaces,
for example "/home/Lea Wiemann/git/t" (I think this has greater
probability happening on operating systems other than Linux, for
example MS Windows with "My Documents" or "Program Files").

>>> +our $baseurl = "http://localhost";
>>> +our($params, $url, $pagedesc, $status);
>> 
>> I think we can use 'my' here;
> 
> They're used in subroutines, so I believe 'our' is correct here.

Actually 'my' would work here too; the problem with gitweb being
forced to use 'our' to make it work with mod_perl isn't about the
fact that they are global variables, but with initializing them,
as mod_perl wraps whole gitweb in a block (processing requests).
 
>> As to the rest of the test: I think as preliminary test it is a good
>> thing to have.  We can think about what tests to add later (unless you
>> rather have exhaustive test suite now...).
> 
> I'll be writing tests as I go and change parts of gitweb.  I won't be 
> able to write exhaustive tests, but I at least want to make sure that 
> the code I'm changing is covered somehow.

Write test when we notice something breaking seems a common theme
in git development... ;-)

-- 
Jakub Narebski
Poland

Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

Jakub Narebski wrote:
> Lea Wiemann wrote:
>> Sure; FWIW I'm generally in favor of having a large initial commit for 
>> new independent files [...]
> 
> I just think that having this separate could help bisectability in
> the case of errors.

Hm...  I wouldn't think that bisect could be necessary for a long linear 
(sequential) test script, where the errors are clearly locatable.

> What could be added is different types of stage output: filenames with
> '*', '+', '=', ':', '?', whitespace, etc.  Checking if submodules
> doesn't trip gitweb would be good idea too.

Yup, filenames with ampersands and semicolons would be fun, too. 
(Submodules seem to work, but that should be covered as well of course.)

> [Full SHA1s] would reduce number of operations when crawling gitweb output.

Right; changed.

> Actually ->cgi_application(<path>) is implemented using ->cgi(<sub>)
> in TWM::CGI.  The bug is that it uses straight "system($application)",
> 
> [it] would fail if you run test from the directory which contains spaces,

OK, now I see what you mean.  Well, awesome, borked lirbareis.  I've put 
this as a TODO into the commit message; will take care of it later.

>>>> +our $baseurl = "http://localhost";
>>>> +our($params, $url, $pagedesc, $status);
>>> I think we can use 'my' here;

Fixed, and thanks for the offline explanations.

-- Lea

[WIP/PATCH v4] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint (if present) to validate the HTML.

TOOD: Make this runnable even when the path to the git tree (i.e. your
working copy) contains blanks.
http://mid.gmane.org/200806202003.55919.jnareb@gmail.com

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
This for everyone's reference so you have my most recent version to
base changes on.

Changed since v3: applied Jakub's suggestions, and added TODO note to
commit message so this doesn't get applied accidentally.

 t/t9503-gitweb-Mechanize.sh |  128 +++++++++++++++++++++++++++++++++++++++++++
 t/t9503/test.pl             |  128 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 256 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..abcf987
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,128 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+test_description='gitweb as CGI (using WWW::Mechanize)
+
+This test uses Perl module Test::WWW::Mechanize::CGI to
+check gitweb output, using HTML::Lint to validate HTML.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+#!/usr/bin/perl
+
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..7e7c98c
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,128 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+my $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require HTML::Lint };
+my $use_lint = !$@;
+diag('HTML::Lint is not installed; no HTML validation tests')
+    unless $use_lint;
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+my $head = $revisions[0];
+
+my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.cgi');
+# The following two lines of code are a workaround for a bug in
+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3
+# (http://rt.cpan.org/Ticket/Display.html?id=36654) for pathnames with
+# spaces (because of "trash directory")
+$gitweb = File::Spec->rel2abs($gitweb);
+$gitweb = Cwd::abs_path($gitweb);
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi_application($gitweb);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV, GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'}, $mech->env);
+
+# import config, predeclaring config variables
+my $site_name = '';
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+my %checked_pages;
+# Validate and spider the current page, if --long-tests (-l) is given.
+sub check_page {
+	my $uri = $mech->uri;
+	if (not $checked_pages{$uri}) {
+		$mech->html_lint_ok('validate') or return 0 if $long_tests && $use_lint;
+		$mech->page_links_ok("check links on $uri") if $long_tests;
+		$checked_pages{$uri} = 1;
+	}
+	return 1
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened (and if --long-tests is given that
+# it validates and has valid links), and return true if it does.  Also
+# set the global variables $params, $pagedesc, and $url for use in the
+# if block.  Optionally pass a third parameter $status to test the
+# HTTP status code of the page (useful for error pages).
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	$url = "$baseurl$params";
+	if($status) {
+		$mech->get($url);
+	} else {
+		$mech->get_ok($url, "GET \"$params\" -- $pagedesc") or return 0;
+	}
+	check_page or return 0;
+	if($status) {
+		return is($mech->status, $status, "GET \"$params\" -- $pagedesc -- yields $status");
+	} else {
+		return 1;
+	}
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+			     fields => { 's' => 'Initial' }
+			   }, "submit search form (default)")) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+# Summary page.
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+	}
+}
+
+
+1;
+__END__
-- 
1.5.6.80.g3141.dirty

[PATCH v5] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.

Also add a GITPERL environment variable that allows running Perl-based
tests with different perl binaries (and thus under different
versions).

Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Windows users: Please apply this patch and run the test, I'd love to
know if it works on Windows!

This patch applies against the *next* branch (without any other
patches, finally).

Note that in order to actually run the test you need the
Test::WWW::Mechanize::CGI Perl module.

Changes since v4:

- Add a $GITPERL variable so you can choose the perl binary and run
  the test suite with different Perl versions.  Note that this affects
  test-lib.sh.

- Add a workaroud for WWW::Mechanize::CGI so that the test suite also
  works if the path to the Git tree contains blanks (which could
  happen on Windows, for instance).

- Integrated all of Jakub's "[RFC/PATCH (WIP)] gitweb: Check that RSS,
  Atom and OPML output is well formed XML", which uses XML::Parser.

- Also added snapshot validation using Archive::Tar.

- Spidering (with --long-tests) is now much more efficient, in that it
  doesn't check any page more than once.

- Added checks to ensure that fragments have corresponding name/id
  attributes in the page.

- Added plenty more actual tests: check more elements on the summary
  page, and also test the commit, commitdiff, tree, blame, history,
  blob, and blob_plain views.  Some of these tests don't do anything
  more than downloading a few pages with the given action (without
  checking any of the content), but this already helps a lot since it
  checks that gitweb doesn't die, that the HTML is valid, and (if
  --long-tests is given) that all links work.

I've tested the test suite with Perl 5.8, Perl 5.10, and with and
without the optional modules (HTML::Lint, XML::Parser, Archive::Tar).

I'd like some feedback on this (especially from Jakub) -- if other
people OK the patch, I think it's ready for inclusion.

Apologies for any tyops and last-minute glitches, I'm getting kinda
tired... :)

-- Lea

 t/t9503-gitweb-Mechanize.sh |  132 ++++++++++++++++
 t/t9503/test.pl             |  354 +++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh               |    2 +
 3 files changed, 488 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..3fe6d8b
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$GITPERL" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	"$GITPERL" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..28894c5
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,354 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+our $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+    or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+    or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+    or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+# files and directories in HEAD root:
+chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);
+
+sub rev_parse {
+	my $name = shift;
+	chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+	$hash or undef;
+}
+
+sub get_type {
+	my $name = shift;
+	chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+	$type or undef;
+}
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# Thus subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+my $cgi = sub {
+	# Use exec, not the shell, to support blanks in the path.
+	my $status = system $gitweb $gitweb;
+	my $value  = $status >> 8;
+
+	croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+	    if ( $status == -1 );
+	croak( qq/Application '$gitweb' exited with value: $value/ )
+	    if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+	   GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+	   SCRIPT_FILENAME => $gitweb,
+	   $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+	my ($uri, $fragment) = split '#', $mech->uri;
+	if (!$verified_uris{$uri}) {
+		$verified_uris{$uri} = 1;
+
+		# Internal errors yield 200, but cause gitweb.cgi to
+		# exit with non-zero exit code, which Mechanize::CGI
+		# translates to 500, so we don't really need to check
+		# for "Software error" here, provided that the test
+		# cases always check the status code.
+		#$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+		# Validating is fast, so we can do it even without
+		# $long_tests.
+		$mech->html_lint_ok('[auto] validate HTML') or return 0
+		    if $html_lint_installed && $mech->is_html;
+		my $content_type = $mech->response->header('Content-Type')
+		    or die "$uri does not have a Content-Type header";
+		if ($xml_parser_installed && $content_type =~ /xml/) {
+			eval { XML::Parser->new->parse($mech->content); };
+			ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+		}
+		if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+			my $snapshot_file = File::Temp->new;
+			print $snapshot_file $mech->content;
+			close $snapshot_file;
+			my $t = Archive::Tar->new;
+			$t->read($snapshot_file->filename, 1);
+			ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+		}
+		# WebService::Validator::Feed::W3C would be nice to
+		# use, but it doesn't support direct input (as opposed
+		# to URIs) long enough for our feeds.
+	}
+	$mech->content_like(qr/(name|id)="$fragment"/,
+			    "[auto] fragment #$fragment exists ($uri)")
+	    if $fragment;
+	return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given.  Do not check the status code of the current page.
+my %spidered_uris;  # pages whose links have been checked
+my %status_checked_uris;  # verified pages whose status is known to be 2xx
+sub check_page {
+	_verify_page or return 0;
+	my $orig_url = $mech->uri;
+	if ($long_tests && !$spidered_uris{$mech->uri} ) {
+		$spidered_uris{$mech->uri} = 1;
+		for my $url (map { $_->url_abs } $mech->followable_links) {
+			if (!$status_checked_uris{$url}) {
+				$status_checked_uris{$url} = 1;
+				$mech->get_ok($url, "[auto] check link ($url)")
+				    or diag("broken link to $url on $orig_url");
+				_verify_page;
+				$mech->back;
+			}
+		}
+	}
+	return 1
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure.  Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages).  You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	$pagedesc = $pagedesc ? " -- $pagedesc" : '';
+	if($params =~ /^$baseurl/) {
+		$url = "$params";
+	} else {
+		$url = "$baseurl$params";
+	}
+	if ($status) {
+		$mech->get($url);
+	} else {
+		$mech->get_ok($url, "get $url$pagedesc") or return 0;
+	}
+	check_page or return 0;
+	if ($status) {
+		return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+	} else {
+		return 1;
+	}
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+	(my $parms, $pagedesc) = @_;
+	$mech->follow_link_ok($parms, "follow link: $pagedesc") or return 0;
+	$url = $mech->uri;
+	return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+			     fields => { 's' => 'Initial' }
+			   }, "submit search form (default)")) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+	}
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog.  We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+		    'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+		    "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads.  (This should be extracted as well.)
+for my $head (@heads) {
+	for my $link_text qw( shortlog log tree ) {
+		ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+	}
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+	my $commit = rev_parse("$tag^{commit}");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+	   "shortlog link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+	   "log link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+	   "commit link for tag '$tag'");
+	# To do: Test tag link for tag objects.
+	# Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view.  Simply retrieve and check.
+{
+	# Broken link in Atom/RSS view -- cannot spider:
+	# http://mid.gmane.org/485EB333.5070108@gmail.com
+	local $long_tests = 0;
+	test_page('?p=.git;a=atom', 'Atom feed');
+	test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+	ok($mech->find_link(url_abs_regex => qr/a=tree/),
+	   "tree link on commit page ($url)");
+	$mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	$mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+	$mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $file (@files) {
+		my $file_hash = rev_parse("HEAD:$file");
+		ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+		if (get_type("HEAD:$file") eq 'blob') {
+			for my $link_text qw( blob blame history raw ) {
+				my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+							    text => $link_text);
+				ok($link, "'$file' file has $link_text link in tree view ($url)");
+			}
+		} else {
+			# Subtree -- to do: write tests.  (Need to set
+			# up a subtree in t9503-gitweb-Mechanize.sh.)
+		}
+	}
+}
+
+
+# Blame view
+{
+	# Broken link in blame view -- cannot spider:
+	# http://mid.gmane.org/485EC621.7090101@gmail.com
+	local $long_tests = 0;
+	if ($mech->get_ok('?p=.git', 'get repository summary') &&
+	    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+		for my $blame_link ($mech->find_all_links(text => 'blame')) {
+			test_page($blame_link->url, "follow blame link from tree view");
+			$mech->content_like(qr/A U Thor/,
+					    "author mentioned on blame page");
+		}
+	}
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $history_link ($mech->find_all_links(text => 'history')) {
+		test_page($history_link->url, "follow history link from tree view");
+		# To do: Expand.
+	}
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $blob_link ($mech->find_all_links(text => 'blob')) {
+		test_page($blob_link->url, "follow blob link from tree view");
+		# To do: Expand beyond standard tests.
+	}
+}
+
+
+# Raw view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $raw_link ($mech->find_all_links(text => 'raw')) {
+		test_page($raw_link->url, "follow raw link from tree view");
+	}
+}
+
+
+1;
+__END__
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a9fc621..504c0bb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB
 
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
+GITPERL=${GITPERL:-perl}
+export GITPERL
 test -d ../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
-- 
1.5.6.86.g5139f.dirty

[PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.

Also add a GITPERL environment variable that allows running Perl-based
tests with different perl binaries (and thus under different
versions).

Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
Signed-off-by: Jakub Narebski <jnareb@gmail.com>
---
Changed since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not
just test execution:

    diff --git a/t/t9503/test.pl b/t/t9503/test.pl
    index 28894c5..947e2e8 100755
    --- a/t/t9503/test.pl
    +++ b/t/t9503/test.pl
    @@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
     my $cgi = sub {
            # Use exec, not the shell, to support blanks in the path.
    -	my $status = system $gitweb $gitweb;
    +	my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
            my $value  = $status >> 8;


 t/t9503-gitweb-Mechanize.sh |  132 ++++++++++++++++
 t/t9503/test.pl             |  354 +++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh               |    2 +
 3 files changed, 488 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..3fe6d8b
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$GITPERL" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	"$GITPERL" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..947e2e8
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,354 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+# We don't count properly when skipping, so no_plan is necessary.
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+our $long_tests = $ENV{GIT_TEST_LONG};
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+    or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+    or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+    or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+# files and directories in HEAD root:
+chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);
+
+sub rev_parse {
+	my $name = shift;
+	chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+	$hash or undef;
+}
+
+sub get_type {
+	my $name = shift;
+	chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+	$type or undef;
+}
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# Thus subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+my $cgi = sub {
+	# Use exec, not the shell, to support blanks in the path.
+	my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
+	my $value  = $status >> 8;
+
+	croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+	    if ( $status == -1 );
+	croak( qq/Application '$gitweb' exited with value: $value/ )
+	    if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+	   GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+	   SCRIPT_FILENAME => $gitweb,
+	   $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+	my ($uri, $fragment) = split '#', $mech->uri;
+	if (!$verified_uris{$uri}) {
+		$verified_uris{$uri} = 1;
+
+		# Internal errors yield 200, but cause gitweb.cgi to
+		# exit with non-zero exit code, which Mechanize::CGI
+		# translates to 500, so we don't really need to check
+		# for "Software error" here, provided that the test
+		# cases always check the status code.
+		#$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+		# Validating is fast, so we can do it even without
+		# $long_tests.
+		$mech->html_lint_ok('[auto] validate HTML') or return 0
+		    if $html_lint_installed && $mech->is_html;
+		my $content_type = $mech->response->header('Content-Type')
+		    or die "$uri does not have a Content-Type header";
+		if ($xml_parser_installed && $content_type =~ /xml/) {
+			eval { XML::Parser->new->parse($mech->content); };
+			ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+		}
+		if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+			my $snapshot_file = File::Temp->new;
+			print $snapshot_file $mech->content;
+			close $snapshot_file;
+			my $t = Archive::Tar->new;
+			$t->read($snapshot_file->filename, 1);
+			ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+		}
+		# WebService::Validator::Feed::W3C would be nice to
+		# use, but it doesn't support direct input (as opposed
+		# to URIs) long enough for our feeds.
+	}
+	$mech->content_like(qr/(name|id)="$fragment"/,
+			    "[auto] fragment #$fragment exists ($uri)")
+	    if $fragment;
+	return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given.  Do not check the status code of the current page.
+my %spidered_uris;  # pages whose links have been checked
+my %status_checked_uris;  # verified pages whose status is known to be 2xx
+sub check_page {
+	_verify_page or return 0;
+	my $orig_url = $mech->uri;
+	if ($long_tests && !$spidered_uris{$mech->uri} ) {
+		$spidered_uris{$mech->uri} = 1;
+		for my $url (map { $_->url_abs } $mech->followable_links) {
+			if (!$status_checked_uris{$url}) {
+				$status_checked_uris{$url} = 1;
+				$mech->get_ok($url, "[auto] check link ($url)")
+				    or diag("broken link to $url on $orig_url");
+				_verify_page;
+				$mech->back;
+			}
+		}
+	}
+	return 1
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure.  Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages).  You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	$pagedesc = $pagedesc ? " -- $pagedesc" : '';
+	if($params =~ /^$baseurl/) {
+		$url = "$params";
+	} else {
+		$url = "$baseurl$params";
+	}
+	if ($status) {
+		$mech->get($url);
+	} else {
+		$mech->get_ok($url, "get $url$pagedesc") or return 0;
+	}
+	check_page or return 0;
+	if ($status) {
+		return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+	} else {
+		return 1;
+	}
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+	(my $parms, $pagedesc) = @_;
+	$mech->follow_link_ok($parms, "follow link: $pagedesc") or return 0;
+	$url = $mech->uri;
+	return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+			     fields => { 's' => 'Initial' }
+			   }, "submit search form (default)")) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+	}
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog.  We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+		    'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+		    "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads.  (This should be extracted as well.)
+for my $head (@heads) {
+	for my $link_text qw( shortlog log tree ) {
+		ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+	}
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+	my $commit = rev_parse("$tag^{commit}");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+	   "shortlog link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+	   "log link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+	   "commit link for tag '$tag'");
+	# To do: Test tag link for tag objects.
+	# Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view.  Simply retrieve and check.
+{
+	# Broken link in Atom/RSS view -- cannot spider:
+	# http://mid.gmane.org/485EB333.5070108@gmail.com
+	local $long_tests = 0;
+	test_page('?p=.git;a=atom', 'Atom feed');
+	test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+	ok($mech->find_link(url_abs_regex => qr/a=tree/),
+	   "tree link on commit page ($url)");
+	$mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	$mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+	$mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $file (@files) {
+		my $file_hash = rev_parse("HEAD:$file");
+		ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+		if (get_type("HEAD:$file") eq 'blob') {
+			for my $link_text qw( blob blame history raw ) {
+				my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+							    text => $link_text);
+				ok($link, "'$file' file has $link_text link in tree view ($url)");
+			}
+		} else {
+			# Subtree -- to do: write tests.  (Need to set
+			# up a subtree in t9503-gitweb-Mechanize.sh.)
+		}
+	}
+}
+
+
+# Blame view
+{
+	# Broken link in blame view -- cannot spider:
+	# http://mid.gmane.org/485EC621.7090101@gmail.com
+	local $long_tests = 0;
+	if ($mech->get_ok('?p=.git', 'get repository summary') &&
+	    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+		for my $blame_link ($mech->find_all_links(text => 'blame')) {
+			test_page($blame_link->url, "follow blame link from tree view");
+			$mech->content_like(qr/A U Thor/,
+					    "author mentioned on blame page");
+		}
+	}
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $history_link ($mech->find_all_links(text => 'history')) {
+		test_page($history_link->url, "follow history link from tree view");
+		# To do: Expand.
+	}
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $blob_link ($mech->find_all_links(text => 'blob')) {
+		test_page($blob_link->url, "follow blob link from tree view");
+		# To do: Expand beyond standard tests.
+	}
+}
+
+
+# Raw view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'follow first tree link on page')) {
+	for my $raw_link ($mech->find_all_links(text => 'raw')) {
+		test_page($raw_link->url, "follow raw link from tree view");
+	}
+}
+
+
+1;
+__END__
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a9fc621..504c0bb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB
 
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
+GITPERL=${GITPERL:-perl}
+export GITPERL
 test -d ../templates/blt || {
 	error "You haven't built things yet, have you?"
 }
-- 
1.5.6.86.g5139f.dirty

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

[Junio C Hamano]

Lea Wiemann <lewiemann@gmail.com> writes:

> Changed since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not
> just test execution:
>
>     diff --git a/t/t9503/test.pl b/t/t9503/test.pl
>     index 28894c5..947e2e8 100755
>     --- a/t/t9503/test.pl
>     +++ b/t/t9503/test.pl
>     @@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
>      my $cgi = sub {
>             # Use exec, not the shell, to support blanks in the path.
>     -	my $status = system $gitweb $gitweb;
>     +	my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
>             my $value  = $status >> 8;

My Perl must be getting rusty.  I had to look up this tricky syntax in
perlfunc.pod, under "=item exec", with examples like:

            $shell = '/bin/csh';
            exec $shell '-sh';		# pretend it's a login shell

        or, more directly,

            exec {'/bin/csh'} '-sh';	# pretend it's a login shell

>  t/t9503-gitweb-Mechanize.sh |  132 ++++++++++++++++
>  t/t9503/test.pl             |  354 +++++++++++++++++++++++++++++++++++++++++++
>  t/test-lib.sh               |    2 +
>  3 files changed, 488 insertions(+), 0 deletions(-)
>  create mode 100755 t/t9503-gitweb-Mechanize.sh
>  create mode 100755 t/t9503/test.pl
>
> diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
> new file mode 100755
> index 0000000..3fe6d8b
> --- /dev/null
> +++ b/t/t9503-gitweb-Mechanize.sh
> @@ -0,0 +1,132 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2008 Jakub Narebski
> +# Copyright (c) 2008 Lea Wiemann

If you mean by this that originally Jakub started and then Lea continued
extending the work, probably the order of Sign-off should match that order
to express the patch flow trail better.

> +# check if test can be run
> +"$GITPERL" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
> +	test_expect_success \
> +		'skipping gitweb tests, perl version is too old' :
> +	test_done
> +	exit
> +}
> +
> +"$GITPERL" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
> +	test_expect_success \
> +		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
> +	test_done
> +	exit
> +}

Nice touch.

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

[Lea Wiemann]

Junio C Hamano wrote:
> My Perl must be getting rusty.  I had to look up this tricky syntax in
> perlfunc.pod,

I looked in up in #perl. ;-)

>> +# Copyright (c) 2008 Jakub Narebski
>> +# Copyright (c) 2008 Lea Wiemann
> 
> If you mean by this that originally Jakub started and then Lea continued
> extending the work, probably the order of Sign-off should match that order
> to express the patch flow trail better.

Sure; I'll change it if I post another version, or feel free to change 
it when you apply the patch.

-- Lea

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

[Jakub Narebski]

Lea Wiemann wrote:

> +# We don't count properly when skipping, so no_plan is necessary.
> +use Test::More qw(no_plan);

I'd rather either skip this comment all together, or change it to
read something like that (I don't have good solution):

+# Counting tests upfront is difficult, as number of tests depends
+# on existence of several Perl modules, and is next to impossible
+# when spidering gitweb output (for --long-test).  Additionally,
+# having number of tests planned stated at beginning is not necessary,
+# as this test is to be run from git test suite, and not to be
+# processed further by TAP (Test Anything Protocol) Perl modules.

See http://www.perlfoundation.org/perl5/index.cgi?testing for
explanation why (and when) 'plan' is useful:

  TAP output usually starts with a plan line:
  
     1..9
  
  which specifies how many tests are to follow. The above example
  specifies 9 tests.
  
  This line is optional, but recommended. Should a test suite die
  part-way through, the plan allows the testing framework to recognise
  this situation, rather than assuming that all tests were completed.

> +our $long_tests = $ENV{GIT_TEST_LONG};

Here also it could be "my $long_tests"; but I am not a Perl hacker,
and do not know which is preferred.

> +eval { require Archive::Tar; };
> +my $archive_tar_installed = !$@
> +    or diag('Archive::Tar is not installed; no tests for valid snapshots');

> +chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
> +chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);

Wouldn't be easier to use '--format=%(refname)' in git-for-each-ref
invocation?  Besides, gitweb now uses in link _full_ refname, i.e.
"refs/heads/<name>" and "refs/tags/<name>" to avoid branch / tag
ambiguity (when there are both branch and head with the same name).

> +# files and directories in HEAD root:
> +chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);

Wouldn't it be easier to use "git ls-tree --names-only HEAD"?

> +sub rev_parse (...)
> +sub get_type (...)

Nice.

> +my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
> +
> +# Thus subroutine was copied (and modified to work with blanks in the
> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed
> +# 'under the same terms as perl itself' and thus GPL compatible.

Errr... I think that without my comment you removed it is hard to
understand what this comment talks about.  "Thus ..." without any
preceding sentence?

> +my $cgi = sub {
> +	# Use exec, not the shell, to support blanks in the path.

blanks = embedded whitespace?
path = path to gitweb?

> +	my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
> +	my $value  = $status >> 8;
> +
> +	croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
> +	    if ( $status == -1 );
> +	croak( qq/Application '$gitweb' exited with value: $value/ )
> +	    if ( $value > 0 );
> +};
> +
> +my $mech = new Test::WWW::Mechanize::CGI;
> +$mech->cgi($cgi);

Looks good.  Thanks.

> +		# WebService::Validator::Feed::W3C would be nice to
> +		# use, but it doesn't support direct input (as opposed
> +		# to URIs) long enough for our feeds.

Could you tell us here what are the limitations?  Are those limits
of W3C SOAP interface for web validation, or the CPAN package mentioned?

> +	return 1

Style: "return 1;"

> +# RSS/Atom/OPML view.  Simply retrieve and check.
> +{
> +	# Broken link in Atom/RSS view -- cannot spider:
> +	# http://mid.gmane.org/485EB333.5070108@gmail.com
> +	local $long_tests = 0;
> +	test_page('?p=.git;a=atom', 'Atom feed');
> +	test_page('?p=.git;a=rss', 'RSS feed');
> +}
> +test_page('?a=opml', 'OPML outline');

This I think should be written using Test::More equivalent of
test_expect_failure in t/test-lib.sh, namely TODO block, either
as 

  TODO: {
    local $TODO = "why";

    ...normal testing code...
  }

or if it causes problem with t9503-gitweb-Mechanize.sh failing, perhaps
as

  TODO: {
    todo_skip "why", <n> if <condition>;

    ...normal testing code...
  }


(And similarly for other pages).

> +# Blob view

I like those comments.


> diff --git a/t/test-lib.sh b/t/test-lib.sh
[...]
> +GITPERL=${GITPERL:-perl}
> +export GITPERL

How it is different from PERL_PATH?

-- 
Jakub Narebski
ShadeHawk on #git
Thorn, Poland

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

[Lea Wiemann]

[Open issue about PERL_PATH at the bottom!]

Jakub Narebski wrote:
> Lea Wiemann wrote:
> 
>> +# We don't count properly when skipping, so no_plan is necessary.
>> +use Test::More qw(no_plan);
> 
> I'd rather either skip this comment all together, or change it [...]

I'll just remove it.  Anybody trying to add a test count will give up 
within 30 seconds. ;-)

>> +our $long_tests = $ENV{GIT_TEST_LONG};
> 
> Here also it could be "my $long_tests";

This one is to make "local $long_tests = 0" work -- it wouldn't work 
with a 'my' declaration.

>> +chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
> 
> Wouldn't be easier to use '--format=%(refname)' in git-for-each-ref

*checks*  No, since I want to strip the leading refs/heads/ anyway (in 
order to test the page text, which mentions heads and tags without the 
directory prefix) -- hence the 'split' wouldn't go away with 
--format=%(refname).

>> +chomp(my @files = map { (split("\t", $_))[1] } `git-ls-tree HEAD`);
> 
> Wouldn't it be easier to use "git ls-tree --names-only HEAD"?

Yup ('--name-only' though).  Wonder why I didn't use it -- fixed.

>> +# Thus subroutine was copied (and modified to work with blanks in the
>> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed
>> +# 'under the same terms as perl itself' and thus GPL compatible.

That's a tyop: s/Thus/This/ (fixed).  It refers to the following "my 
$cgi = sub".  Does the comment make sense now?

>> +my $cgi = sub {
>> +	# Use exec, not the shell, to support blanks in the path.
> 
> blanks = embedded whitespace?  path = path to gitweb?

Yup.  Improved comment, and simplified system call:

-	# Use exec, not the shell, to support blanks in the path.
-	my $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;
+	# Use exec, not the shell, to support embedded whitespace in
+	# the path to gitweb.cgi.
+	my $status = system $ENV{GITPERL}, $gitweb;

>> +		# WebService::Validator::Feed::W3C would be nice to
>> +		# use, but it doesn't support direct input (as opposed
>> +		# to URIs) long enough for our feeds.
> 
> Could you tell us here what are the limitations?  Are those limits
> of W3C SOAP interface for web validation, or the CPAN package mentioned?

No idea.  I tried it and it told me that the URL was too long -- I 
suspect it's the W3C server that rejected it.  I wouldn't spend more 
effort trying to get online validation to work; it's probably easier to 
just occasionally validate manually. ;)  XML well-formedness tests 
should catch most occasional breakages just fine.

>> +	# Broken link in Atom/RSS view -- cannot spider:
>> +	# http://mid.gmane.org/485EB333.5070108@gmail.com
>> +	local $long_tests = 0;
>> +	test_page('?p=.git;a=atom', 'Atom feed');
> 
> This I think should be written using Test::More equivalent of
> test_expect_failure in t/test-lib.sh, namely TODO block,

Right -- here's my new version (which still fails if the feeds die or 
are not well-formed XML -- I'll want that when I change gitweb):

# RSS/Atom/OPML view
# Simply retrieve and verify well-formedness, but don't spider.
$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
TODO: {
	# Now spider -- but there are broken links.
	# http://mid.gmane.org/485EB333.5070108@gmail.com
	local $TODO = "fix broken links in Atom/RSS feeds";
	test_page('?p=.git;a=atom', 'Atom feed');
	test_page('?p=.git;a=rss', 'RSS feed');
}
test_page('?a=opml', 'OPML outline');

>> [in t/test-lib.sh:]
>> +GITPERL=${GITPERL:-perl}
>> +export GITPERL
>> [The idea is to easily run the test suite with different perl versions.]
> 
> How it is different from PERL_PATH?

Right, I didn't think of that.  PERL_PATH isn't available in the tests 
though, it's only used internally by the Makefile to generate (among 
other things) gitweb.cgi.  This means that while we can control under 
which Perl version gitweb.cgi runs, we cannot control under which Perl 
version the test suite runs (at least without $PATH trickery).  Does 
this bother us?

If yes, I'd suggest we keep GITPERL but rename it to GIT_TEST_PERL, 
because that's what it's about.  If not, I'll rip it out and simply call 
'perl' in the test shell script, whatever version it may be.

-- Lea

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

[Jakub Narebski]

On Mon, 23 Jun 2008, Lea Wiemann wrote:
> Jakub Narebski wrote:
>> Lea Wiemann wrote:
>> 
>>> +# We don't count properly when skipping, so no_plan is necessary.
>>> +use Test::More qw(no_plan);
>> 
>> I'd rather either skip this comment all together, or change it [...]
> 
> I'll just remove it.  Anybody trying to add a test count will give up 
> within 30 seconds. ;-)

I just think that not having comment is better than have comment which
is not fully true, or five lines of explanations which could be
expanded further.

Simply: planning (coming with number of tests upfront) is hard, and for
this situation it doesn't give us anything.
 
>>> +our $long_tests = $ENV{GIT_TEST_LONG};
>> 
>> Here also it could be "my $long_tests";
> 
> This one is to make "local $long_tests = 0" work -- it wouldn't work 
> with a 'my' declaration.

Hmmmm...

Siednote: some time ago I though that I understood difference between
lexical scoping (my) and dynamical scoping (local)...
 
>>> +chomp(my @heads = map { (split('/', $_))[2] } 
>>> +	`git-for-each-ref --sort=-committerdate refs/heads`); 
>> 
>> Wouldn't be easier to use '--format=%(refname)' in git-for-each-ref
> 
> *checks*  No, since I want to strip the leading refs/heads/ anyway (in 
> order to test the page text, which mentions heads and tags without the 
> directory prefix) -- hence the 'split' wouldn't go away with 
> --format=%(refname).

Well, you could have replaced 'split' by s!^refs/!! ;-)))

>>> +# Thus subroutine was copied (and modified to work with blanks in the
>>> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed
>>> +# 'under the same terms as perl itself' and thus GPL compatible.
> 
> That's a tyop: s/Thus/This/ (fixed).  It refers to the following "my 
> $cgi = sub".  Does the comment make sense now?

It makes sense now, thanks.

Although I guess it would be nice to have link to the ticket to the bug
(report) in WWW::Mechanize::CGI, so we could use cgi_application()
"again" if it gets fixed...

> +	my $status = system $ENV{GITPERL}, $gitweb;

...if not for the trick with explicitly calling Perl (GITPERL), to
allow testing different Perl versions with gitweb.
 
>>> +		# WebService::Validator::Feed::W3C would be nice to
>>> +		# use, but it doesn't support direct input (as opposed
>>> +		# to URIs) long enough for our feeds.
>> 
>> Could you tell us here what are the limitations?  Are those limits
>> of W3C SOAP interface for web validation, or the CPAN package mentioned?
> 
> No idea.  I tried it and it told me that the URL was too long -- I 
> suspect it's the W3C server that rejected it.  I wouldn't spend more 
> effort trying to get online validation to work; it's probably easier to 
> just occasionally validate manually. ;)  XML well-formedness tests 
> should catch most occasional breakages just fine.

Right.
 
Well, W3C feed validator provides SOAP interface we could use, using
POST of course and not GET (but it requires net connection), and also
standalone CLI validator (unfortunately in Python).

>>> +	# Broken link in Atom/RSS view -- cannot spider:
>>> +	# http://mid.gmane.org/485EB333.5070108@gmail.com
>>> +	local $long_tests = 0;
>>> +	test_page('?p=.git;a=atom', 'Atom feed');
>> 
>> This I think should be written using Test::More equivalent of
>> test_expect_failure in t/test-lib.sh, namely TODO block,
> 
> Right -- here's my new version (which still fails if the feeds die or 
> are not well-formed XML -- I'll want that when I change gitweb):
> 
> # RSS/Atom/OPML view
> # Simply retrieve and verify well-formedness, but don't spider.
> $mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
> $mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
> TODO: {
> 	# Now spider -- but there are broken links.
> 	# http://mid.gmane.org/485EB333.5070108@gmail.com
> 	local $TODO = "fix broken links in Atom/RSS feeds";
> 	test_page('?p=.git;a=atom', 'Atom feed');
> 	test_page('?p=.git;a=rss', 'RSS feed');
> }
> test_page('?a=opml', 'OPML outline');

Does it make t9503-gitweb.Mechanize.sh fail?
 
>>> [in t/test-lib.sh:]
>>> +GITPERL=${GITPERL:-perl}
>>> +export GITPERL
>>> [The idea is to easily run the test suite with different perl versions.]
>> 
>> How it is different from PERL_PATH?
> 
> Right, I didn't think of that.  PERL_PATH isn't available in the tests 
> though, it's only used internally by the Makefile to generate (among 
> other things) gitweb.cgi. [...]

That's what I was asking about.

-- 
Jakub Narebski
Poland

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

[Lea Wiemann]

Jakub Narebski wrote:
> Although I guess it would be nice to have link to the ticket to the bug
> (report) in WWW::Mechanize::CGI, so we could use cgi_application()
> "again" if it gets fixed...

Sure, I've added a link to 
http://rt.cpan.org/Ticket/Display.html?id=36654 for completeness, though 
we won't be able to use it if it gets fixed, since we can't break 
compatibility to older versions.  Btw, you may want to add a reply to 
the bug report (if that's possible -- haven't used CPAN's ticket system 
before) that "system $application $application" would be a better 
solution, since it doesn't suffer from quoting issues.

>> Right -- here's my new version (which still fails if the feeds die or 
>> are not well-formed XML -- I'll want that when I change gitweb):
>>
>> [snip code]
> 
> Does it make t9503-gitweb.Mechanize.sh fail?

No, my point was that it will fail *in case* it breaks (which is good). 
  Putting everything into a TODO block would silently ignore issues with 
the feeds.

>>>> +GITPERL=${GITPERL:-perl}
>>>> +export GITPERL

Breaking the indecisiveness, I've ripped it out. ;-)  We can still test 
gitweb under different Perl versions by make'ing it with different 
PERL_PATHs, so I think it'll be fine.

-- Lea

[PATCH v7] gitweb: add test suite with Test::WWW::Mechanize::CGI

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
Changes since v6:

- Removed GITPERL again.

- Added some basic tests for the page contents of blob and blob_plain
  views.

- Incorporated Jakub's suggestions (see parent posts); in particular
  blame and feed views *do* get spidered now in --long-tests mode, but
  the spider tests are marked as TODO and thus don't cause the test
  suite to fail.

- Some minor (internal) changes: @files only contains blobs, not
  trees; improved the logic that avoids checking pages twice; die if
  test.pl is run directly rather than from the shell script.

 t/t9503-gitweb-Mechanize.sh |  132 +++++++++++++++
 t/t9503/test.pl             |  378 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 510 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..6f7168e
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,132 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+perl -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+$0 test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	perl ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..872dbfd
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,378 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw(abs_path);
+use File::Spec;
+use File::Temp;
+
+use Test::More qw(no_plan);
+use Test::WWW::Mechanize::CGI;
+
+die "this must be run by calling the t/t*.sh shell script(s)\n"
+    if Cwd->cwd !~ /trash directory$/;
+
+our $long_tests = $ENV{GIT_TEST_LONG}; # "our" so we can use "local $long_tests"
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+    or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+    or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+    or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+sub rev_parse {
+	my $name = shift;
+	chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+	$hash or undef;
+}
+
+sub get_type {
+	my $name = shift;
+	chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+	$type or undef;
+}
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);
+my @files = grep { get_type("HEAD:$_") eq 'blob' } @root_entries or die;
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+# This subroutine was copied (and modified to work with blanks in the
+# application path) from WWW::Mechanize::CGI 0.3, which is licensed
+# 'under the same terms as perl itself' and thus GPL compatible.
+# http://rt.cpan.org/Ticket/Display.html?id=36654
+my $cgi = sub {
+	# Use exec, not the shell, to support embedded whitespace in
+	# the path to gitweb.cgi.
+	my $status = system $gitweb $gitweb;
+	my $value  = $status >> 8;
+
+	croak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )
+	    if ( $status == -1 );
+	croak( qq/Application '$gitweb' exited with value: $value/ )
+	    if ( $value > 0 );
+};
+
+my $mech = new Test::WWW::Mechanize::CGI;
+$mech->cgi($cgi);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+	   GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+	   SCRIPT_FILENAME => $gitweb,
+	   $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+	my ($uri, $fragment) = split '#', $mech->uri;
+	$mech->content_like(qr/(name|id)="$fragment"/,
+			    "[auto] fragment #$fragment exists ($uri)")
+	    if $fragment;
+
+	return 1 if $verified_uris{$uri};
+	$verified_uris{$uri} = 1;
+
+	# Internal errors yield 200 but cause gitweb.cgi to exit with
+	# non-zero exit code, which Mechanize::CGI translates to 500,
+	# so we don't really need to check for "Software error" here,
+	# provided that the test cases always check the status code.
+	#$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+	# Validate.  This is fast, so we can do it even without
+	# $long_tests.
+	$mech->html_lint_ok('[auto] validate HTML') or return 0
+	    if $html_lint_installed && $mech->is_html;
+	my $content_type = $mech->response->header('Content-Type')
+	    or die "$uri does not have a Content-Type header";
+	if ($xml_parser_installed && $content_type =~ /xml/) {
+		eval { XML::Parser->new->parse($mech->content); };
+		ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+	}
+	if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+		my $snapshot_file = File::Temp->new;
+		print $snapshot_file $mech->content;
+		close $snapshot_file;
+		my $t = Archive::Tar->new;
+		$t->read($snapshot_file->filename, 1);
+		ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+	}
+	# WebService::Validator::Feed::W3C would be nice to
+	# use, but it doesn't support direct input (as opposed
+	# to URIs) long enough for our feeds.
+
+	return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given.  Do not check the status code of the current page.
+my %spidered_uris;  # pages whose links have been checked
+my %status_checked_uris;  # verified pages whose status is known to be 2xx
+sub check_page {
+	_verify_page or return 0;
+	my $orig_url = $mech->uri;
+	if ($long_tests && !$spidered_uris{$mech->uri} ) {
+		$spidered_uris{$mech->uri} = 1;
+		for my $url (map { $_->url_abs } $mech->followable_links) {
+			if (!$status_checked_uris{$url}) {
+				$status_checked_uris{$url} = 1;
+				$mech->get_ok($url, "[auto] check link ($url)")
+				    or diag("broken link to $url on $orig_url");
+				_verify_page;
+				$mech->back;
+			}
+		}
+	}
+	return 1;
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure.  Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages).  You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	$pagedesc = $pagedesc ? " -- $pagedesc" : '';
+	if($params =~ /^$baseurl/) {
+		$url = "$params";
+	} else {
+		$url = "$baseurl$params";
+	}
+	if ($status) {
+		$mech->get($url);
+	} else {
+		$mech->get_ok($url, "get $url$pagedesc") or return 0;
+	}
+	check_page or return 0;
+	if ($status) {
+		return is($mech->status, $status, "getting $url$pagedesc -- yields $status");
+	} else {
+		return 1;
+	}
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example: follow_link( { text => 'commit' }, 'first commit link')
+# Like test_page, but does not support status code testing.
+sub follow_link {
+	(my $parms, $pagedesc) = @_;
+	unless ($mech->follow_link_ok($parms, "follow link: $pagedesc")) {
+		diag("cannot follow link ($pagedesc) on page " . $mech->uri);
+		return 0;
+	}
+	$url = $mech->uri;
+	return check_page;
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', 
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+$mech->get_ok('?p=.git', 'get repository summary');
+if ($mech->submit_form_ok( { form_number => 1,
+			     fields => { 's' => 'Initial' }
+			   }, "submit search form (default)")) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains searched commit');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+$mech->get_ok('?p=.git', 'get repository summary');
+for my $revision (@revisions[0..2]) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		ok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), "$link_text link for $revision");
+	}
+}
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog.  We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+		    'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+		    "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads.  (This should be extracted as well.)
+for my $head (@heads) {
+	for my $link_text qw( shortlog log tree ) {
+		ok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), "$link_text link for head '$head'");
+	}
+}
+
+# Check tags (assume we only have tags referring to commits).
+for my $tag (@tags) {
+	my $commit = rev_parse("$tag^{commit}");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),
+	   "shortlog link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),
+	   "log link for tag '$tag'");
+	ok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),
+	   "commit link for tag '$tag'");
+	# To do: Test tag link for tag objects.
+	# Why don't we have tree + snapshot links?
+}
+
+
+# RSS/Atom/OPML view
+# Simply retrieve and verify well-formedness, but don't spider.
+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
+TODO: {
+	# Now spider -- but there are broken links.
+	# http://mid.gmane.org/485EB333.5070108@gmail.com
+	local $TODO = "fix broken links in Atom/RSS feeds";
+	test_page('?p=.git;a=atom', 'Atom feed');
+	test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master')) {
+	ok($mech->find_link(url_abs_regex => qr/a=tree/),
+	   "tree link on commit page ($url)");
+	$mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	$mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+	$mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+
+
+# Tree view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'first tree link on page')) {
+	for my $file (@files) {
+		my $file_hash = rev_parse("HEAD:$file");
+		ok($mech->find_link(text => $file), "'$file' listed (and linked) in tree view ($url)");
+		for my $link_text qw( blob blame history raw ) {
+			my $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,
+						    text => $link_text);
+			ok($link, "'$file' file has $link_text link in tree view ($url)");
+		}
+	}
+	# To do: write tests for subtrees.  (Need to set up subtrees
+	# in t9503-gitweb-Mechanize.sh.)
+}
+
+
+# Blame view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'first tree link on page')) {
+	for my $blame_link ($mech->find_all_links(text => 'blame')) {
+		my $url = $blame_link->url;
+		$mech->get_ok($url, "get $url -- blame link on tree view")
+		    and _verify_page;
+		$mech->content_like(qr/A U Thor/,
+				    "author mentioned on blame page");
+		TODO: {
+			# Now spider -- but there are broken links.
+			# http://mid.gmane.org/485EC621.7090101@gmail.com
+			local $TODO = "fix broken links in certain blame views";
+			check_page;
+		}
+	}
+}
+
+
+# History view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'first tree link on page')) {
+	for my $history_link ($mech->find_all_links(text => 'history')) {
+		test_page($history_link->url, "follow history link from tree view");
+		# To do: Expand.
+	}
+}
+
+
+# Blob view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'first tree link on page')) {
+	for my $file (@files) {
+		if (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },
+				 "\"$file\" (blob) entry on tree view")) {
+			chomp(my $first_line_regex = (`cat "$file"`)[0]);
+			$first_line_regex =~ s/ / |&nbsp;/g;
+			# Hope that the first line doesn't contain any
+			# HTML-escapable character.
+			$mech->content_like(qr/$first_line_regex/,
+					    "blob view contains first line of file ($url)");
+			$mech->back;
+		}
+	}
+}
+
+
+# Raw (blob_plain) view
+if ($mech->get_ok('?p=.git', 'get repository summary') &&
+    follow_link( { text => 'tree' }, 'first tree link on page')) {
+	for my $file (@files) {
+		if (follow_link( { text => 'raw', url_abs_regex => qr/[^a-z]f=$file(;|$)/ },
+				 "raw (blob_plain) entry for \"$file\" in tree view")) {
+			chomp(my $last_line = (`cat "$file"`)[-1]);
+			$mech->content_contains(
+				$last_line, "blob_plain view contains last line of file");
+			$mech->back;
+		}
+	}
+}
+
+
+1;
+__END__
-- 
1.5.6.109.g0c85.dirty

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

[Junio C Hamano]

Lea Wiemann <lewiemann@gmail.com> writes:

>> How it is different from PERL_PATH?
>
> Right, I didn't think of that.  PERL_PATH isn't available in the tests
> though, it's only used internally by the Makefile to generate (among
> other things) gitweb.cgi.  This means that while we can control under
> which Perl version gitweb.cgi runs, we cannot control under which Perl
> version the test suite runs (at least without $PATH trickery).  Does
> this bother us?
>
> If yes, I'd suggest we keep GITPERL but rename it to GIT_TEST_PERL,
> because that's what it's about.  If not, I'll rip it out and simply
> call 'perl' in the test shell script, whatever version it may be.

That sounds wrong, as the point of tests would be to make sure the stuff
you are going to install would work with what you thought will be used
from the system.

If "isn't available in the tests" is the problem, is it possible to make
it available?  We are passing down SHELL_PATH from primary Makefile to t/
and you should be able to do the same for Perl path...

About the Test::WWW:Mechanize::CGI thing, how widely available is it?  I
do not think it is packaged for Debian nor Ubuntu, for example.

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

[Lea Wiemann]

Junio C Hamano wrote:
> We are passing down SHELL_PATH from primary Makefile to t/
> and you should be able to do the same for Perl path...

I see; that'll work, thanks!  Will send v8 soon.

> About the Test::WWW:Mechanize::CGI thing, how widely available is it?

Not very, you basically have to install it from CPAN.  If it's not 
installed, the only message you get from the test is:

ok 1: skipping gitweb tests, Test::WWW::Mechanize::CGI not found

Should optional test dependencies like Test::WWW::Mechnanize::CGI be 
documented in INSTALL?

Best,

     Lea

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

[Jakub Narebski]

On Tue, 24 Jun 2008, Junio C Hamano wrote:

> About the Test::WWW::Mechanize::CGI thing, how widely available is it?
> I do not think it is packaged for Debian nor Ubuntu, for example.

Not very widely; practically you have to install it from CPAN.  
Test::WWW::Mechanize::CGI is at v0.1, WWW::Mechanize::CGI is at v0.3.

But if Test::WWW::Mechanize::CGI is not installed, test would be not 
run.  If you are gitweb developer, then installing locally in $HOME 
from CPAN is I guess viable option; if you are not gitweb developer, we 
still have t/t9500-gitweb-standalone-no-errors.sh


The whole point of using those packages was that it makes it _easy_
to write those tests.  This consist of two parts: 

1). running gitweb as if it was CGI application, which otherwise would 
either require deep knowledge of how CGI application is invoked (what 
it is in WWW::Mechanize::CGI and required dependence 
HTTP::Request::AsCGI which I think does all the work) or working
web server, done a la "git instaweb",

2.) accessing and testing gitweb output (what Test::WWW::Mechanize,
WWW::Mechanize does; we could do the same with LWP* modules from 
libwww-perl / perl-libwww-perl package and Test::More from Perl, but
it would be repeating Test::WWW::Mechanize work).
-- 
Jakub Narebski
Poland

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

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
This patch applies on next.

Changes since v7:

- In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets
  source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.

- .git/description in the test repository no longer depends on $0
  (this would e.g. cause 'cd t; make' to fail).

- Add test_link subroutine and use it everywhere in place of
  ok(find_link...) so that links whose presence get tested get checked
  and spidered in --long-tests mode.

- Add simple page caching (reduces execution time without --long-tests
  by 25%).  That's *really* helpful when you have to run those tests
  on a regular basis. ;)  (WWW::Mechanize::Cached won't work with
  TWM::CGI, so we have to implement it ourselves; but it's easier
  anyway.)

- Follow redirects rather than failing.

- Test subdirectories in tree view.

- Test error handling for non-existent hashes or hashes of wrong type.

- Test commitdiff_plain view.

- Expand test for history view.

- Test tag objects (not just symbolic tags) in tag list.

- Test a specific bug (under "diff formatting", marked TODO).

- Do not test correctness of line number fragments (#l[0-9]+); they're
  broken too often right now.

- Probably some more minor improvements I've forgotten about. :)

 Makefile                    |    1 +
 t/t9503-gitweb-Mechanize.sh |  138 +++++++++++
 t/t9503/test.pl             |  553 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 692 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/Makefile b/Makefile
index a129491..3bf4f14 100644
--- a/Makefile
+++ b/Makefile
@@ -1251,6 +1251,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 
 GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
+	@echo PERL_PATH=\''$(PERL_PATH_SQ)'\' >>$@
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..4585cea
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,138 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$PERL_PATH" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+"$PERL_PATH" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	mkdir dir1 &&
+	echo "New file" >> dir1/file1 &&
+	git add dir1/file1 &&
+	test_tick && git commit -a -m "File added in subdirectory." &&
+	git tag -m "creating a tag object" tag-object
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$PERL_PATH" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+gitweb test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	"$PERL_PATH" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..43ad062
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,553 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw( abs_path );
+use File::Spec;
+use File::Temp;
+use Storable;
+
+use Test::More qw(no_plan);
+
+die "this must be run by calling the t/t*.sh shell script(s)\n"
+    if Cwd->cwd !~ /trash directory$/;
+
+our $long_tests = $ENV{GIT_TEST_LONG}; # "our" so we can use "local $long_tests"
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+    or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+    or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+    or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+sub rev_parse {
+	my $name = shift;
+	chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+	$hash or undef;
+}
+
+sub get_type {
+	my $name = shift;
+	chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+	$type or undef;
+}
+
+
+package OurMechanize;
+
+use base qw( Test::WWW::Mechanize::CGI );
+
+my %page_cache;
+# Cache requests.
+sub _make_request {
+	my ($self, $request) = (shift, shift);
+
+	my $response;
+	unless ($response = Storable::thaw($page_cache{$request->uri})) {
+		$response = $self->SUPER::_make_request($request, @_);
+		$page_cache{$request->uri} = Storable::freeze($response);
+	}
+	return $response;
+}
+
+# Fix whitespace problem.
+sub cgi_application {
+	my ($self, $application) = @_;
+
+	# This subroutine was copied (and modified) from
+	# WWW::Mechanize::CGI 0.3, which is licensed 'under the same
+	# terms as perl itself' and thus GPL compatible.
+	my $cgi = sub {
+		# Use exec, not the shell, to support embedded
+		# whitespace in the path to $application.
+		# http://rt.cpan.org/Ticket/Display.html?id=36654
+		my $status = system $application $application;
+		my $value  = $status >> 8;
+
+		croak( qq/Failed to execute application '$application'. Reason: '$!'/ )
+		    if ( $status == -1 );
+		croak( qq/Application '$application' exited with value: $value/ )
+		    if ( $value > 0 );
+	};
+
+	$self->cgi($cgi);
+}
+
+package main;
+
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+my @tag_objects = grep { get_type($_) eq 'tag' } @tags;
+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);
+my @files = grep { get_type("HEAD:$_") eq 'blob' } @root_entries or die;
+my @directories = grep { get_type("HEAD:$_") eq 'tree' } @root_entries or die;
+# Only test one file and directory each.
+@files = $files[0] unless $long_tests;
+@directories = $directories[0] unless $long_tests;
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+my $mech = OurMechanize->new;
+$mech->cgi_application($gitweb);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+	   GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+	   SCRIPT_FILENAME => $gitweb,
+	   $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+	my ($uri, $fragment) = split '#', $mech->uri;
+	TODO: {
+		local $TODO = 'line number fragments can be broken for diffs and blames'
+		    if $fragment && $fragment =~ /^l[0-9]+$/;
+		$mech->content_like(qr/(name|id)="$fragment"/,
+				    "[auto] fragment #$fragment exists ($uri)")
+		    if $fragment;
+	}
+
+	return 1 if $verified_uris{$uri};
+	$verified_uris{$uri} = 1;
+
+	# Internal errors yield 200 but cause gitweb.cgi to exit with
+	# non-zero exit code, which Mechanize::CGI translates to 500,
+	# so we don't really need to check for "Software error" here,
+	# provided that the test cases always check the status code.
+	#$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+	# Validate.  This is fast, so we can do it even without
+	# $long_tests.
+	$mech->html_lint_ok('[auto] validate HTML') or return 0
+	    if $html_lint_installed && $mech->is_html;
+	my $content_type = $mech->response->header('Content-Type')
+	    or die "$uri does not have a Content-Type header";
+	if ($xml_parser_installed && $content_type =~ /xml/) {
+		eval { XML::Parser->new->parse($mech->content); };
+		ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+	}
+	if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+		my $snapshot_file = File::Temp->new;
+		print $snapshot_file $mech->content;
+		close $snapshot_file;
+		my $t = Archive::Tar->new;
+		$t->read($snapshot_file->filename, 1);
+		ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+	}
+	# WebService::Validator::Feed::W3C would be nice to
+	# use, but it doesn't support direct input (as opposed
+	# to URIs) long enough for our feeds.
+
+	return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given.  Do not check the status code of the current page.
+my %spidered_uris;  # pages whose links have been checked
+my %status_checked_uris;  # verified pages whose status is known to be 2xx
+sub check_page {
+	_verify_page or return 0;
+	if ($long_tests && !$spidered_uris{$mech->uri} ) {
+		$spidered_uris{$mech->uri} = 1;
+		my $orig_url = $mech->uri;
+		TODO: {
+			local $TODO = "blame links can be broken sometimes"
+			    if $orig_url =~ /a=blame/;
+			for my $url (map { $_->url_abs } $mech->followable_links) {
+				if (!$status_checked_uris{$url}) {
+					$status_checked_uris{$url} = 1;
+					local $long_tests = 0;  # stop recursing
+					test_page($url, "[auto] check link ($url)")
+					    or diag("broken link to $url on $orig_url");
+					$mech->back;
+				}
+			}
+		}
+	}
+	return 1;
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure.  Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages).  You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	# missing $pagedesc is usually accidental
+	die "$params: no pagedesc given" unless defined $pagedesc;
+	if($params =~ /^$baseurl/) {
+		$url = "$params";
+	} else {
+		$url = "$baseurl$params";
+	}
+	$mech->get($url);
+	like($mech->status, $status ? qr/$status/ : qr/^[23][0-9][0-9]$/,
+	     "$pagedesc: $url" . ($status ? " -- yields $status" : ""))
+	    or return 0;
+	if ($mech->status =~ /^3/) {
+		# Don't check 3xx, they tend to look funny.
+		my $location = $mech->response->header('Location');
+		$mech->back;  # compensate for history
+		return test_page($location, "follow redirect from $url");
+	} else {
+		return check_page;
+	}
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example:
+# if (follow_link( { text => 'commit' }, 'first commit link')) {
+#     $mech->...;
+#     $mech->back;
+# }
+# Like test_page, but does not support status code testing, and
+# returns true if there was a link at all, regardless of whether it
+# was [23]xx or not.
+sub follow_link {
+	(my $parms, $pagedesc) = @_;
+	my $link = $mech->find_link(%$parms);
+	my $current_url = $mech->uri;
+	ok($link, "link exists: $pagedesc (on page $current_url)") or return 0;
+	test_page($link->url, "follow link: $pagedesc (on page $current_url)");
+	return 1;
+}
+
+# like follow_link, except that only checks and goes back immediately;
+# use this instead of ok(find_link...).
+sub test_link {
+	my ($parms, $pagedesc) = @_;
+	my $current_url = $mech->uri;
+	if($long_tests) {
+		# Check status, validate, spider.
+		return follow_link($parms, $pagedesc) && $mech->back;
+	} else {
+		# Only check presence of the link (much faster).
+		return ok($mech->find_link(%$parms),
+			  "link exists: $pagedesc (on page $current_url)");
+	}
+}
+
+sub get_summary {
+	test_page('?p=.git', 'repository summary')
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('gitweb test repository',
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+if (get_summary && $mech->submit_form_ok(
+	    { form_number => 1, fields => { 's' => 'Initial' } },
+	    'submit search form (default)')) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains commit we searched for');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+get_summary or die 'summary page failed; no point in running the remaining tests';
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+for my $revision (@revisions) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		test_link( { url_abs_regex => qr/h=$revision/, text => $link_text },
+			   "$link_text link for $revision");
+	}
+}
+
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog.  We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+		    'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+		    "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads.  (This should be extracted as well.)
+for my $head (@heads) {
+	for my $link_text qw( shortlog log tree ) {
+		test_link( { url_abs_regex => qr{h=refs/heads/$head}, text => $link_text },
+			   "$link_text link for head '$head'");
+	}
+}
+
+# Check tags (assume we only have tags referring to commits, not to
+# blobs or trees).
+for my $tag (@tags) {
+	my $commit = rev_parse("$tag^{commit}");
+	test_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog' },
+		   "shortlog link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'log' },
+		   "log link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=$commit}, text => 'commit' },
+		   "commit link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=$commit}, text => $tag },
+	   "'$tag' links to the commit as well");
+	# To do: Test tag link for tag objects.
+	# Why don't we have tree + snapshot links?
+}
+for my $tag (@tag_objects) {
+	my $tag_sha1 = rev_parse($tag);
+	test_link( { url_abs_regex => qr{h=$tag_sha1}, text => 'tag' },
+		   "tag link for tag object '$tag'" );
+}
+
+
+# RSS/Atom/OPML view
+# Simply retrieve and verify well-formedness, but don't spider.
+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
+TODO: {
+	# Now spider -- but there are broken links.
+	# http://mid.gmane.org/485EB333.5070108@gmail.com
+	local $TODO = "fix broken links in Atom/RSS feeds";
+	test_page('?p=.git;a=atom', 'Atom feed');
+	test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master', 'view HEAD commit')) {
+	my $tree_sha1 = rev_parse('master:');
+	test_link( { url_abs_regex => qr/a=tree/, text => rev_parse('master:') },
+		   "SHA1 link to tree on commit page ($url)");
+	test_link( { url_abs_regex => qr/h=$tree_sha1/, text => 'tree' },
+		   "'tree' link to tree on commit page ($url)");
+	$mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if (get_summary &&
+    follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	$mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+	$mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+test_page("?p=.git;a=commitdiff;h=$revisions[-1]",
+	  'commitdiff without parent');
+
+# Diff formattting problem.
+if (get_summary &&
+    follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	TODO: {
+		local $TODO = "bad a/* link in diff";
+		if (follow_link( { text_regex => qr!^a/! },
+				 'a/* link (probably wrong)')) {
+			# The page we land on here is broken already.
+			follow_link( { url_abs_regex => qr/a=blob_plain/ },
+				     'linked file name');  # bang
+		}
+	}
+}
+
+
+# Raw commitdiff (commitdiff_plain) view
+if (test_page('?p=.git;a=commit;h=refs/tags/tag-object',
+	      'commit view of tags/tag-object') &&
+    follow_link( { text => 'commitdiff' }, "'commitdiff'") &&
+    follow_link( { text => 'raw' }, "'raw' (commitdiff_plain)")) {
+	$mech->content_like(qr/^From: A U Thor <author\@example.com>$/m,
+			    'commitdiff_plain: From header');
+	TODO: {
+		local $TODO = 'date header mangles timezone';
+		$mech->content_like(qr/^Date: Thu, 7 Apr 2005 15:..:13 -0700$/m,
+				    'commitdiff_plain: Date header (correct)');
+	}
+	$mech->content_like(qr/^Date: Thu, 7 Apr 2005 22:..:13 \+0000 \(-0700\)$/m,
+			    'commitdiff_plain: Date header (UTC, wrong)');
+	$mech->content_like(qr/^Subject: .+$/m,
+			    'commitdiff_plain: Subject header');
+	# $ markers inexplicably don't work here if we use like(...)
+	# or $mech->content_like().
+	ok($mech->content =~ /^X-Git-Tag: tag-object\^0$/m,
+	   'commitdiff_plain: X-Git-Tag header');
+	ok($mech->content =~ /^X-Git-Url: $baseurl\?p=\.git;a=commitdiff_plain;h=refs%2Ftags%2Ftag-object$/m,
+	   'commitdiff_plain: X-Git-Url header');
+	ok($mech->content =~ /^---$/m, 'commitdiff_plain: separator$');
+	$mech->content_like(qr/^diff --git /m, 'commitdiff_plain: diff$');
+}
+
+
+# Tree view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		my $sha1 = rev_parse("HEAD:$file");
+		test_link( { text => $file, url_abs_regex => qr/h=$sha1/ },
+			   "'$file' is listed and linked");
+		test_link({ url_abs_regex => qr/f=$file/, text => $_ },
+			  "'$_' link") foreach qw( blame blob history raw );
+	}
+	for my $directory (@directories) {
+		my $sha1 = rev_parse("HEAD:$directory");
+		test_link({ url_abs_regex => qr/f=$directory/, text => $_ },
+			  "'$_' link") foreach qw( tree history );
+		if(follow_link( { text => $directory, url_abs_regex => qr/h=$sha1/ },
+				"'$directory is listed and linked" )) {
+			if(follow_link( { text => '..' }, 'parent directory')) {
+				test_link({ url_abs_regex => qr/h=$sha1/,
+					    text => $directory },
+					  'back to original tree view');
+				$mech->back;
+			}
+			$mech->back;
+		}
+	}
+}
+
+
+# Blame view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $blame_link ($mech->find_all_links(text => 'blame')) {
+		my $url = $blame_link->url;
+		$mech->get_ok($url, "get $url -- blame link on tree view")
+		    and _verify_page;
+		$mech->content_like(qr/A U Thor/,
+				    "author mentioned on blame page");
+		TODO: {
+			# Now spider -- but there are broken links.
+			# http://mid.gmane.org/485EC621.7090101@gmail.com
+			local $TODO = "fix broken links in certain blame views";
+			check_page;
+		}
+		last unless $long_tests; # only test first blame link
+	}
+}
+
+
+# History view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files, @directories) {
+		my $type = get_type("HEAD:$file");  # blob or tree
+		if (follow_link( { text => 'history', url_abs_regex => qr/f=$file/ },
+				 "history link for '$file'")) {
+			# There is at least one commit, so A U Thor is mentioned.
+			$mech->content_contains('A U Thor', 'A U Thor mentioned');
+			# The following tests test for at least *one*
+			# link of each type and are weak since we
+			# don't have any knowledge of commit hashes.
+			test_link( { text => $type, url_abs_regex => qr/f=$file/ },
+				   "$type");
+			test_link( { text => 'commitdiff' },
+				   "commitdiff");
+			test_link( { url_abs_regex => qr/a=commit;.*h=[a-f0-9]{40}/ },
+				   "subject links to commit"); # weak, brittle
+			$mech->back;
+		}
+	}
+}
+
+
+# Blob view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		if (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },
+				 "\"$file\" (blob) entry on tree view")) {
+			chomp(my $first_line_regex = (`cat "$file"`)[0]);
+			$first_line_regex =~ s/ / |&nbsp;/g;
+			# Hope that the first line doesn't contain any
+			# HTML-escapable character.
+			$mech->content_like(qr/$first_line_regex/,
+					    "blob view contains first line of file ($url)");
+			$mech->back;
+		}
+	}
+}
+
+
+# Raw (blob_plain) view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		if (follow_link( { text => 'raw', url_abs_regex => qr/f=$file/ },
+				 "raw (blob_plain) entry for \"$file\" in tree view")) {
+			chomp(my $first_line = (`cat "$file"`)[0]);
+			$mech->content_contains(
+				$first_line, "blob_plain view contains first line of file");
+			$mech->back;
+		}
+	}
+}
+
+
+# Error handling
+# Pass valid and invalid paths to various file-based actions
+for my $action qw( blame blob blob_plain blame ) {
+	test_page("?p=.git;a=$action;f=$files[0];hb=HEAD",
+		  "$action: look up existent file");
+	test_page("?p=.git;a=$action;f=does_not_exist;hb=HEAD",
+		  "$action: look up non-existent file", 404);
+	TODO: {
+		local $TODO = 'wrong error code (but using Git::Repo will fix this)';
+		test_page("?p=.git;a=$action;f=$directories[0];hb=HEAD",
+			  "$action: look up directory", 400);
+	}
+}
+TODO: {
+	local $TODO = 'wrong error code (but using Git::Repo will fix this)';
+	test_page("?p=.git;a=tree;f=$files[0];hb=HEAD",
+		  'tree: look up existent file', 400);
+}
+# Pass valid and invalid paths to tree action
+test_page("?p=.git;a=tree;f=does_not_exist;hb=HEAD",
+	  'tree: look up non-existent file', 404);
+test_page("?p=.git;a=tree;f=$directories[0];hb=HEAD",
+	  'tree: look up directory');
+TODO: {
+	local $TODO = 'cannot use f=/ or f= for trees';
+	test_page("?p=.git;a=tree;f=/;hb=HEAD", 'tree: look up directory');
+}
+
+
+1;
+__END__
-- 
1.5.6.109.g33ded.dirty

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

[Lea Wiemann]

This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
to validate the HTML/XML/tgz output, and checks all links on the
tested pages if --long-tests is given.

Signed-off-by: Jakub Narebski <jnareb@gmail.com>
Signed-off-by: Lea Wiemann <LeWiemann@gmail.com>
---
[Resent with correct subject.]

This patch applies on next.

Changes since v7:

- In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets
  source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.

- .git/description in the test repository no longer depends on $0
  (this would e.g. cause 'cd t; make' to fail).

- Add test_link subroutine and use it everywhere in place of
  ok(find_link...) so that links whose presence get tested get checked
  and spidered in --long-tests mode.

- Add simple page caching (reduces execution time without --long-tests
  by 25%).  That's *really* helpful when you have to run those tests
  on a regular basis. ;)  (WWW::Mechanize::Cached won't work with
  TWM::CGI, so we have to implement it ourselves; but it's easier
  anyway.)

- Follow redirects rather than failing.

- Test subdirectories in tree view.

- Test error handling for non-existent hashes or hashes of wrong type.

- Test commitdiff_plain view.

- Expand test for history view.

- Test tag objects (not just symbolic tags) in tag list.

- Test a specific bug (under "diff formatting", marked TODO).

- Do not test correctness of line number fragments (#l[0-9]+); they're
  broken too often right now.

- Probably some more minor improvements I've forgotten about. :)

 Makefile                    |    1 +
 t/t9503-gitweb-Mechanize.sh |  138 +++++++++++
 t/t9503/test.pl             |  553 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 692 insertions(+), 0 deletions(-)
 create mode 100755 t/t9503-gitweb-Mechanize.sh
 create mode 100755 t/t9503/test.pl

diff --git a/Makefile b/Makefile
index a129491..3bf4f14 100644
--- a/Makefile
+++ b/Makefile
@@ -1251,6 +1251,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS
 
 GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS
 	@echo SHELL_PATH=\''$(SHELL_PATH_SQ)'\' >$@
+	@echo PERL_PATH=\''$(PERL_PATH_SQ)'\' >>$@
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh
new file mode 100755
index 0000000..4585cea
--- /dev/null
+++ b/t/t9503-gitweb-Mechanize.sh
@@ -0,0 +1,138 @@
+#!/bin/sh
+#
+# Copyright (c) 2008 Jakub Narebski
+# Copyright (c) 2008 Lea Wiemann
+#
+
+# This test supports the --long-tests option.
+
+# This test only runs on Perl 5.8 and later versions, since
+# Test::WWW::Mechanize::CGI requires Perl 5.8.
+
+test_description='gitweb tests (using WWW::Mechanize)
+
+This test uses Test::WWW::Mechanize::CGI to test gitweb.'
+
+# helper functions
+
+safe_chmod () {
+	chmod "$1" "$2" &&
+	if [ "$(git config --get core.filemode)" = false ]
+	then
+		git update-index --chmod="$1" "$2"
+	fi
+}
+
+. ./test-lib.sh
+
+# check if test can be run
+"$PERL_PATH" -MEncode -e 'decode_utf8("", Encode::FB_CROAK)' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, perl version is too old' :
+	test_done
+	exit
+}
+
+"$PERL_PATH" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {
+	test_expect_success \
+		'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :
+	test_done
+	exit
+}
+
+# set up test repository
+test_expect_success 'set up test repository' '
+
+	echo "Not an empty file." > file &&
+	git add file &&
+	test_tick && git commit -a -m "Initial commit." &&
+	git branch b &&
+
+	echo "New file" > new_file &&
+	git add new_file &&
+	test_tick && git commit -a -m "File added." &&
+
+	safe_chmod +x new_file &&
+	test_tick && git commit -a -m "Mode changed." &&
+
+	git mv new_file renamed_file &&
+	test_tick && git commit -a -m "File renamed." &&
+
+	rm renamed_file &&
+	ln -s file renamed_file &&
+	test_tick && git commit -a -m "File to symlink." &&
+	git tag with-symlink &&
+
+	git rm renamed_file &&
+	rm -f renamed_file &&
+	test_tick && git commit -a -m "File removed." &&
+
+	cp file file2 &&
+	git add file2 &&
+	test_tick && git commit -a -m "File copied." &&
+
+	echo "New line" >> file2 &&
+	safe_chmod +x file2 &&
+	test_tick && git commit -a -m "Mode change and modification." &&
+
+	mkdir dir1 &&
+	echo "New file" >> dir1/file1 &&
+	git add dir1/file1 &&
+	test_tick && git commit -a -m "File added in subdirectory." &&
+	git tag -m "creating a tag object" tag-object
+
+	git checkout b &&
+	echo "Branch" >> b &&
+	git add b &&
+	test_tick && git commit -a -m "On branch" &&
+	git checkout master &&
+	test_tick && git pull . b
+'
+
+# set up empty repository
+# TODO!
+
+# set up repositories for gitweb
+# TODO!
+
+# set up gitweb configuration
+safe_pwd="$("$PERL_PATH" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')"
+cat >gitweb_config.perl <<EOF
+# gitweb configuration for tests
+
+our \$version = "current";
+our \$GIT = "$GIT_EXEC_PATH/git";
+our \$projectroot = "$safe_pwd";
+our \$project_maxdepth = 8;
+our \$home_link_str = "projects";
+our \$site_name = "[localhost]";
+our \$site_header = "";
+our \$site_footer = "";
+our \$home_text = "indextext.html";
+our @stylesheets = ("file:///$safe_pwd/../../gitweb/gitweb.css");
+our \$logo = "file:///$safe_pwd/../../gitweb/git-logo.png";
+our \$favicon = "file:///$safe_pwd/../../gitweb/git-favicon.png";
+our \$projects_list = "";
+our \$export_ok = "";
+our \$strict_export = "";
+our %feature;
+\$feature{'blame'}{'default'} = [1];
+
+1;
+__END__
+EOF
+
+cat >.git/description <<EOF
+gitweb test repository
+EOF
+
+GITWEB_CONFIG="$(pwd)/gitweb_config.perl"
+export GITWEB_CONFIG
+
+# run tests
+
+test_external \
+	'test gitweb output' \
+	"$PERL_PATH" ../t9503/test.pl
+
+test_done
diff --git a/t/t9503/test.pl b/t/t9503/test.pl
new file mode 100755
index 0000000..43ad062
--- /dev/null
+++ b/t/t9503/test.pl
@@ -0,0 +1,553 @@
+#!/usr/bin/perl
+use lib (split(/:/, $ENV{GITPERLLIB}));
+
+# This test supports the --long-tests option.
+
+use warnings;
+use strict;
+
+use Cwd qw( abs_path );
+use File::Spec;
+use File::Temp;
+use Storable;
+
+use Test::More qw(no_plan);
+
+die "this must be run by calling the t/t*.sh shell script(s)\n"
+    if Cwd->cwd !~ /trash directory$/;
+
+our $long_tests = $ENV{GIT_TEST_LONG}; # "our" so we can use "local $long_tests"
+
+eval { require Archive::Tar; };
+my $archive_tar_installed = !$@
+    or diag('Archive::Tar is not installed; no tests for valid snapshots');
+
+eval { require HTML::Lint; };
+my $html_lint_installed = !$@
+    or diag('HTML::Lint is not installed; no HTML validation tests');
+
+eval { require XML::Parser; };
+my $xml_parser_installed = !$@
+    or diag('XML::Parser is not installed; no tests for well-formed XML');
+
+sub rev_parse {
+	my $name = shift;
+	chomp(my $hash = `git rev-parse $name 2> /dev/null`);
+	$hash or undef;
+}
+
+sub get_type {
+	my $name = shift;
+	chomp(my $type = `git cat-file -t $name 2> /dev/null`);
+	$type or undef;
+}
+
+
+package OurMechanize;
+
+use base qw( Test::WWW::Mechanize::CGI );
+
+my %page_cache;
+# Cache requests.
+sub _make_request {
+	my ($self, $request) = (shift, shift);
+
+	my $response;
+	unless ($response = Storable::thaw($page_cache{$request->uri})) {
+		$response = $self->SUPER::_make_request($request, @_);
+		$page_cache{$request->uri} = Storable::freeze($response);
+	}
+	return $response;
+}
+
+# Fix whitespace problem.
+sub cgi_application {
+	my ($self, $application) = @_;
+
+	# This subroutine was copied (and modified) from
+	# WWW::Mechanize::CGI 0.3, which is licensed 'under the same
+	# terms as perl itself' and thus GPL compatible.
+	my $cgi = sub {
+		# Use exec, not the shell, to support embedded
+		# whitespace in the path to $application.
+		# http://rt.cpan.org/Ticket/Display.html?id=36654
+		my $status = system $application $application;
+		my $value  = $status >> 8;
+
+		croak( qq/Failed to execute application '$application'. Reason: '$!'/ )
+		    if ( $status == -1 );
+		croak( qq/Application '$application' exited with value: $value/ )
+		    if ( $value > 0 );
+	};
+
+	$self->cgi($cgi);
+}
+
+package main;
+
+
+my @revisions = split /\s/, `git-rev-list --first-parent HEAD`;
+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);
+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);
+my @tag_objects = grep { get_type($_) eq 'tag' } @tags;
+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);
+my @files = grep { get_type("HEAD:$_") eq 'blob' } @root_entries or die;
+my @directories = grep { get_type("HEAD:$_") eq 'tree' } @root_entries or die;
+# Only test one file and directory each.
+@files = $files[0] unless $long_tests;
+@directories = $directories[0] unless $long_tests;
+
+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));
+
+my $mech = OurMechanize->new;
+$mech->cgi_application($gitweb);
+# On some systems(?) it's necessary to have %ENV here, otherwise the
+# CGI process won't get *any* of the current environment variables
+# (not even PATH, etc.)
+$mech->env(%ENV,
+	   GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},
+	   SCRIPT_FILENAME => $gitweb,
+	   $mech->env);
+
+# import config, predeclaring config variables
+our $site_name;
+require_ok($ENV{'GITWEB_CONFIG'})
+	or diag('Could not load gitweb config; some tests would fail');
+
+# Perform non-recursive checks on the current page, but do not check
+# the status code.
+my %verified_uris;
+sub _verify_page {
+	my ($uri, $fragment) = split '#', $mech->uri;
+	TODO: {
+		local $TODO = 'line number fragments can be broken for diffs and blames'
+		    if $fragment && $fragment =~ /^l[0-9]+$/;
+		$mech->content_like(qr/(name|id)="$fragment"/,
+				    "[auto] fragment #$fragment exists ($uri)")
+		    if $fragment;
+	}
+
+	return 1 if $verified_uris{$uri};
+	$verified_uris{$uri} = 1;
+
+	# Internal errors yield 200 but cause gitweb.cgi to exit with
+	# non-zero exit code, which Mechanize::CGI translates to 500,
+	# so we don't really need to check for "Software error" here,
+	# provided that the test cases always check the status code.
+	#$mech->content_lacks('<h1>Software error:</h1>') or return 0;
+
+	# Validate.  This is fast, so we can do it even without
+	# $long_tests.
+	$mech->html_lint_ok('[auto] validate HTML') or return 0
+	    if $html_lint_installed && $mech->is_html;
+	my $content_type = $mech->response->header('Content-Type')
+	    or die "$uri does not have a Content-Type header";
+	if ($xml_parser_installed && $content_type =~ /xml/) {
+		eval { XML::Parser->new->parse($mech->content); };
+		ok(!$@, "[auto] check for XML well-formedness ($uri)") or diag($@);
+	}
+	if ($archive_tar_installed && $uri =~ /sf=tgz/) {
+		my $snapshot_file = File::Temp->new;
+		print $snapshot_file $mech->content;
+		close $snapshot_file;
+		my $t = Archive::Tar->new;
+		$t->read($snapshot_file->filename, 1);
+		ok($t->get_files, "[auto] valid tgz snapshot ($uri)");
+	}
+	# WebService::Validator::Feed::W3C would be nice to
+	# use, but it doesn't support direct input (as opposed
+	# to URIs) long enough for our feeds.
+
+	return 1;
+}
+
+# Verify and spider the current page, the latter only if --long-tests
+# (-l) is given.  Do not check the status code of the current page.
+my %spidered_uris;  # pages whose links have been checked
+my %status_checked_uris;  # verified pages whose status is known to be 2xx
+sub check_page {
+	_verify_page or return 0;
+	if ($long_tests && !$spidered_uris{$mech->uri} ) {
+		$spidered_uris{$mech->uri} = 1;
+		my $orig_url = $mech->uri;
+		TODO: {
+			local $TODO = "blame links can be broken sometimes"
+			    if $orig_url =~ /a=blame/;
+			for my $url (map { $_->url_abs } $mech->followable_links) {
+				if (!$status_checked_uris{$url}) {
+					$status_checked_uris{$url} = 1;
+					local $long_tests = 0;  # stop recursing
+					test_page($url, "[auto] check link ($url)")
+					    or diag("broken link to $url on $orig_url");
+					$mech->back;
+				}
+			}
+		}
+	}
+	return 1;
+}
+
+my $baseurl = "http://localhost";
+my ($params, $url, $pagedesc, $status);
+
+# test_page ( <params>, <page_description>, <expected_status> )
+# Example:
+# if (test_page('?p=.git;a=summary', 'repository summary')) {
+#     $mech->...;
+#     $mech->...;
+# }
+#
+# Test that the page can be opened, call _verify_page on it, and
+# return true if there was no test failure.  Also set the global
+# variables $params, $pagedesc, and $url for use in the if block.
+# Optionally pass a third parameter $status to test the HTTP status
+# code of the page (useful for error pages).  You can also pass a full
+# URL instead of just parameters as the first parameter.
+sub test_page {
+	($params, $pagedesc, $status) = @_;
+	# missing $pagedesc is usually accidental
+	die "$params: no pagedesc given" unless defined $pagedesc;
+	if($params =~ /^$baseurl/) {
+		$url = "$params";
+	} else {
+		$url = "$baseurl$params";
+	}
+	$mech->get($url);
+	like($mech->status, $status ? qr/$status/ : qr/^[23][0-9][0-9]$/,
+	     "$pagedesc: $url" . ($status ? " -- yields $status" : ""))
+	    or return 0;
+	if ($mech->status =~ /^3/) {
+		# Don't check 3xx, they tend to look funny.
+		my $location = $mech->response->header('Location');
+		$mech->back;  # compensate for history
+		return test_page($location, "follow redirect from $url");
+	} else {
+		return check_page;
+	}
+}
+
+# follow_link ( \%parms, $pagedesc )
+# Example:
+# if (follow_link( { text => 'commit' }, 'first commit link')) {
+#     $mech->...;
+#     $mech->back;
+# }
+# Like test_page, but does not support status code testing, and
+# returns true if there was a link at all, regardless of whether it
+# was [23]xx or not.
+sub follow_link {
+	(my $parms, $pagedesc) = @_;
+	my $link = $mech->find_link(%$parms);
+	my $current_url = $mech->uri;
+	ok($link, "link exists: $pagedesc (on page $current_url)") or return 0;
+	test_page($link->url, "follow link: $pagedesc (on page $current_url)");
+	return 1;
+}
+
+# like follow_link, except that only checks and goes back immediately;
+# use this instead of ok(find_link...).
+sub test_link {
+	my ($parms, $pagedesc) = @_;
+	my $current_url = $mech->uri;
+	if($long_tests) {
+		# Check status, validate, spider.
+		return follow_link($parms, $pagedesc) && $mech->back;
+	} else {
+		# Only check presence of the link (much faster).
+		return ok($mech->find_link(%$parms),
+			  "link exists: $pagedesc (on page $current_url)");
+	}
+}
+
+sub get_summary {
+	test_page('?p=.git', 'repository summary')
+}
+
+if (test_page '', 'project list (implicit)') {
+	$mech->title_like(qr!$site_name!,
+		"title contains $site_name");
+	$mech->content_contains('gitweb test repository',
+		'lists test repository (by description)');
+}
+
+# Test repository summary: implicit, implicit with pathinfo, explicit.
+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {
+	if (test_page $sumparams, 'repository summary') {
+		$mech->title_like(qr!$site_name.*\.git/summary!,
+				  "title contains $site_name and \".git/summary\"");
+	}
+}
+
+# Search form (on summary page).
+if (get_summary && $mech->submit_form_ok(
+	    { form_number => 1, fields => { 's' => 'Initial' } },
+	    'submit search form (default)')) {
+	check_page;
+	$mech->content_contains('Initial commit',
+				'content contains commit we searched for');
+}
+
+test_page('?p=non-existent.git', 'non-existent project', 404);
+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);
+
+
+# Summary view
+
+get_summary or die 'summary page failed; no point in running the remaining tests';
+
+# Check short log.  To do: Extract into separate test_short_log
+# function since the short log occurs on several pages.
+for my $revision (@revisions) {
+	for my $link_text qw( commit commitdiff tree snapshot ) {
+		test_link( { url_abs_regex => qr/h=$revision/, text => $link_text },
+			   "$link_text link for $revision");
+	}
+}
+
+# Check that branches and tags are highlighted in green and yellow in
+# the shortlog.  We assume here that we are on master, so it should be
+# at the top.
+$mech->content_like(qr{<span [^>]*class="head"[^>]*>master</span>},
+		    'master branch is highlighted in shortlog');
+$mech->content_like(qr{<span [^>]*class="tag"[^>]*>$tags[0]</span>},
+		    "$tags[0] (most recent tag) is highlighted in shortlog");
+
+# Check heads.  (This should be extracted as well.)
+for my $head (@heads) {
+	for my $link_text qw( shortlog log tree ) {
+		test_link( { url_abs_regex => qr{h=refs/heads/$head}, text => $link_text },
+			   "$link_text link for head '$head'");
+	}
+}
+
+# Check tags (assume we only have tags referring to commits, not to
+# blobs or trees).
+for my $tag (@tags) {
+	my $commit = rev_parse("$tag^{commit}");
+	test_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog' },
+		   "shortlog link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'log' },
+		   "log link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=$commit}, text => 'commit' },
+		   "commit link for tag '$tag'");
+	test_link( { url_abs_regex => qr{h=$commit}, text => $tag },
+	   "'$tag' links to the commit as well");
+	# To do: Test tag link for tag objects.
+	# Why don't we have tree + snapshot links?
+}
+for my $tag (@tag_objects) {
+	my $tag_sha1 = rev_parse($tag);
+	test_link( { url_abs_regex => qr{h=$tag_sha1}, text => 'tag' },
+		   "tag link for tag object '$tag'" );
+}
+
+
+# RSS/Atom/OPML view
+# Simply retrieve and verify well-formedness, but don't spider.
+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;
+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;
+TODO: {
+	# Now spider -- but there are broken links.
+	# http://mid.gmane.org/485EB333.5070108@gmail.com
+	local $TODO = "fix broken links in Atom/RSS feeds";
+	test_page('?p=.git;a=atom', 'Atom feed');
+	test_page('?p=.git;a=rss', 'RSS feed');
+}
+test_page('?a=opml', 'OPML outline');
+
+
+# Commit view
+if (test_page('?p=.git;a=commit;h=master', 'view HEAD commit')) {
+	my $tree_sha1 = rev_parse('master:');
+	test_link( { url_abs_regex => qr/a=tree/, text => rev_parse('master:') },
+		   "SHA1 link to tree on commit page ($url)");
+	test_link( { url_abs_regex => qr/h=$tree_sha1/, text => 'tree' },
+		   "'tree' link to tree on commit page ($url)");
+	$mech->content_like(qr/A U Thor/, "author mentioned on commit page ($url)");
+}
+
+
+# Commitdiff view
+if (get_summary &&
+    follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	$mech->content_like(qr/new file with mode/, "commitdiff has diffstat ($url)");
+	$mech->content_like(qr/new file mode/, "commitdiff has diff ($url)");
+}
+test_page("?p=.git;a=commitdiff;h=$revisions[-1]",
+	  'commitdiff without parent');
+
+# Diff formattting problem.
+if (get_summary &&
+    follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&
+    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
+	TODO: {
+		local $TODO = "bad a/* link in diff";
+		if (follow_link( { text_regex => qr!^a/! },
+				 'a/* link (probably wrong)')) {
+			# The page we land on here is broken already.
+			follow_link( { url_abs_regex => qr/a=blob_plain/ },
+				     'linked file name');  # bang
+		}
+	}
+}
+
+
+# Raw commitdiff (commitdiff_plain) view
+if (test_page('?p=.git;a=commit;h=refs/tags/tag-object',
+	      'commit view of tags/tag-object') &&
+    follow_link( { text => 'commitdiff' }, "'commitdiff'") &&
+    follow_link( { text => 'raw' }, "'raw' (commitdiff_plain)")) {
+	$mech->content_like(qr/^From: A U Thor <author\@example.com>$/m,
+			    'commitdiff_plain: From header');
+	TODO: {
+		local $TODO = 'date header mangles timezone';
+		$mech->content_like(qr/^Date: Thu, 7 Apr 2005 15:..:13 -0700$/m,
+				    'commitdiff_plain: Date header (correct)');
+	}
+	$mech->content_like(qr/^Date: Thu, 7 Apr 2005 22:..:13 \+0000 \(-0700\)$/m,
+			    'commitdiff_plain: Date header (UTC, wrong)');
+	$mech->content_like(qr/^Subject: .+$/m,
+			    'commitdiff_plain: Subject header');
+	# $ markers inexplicably don't work here if we use like(...)
+	# or $mech->content_like().
+	ok($mech->content =~ /^X-Git-Tag: tag-object\^0$/m,
+	   'commitdiff_plain: X-Git-Tag header');
+	ok($mech->content =~ /^X-Git-Url: $baseurl\?p=\.git;a=commitdiff_plain;h=refs%2Ftags%2Ftag-object$/m,
+	   'commitdiff_plain: X-Git-Url header');
+	ok($mech->content =~ /^---$/m, 'commitdiff_plain: separator$');
+	$mech->content_like(qr/^diff --git /m, 'commitdiff_plain: diff$');
+}
+
+
+# Tree view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		my $sha1 = rev_parse("HEAD:$file");
+		test_link( { text => $file, url_abs_regex => qr/h=$sha1/ },
+			   "'$file' is listed and linked");
+		test_link({ url_abs_regex => qr/f=$file/, text => $_ },
+			  "'$_' link") foreach qw( blame blob history raw );
+	}
+	for my $directory (@directories) {
+		my $sha1 = rev_parse("HEAD:$directory");
+		test_link({ url_abs_regex => qr/f=$directory/, text => $_ },
+			  "'$_' link") foreach qw( tree history );
+		if(follow_link( { text => $directory, url_abs_regex => qr/h=$sha1/ },
+				"'$directory is listed and linked" )) {
+			if(follow_link( { text => '..' }, 'parent directory')) {
+				test_link({ url_abs_regex => qr/h=$sha1/,
+					    text => $directory },
+					  'back to original tree view');
+				$mech->back;
+			}
+			$mech->back;
+		}
+	}
+}
+
+
+# Blame view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $blame_link ($mech->find_all_links(text => 'blame')) {
+		my $url = $blame_link->url;
+		$mech->get_ok($url, "get $url -- blame link on tree view")
+		    and _verify_page;
+		$mech->content_like(qr/A U Thor/,
+				    "author mentioned on blame page");
+		TODO: {
+			# Now spider -- but there are broken links.
+			# http://mid.gmane.org/485EC621.7090101@gmail.com
+			local $TODO = "fix broken links in certain blame views";
+			check_page;
+		}
+		last unless $long_tests; # only test first blame link
+	}
+}
+
+
+# History view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files, @directories) {
+		my $type = get_type("HEAD:$file");  # blob or tree
+		if (follow_link( { text => 'history', url_abs_regex => qr/f=$file/ },
+				 "history link for '$file'")) {
+			# There is at least one commit, so A U Thor is mentioned.
+			$mech->content_contains('A U Thor', 'A U Thor mentioned');
+			# The following tests test for at least *one*
+			# link of each type and are weak since we
+			# don't have any knowledge of commit hashes.
+			test_link( { text => $type, url_abs_regex => qr/f=$file/ },
+				   "$type");
+			test_link( { text => 'commitdiff' },
+				   "commitdiff");
+			test_link( { url_abs_regex => qr/a=commit;.*h=[a-f0-9]{40}/ },
+				   "subject links to commit"); # weak, brittle
+			$mech->back;
+		}
+	}
+}
+
+
+# Blob view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		if (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },
+				 "\"$file\" (blob) entry on tree view")) {
+			chomp(my $first_line_regex = (`cat "$file"`)[0]);
+			$first_line_regex =~ s/ / |&nbsp;/g;
+			# Hope that the first line doesn't contain any
+			# HTML-escapable character.
+			$mech->content_like(qr/$first_line_regex/,
+					    "blob view contains first line of file ($url)");
+			$mech->back;
+		}
+	}
+}
+
+
+# Raw (blob_plain) view
+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {
+	for my $file (@files) {
+		if (follow_link( { text => 'raw', url_abs_regex => qr/f=$file/ },
+				 "raw (blob_plain) entry for \"$file\" in tree view")) {
+			chomp(my $first_line = (`cat "$file"`)[0]);
+			$mech->content_contains(
+				$first_line, "blob_plain view contains first line of file");
+			$mech->back;
+		}
+	}
+}
+
+
+# Error handling
+# Pass valid and invalid paths to various file-based actions
+for my $action qw( blame blob blob_plain blame ) {
+	test_page("?p=.git;a=$action;f=$files[0];hb=HEAD",
+		  "$action: look up existent file");
+	test_page("?p=.git;a=$action;f=does_not_exist;hb=HEAD",
+		  "$action: look up non-existent file", 404);
+	TODO: {
+		local $TODO = 'wrong error code (but using Git::Repo will fix this)';
+		test_page("?p=.git;a=$action;f=$directories[0];hb=HEAD",
+			  "$action: look up directory", 400);
+	}
+}
+TODO: {
+	local $TODO = 'wrong error code (but using Git::Repo will fix this)';
+	test_page("?p=.git;a=tree;f=$files[0];hb=HEAD",
+		  'tree: look up existent file', 400);
+}
+# Pass valid and invalid paths to tree action
+test_page("?p=.git;a=tree;f=does_not_exist;hb=HEAD",
+	  'tree: look up non-existent file', 404);
+test_page("?p=.git;a=tree;f=$directories[0];hb=HEAD",
+	  'tree: look up directory');
+TODO: {
+	local $TODO = 'cannot use f=/ or f= for trees';
+	test_page("?p=.git;a=tree;f=/;hb=HEAD", 'tree: look up directory');
+}
+
+
+1;
+__END__
-- 
1.5.6.109.g33ded.dirty

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

[Jakub Narebski]

On Thu, 26 June 2008, Lea Wiemann wrote:

> This test uses Test::WWW::Mechanize::CGI to check gitweb's output.  It
> also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)

Wouldn't it be better to use "(each if present)"?

I am not a native English speaker, though.

[...]
> Changes since v7:
> 
> - In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets
>   source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.

Good idea.

> - .git/description in the test repository no longer depends on $0
>   (this would e.g. cause 'cd t; make' to fail).

> +cat >.git/description <<EOF
> +gitweb test repository
> +EOF

I'd rather use more descriptive name, so when you look it up in gitweb
to manually (visually?) checks its output you would know of which test
is it.  Something like "gitweb Mechanize test repository", or "test
repository for gitweb's Mechanize test".

> - Add test_link subroutine and use it everywhere in place of
>   ok(find_link...) so that links whose presence get tested get checked
>   and spidered in --long-tests mode.

Nice.

> - Add simple page caching (reduces execution time without --long-tests
>   by 25%).  That's *really* helpful when you have to run those tests
>   on a regular basis. ;)

What do you need caching for?  You check if page was already accessed
when spidering...

If it is about checking links, alternate solution would be to replace
simple $mech->page_links_ok( [ $desc ] ) by finding all the links
either using $mech->followable_links() or $mech->find_all_links( ... ),
or just $mech->links, then filtering out links which you have checked
already, then checking selected links using $mech->links_ok( $links [, $desc ] )

>   (WWW::Mechanize::Cached won't work with
>   TWM::CGI, so we have to implement it ourselves; but it's easier
>   anyway.)

> +package OurMechanize;
> +
> +use base qw( Test::WWW::Mechanize::CGI );

Why this package is not named WWW::Mechanize::CGI::Cached, I wonder?
Is it because of corrected cgi_application method?

> +
> +my %page_cache;
> +# Cache requests.
> +sub _make_request {
> +	my ($self, $request) = (shift, shift);
> +
> +	my $response;
> +	unless ($response = Storable::thaw($page_cache{$request->uri})) {
> +		$response = $self->SUPER::_make_request($request, @_);
> +		$page_cache{$request->uri} = Storable::freeze($response);
> +	}
> +	return $response;
> +}
> +
> +# Fix whitespace problem.
> +sub cgi_application {
> +	my ($self, $application) = @_;
> +
> +	# This subroutine was copied (and modified) from
> +	# WWW::Mechanize::CGI 0.3, which is licensed 'under the same
> +	# terms as perl itself' and thus GPL compatible.
> +	my $cgi = sub {
> +		# Use exec, not the shell, to support embedded
> +		# whitespace in the path to $application.
> +		# http://rt.cpan.org/Ticket/Display.html?id=36654
> +		my $status = system $application $application;
> +		my $value  = $status >> 8;
> +
> +		croak( qq/Failed to execute application '$application'. Reason: '$!'/ )
> +		    if ( $status == -1 );
> +		croak( qq/Application '$application' exited with value: $value/ )
> +		    if ( $value > 0 );
> +	};
> +
> +	$self->cgi($cgi);
> +}
> +

By the way, if you want to add a comment to mentioned WM::CGI ticket
http://rt.cpan.org/Ticket/Display.html?id=36654 you have to either
register, or send comment via mail with the following info

>From "Bugs in WWW-Mechanize-CGI via RT" <bug-WWW-Mechanize-CGI@rt.cpan.org>:
|
| Please include the string:
|
|         [rt.cpan.org #36654]
|
| in the subject line of all future correspondence about this issue. To do so, 
| you may reply to this message.

> - Follow redirects rather than failing.

Nice. Where it is?
 
> - Test subdirectories in tree view.
> - Test error handling for non-existent hashes or hashes of wrong type.
> - Test commitdiff_plain view.
> - Expand test for history view.
> - Test tag objects (not just symbolic tags) in tag list.

More tests are always nice to have, although I begin to wonder if
splitting this test into smaller part wouldn't be a good idea...

> - Test a specific bug (under "diff formatting", marked TODO).

> +# Diff formattting problem.

(One 't' too many: should be "Diff formatting problem")

> +if (get_summary &&
> +    follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&
> +    follow_link( { text => 'commitdiff' }, 'commitdiff')) {
> +	TODO: {
> +		local $TODO = "bad a/* link in diff";
> +		if (follow_link( { text_regex => qr!^a/! },
> +				 'a/* link (probably wrong)')) {
> +			# The page we land on here is broken already.
> +			follow_link( { url_abs_regex => qr/a=blob_plain/ },
> +				     'linked file name');  # bang
> +		}
> +	}
> +}
 
I'll try to investigate; I guess this uses wrong name or wrong hash
for preimage.

> - Do not test correctness of line number fragments (#l[0-9]+); they're
>   broken too often right now.

What do you mean by broken?

> - Probably some more minor improvements I've forgotten about. :)

For example

> +die "this must be run by calling the t/t*.sh shell script(s)\n"
> +    if Cwd->cwd !~ /trash directory$/;

 
Good work!

-- 
Jakub Narebski
Poland

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

[Lea Wiemann]

Jakub Narebski wrote:
> On Thu, 26 June 2008, Lea Wiemann wrote:
>> It also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
> 
> Wouldn't it be better to use "(each if present)"?
> I am not a native English speaker, though.

Me neither, but I prefer "if present, each" off the top of my head...

>> - Add simple page caching (reduces execution time without --long-tests
>>   by 25%).  That's *really* helpful when you have to run those tests
>>   on a regular basis. ;)
> 
> What do you need caching for?  You check if page was already accessed
> when spidering...

This is actually about the short tests (I don't think it gains much for
spidering, percentage-wise).  The test suite uses some repetitive set-up
code (like get_summary && follow_link 'tree' && ...), so the second and
third times these pages are loaded we can save some time.

> replace $mech->page_links_ok [with] finding all the links [...], 
> then filtering out links which you have checked already, then checking
> selected links using $mech->links_ok

That's basically what it does right now. :)

>> +package OurMechanize;
>> +use base qw( Test::WWW::Mechanize::CGI );
> 
> Why this package is not named WWW::Mechanize::CGI::Cached, I wonder?
> Is it because of corrected cgi_application method?

Yeah, basically. :)  Plus, it's not to be confused with a proper
implementation of a TWM::CGI::Cached package.  (For instance, it uses
the URL rather than the complete request as cache key.  Hence it ignores
headers like Referer; but that's great for the Gitweb tests since TWM
apparently sets the Referer, but Gitweb doesn't check it, so there's no
need to refetch a page because the Referer has changed.)

> By the way, if you want to add a comment to mentioned WM::CGI ticket [...]

Thanks; done.

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

> I begin to wonder if
> splitting this test into smaller part wouldn't be a good idea...

It's not yet painful (or long-running) enough for me to care. :)  I'm
fine with a 500-lines test module.

>> +# Diff formattting problem.
> 
> (One 't' too many:

Fixed.

>> +			follow_link( { url_abs_regex => qr/a=blob_plain/ },
>> +				     'linked file name');  # bang
>  
> I'll try to investigate; I guess this uses wrong name or wrong hash
> for preimage.

Thanks!

>> - 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. ;-)

(There are also some other [mostly minor] issues marked with "TODO:" in
the test code; I didn't want to swamp the list with half a dozen bug
reports though, apart from time constraints on my end.)

> Good work!

Thanks! :-)  I'll send v9 in (hopefully) 2-3 days, together with the
first working version of the caching code.

-- Lea

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

[Jakub Narebski]

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

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

[Lea Wiemann]

Jakub Narebski wrote:
> some time ago I have send a patch which converted support for
> links with hash and without action to use redirect instead of
> silently filling correct action based on type of object [...]
> IMHO it is better as it should prevent bookmarking "expensive" URL.

Haha, I'd actually suggest the opposite. ;-)  Figuring out the right
action is almost free since you have to fetch the object anyways, so I
doubt it'll make any difference performance-wise (though it'd be
interesting to benchmark this).  However, gitweb's URLs are
prohibitively long -- so that nobody uses them in email --, and
(automatically?) dropping the action parameter where possible would be a
good first step to shortening them.  Another idea would be to shorten
the hashes.

>> there are links to line numbers that don't exist
> 
> I wonder if those are intentional (or at least known) breaking, to form
> approximate blame file history browsing;

*nods*  I wasn't following this in detail; if it turns out to be
unfixable, we could also remove the fragment checks for line numbers
(rather than running them as "TODO:" tests).

> [Git::Commit:] you should not error out on unknown header in commit object,

Unless this can actually happen in practice, I'd rather die aggressively
-- it prevents errors (like cache hiccups) slipping through unnoticed.

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

[Jakub Narebski]

On Mon, 30 Jun 2008, Lea Wiemann wrote:
> Jakub Narebski wrote:
>>
>> some time ago I have send a patch which converted support for
>> links with hash and without action to use redirect instead of
>> silently filling correct action based on type of object [...]
>> IMHO it is better as it should prevent bookmarking "expensive" URL.
> 
> Haha, I'd actually suggest the opposite. ;-)  Figuring out the right
> action is almost free since you have to fetch the object anyways, so I
> doubt it'll make any difference performance-wise (though it'd be
> interesting to benchmark this).

It is one fork more[*1*], which doesn't matter on operating systems
like Linux where forking is fairly cheap.

But perhaps avoiding redirect would be better solution... well, yet
another solution would be to leave it up to gitweb configuration ;-)

[*1*] Or pre-fetching object (together with its type) which might be
large, into memory.

> However, gitweb's URLs are 
> prohibitively long -- so that nobody uses them in email --, and
> (automatically?) dropping the action parameter where possible would be a
> good first step to shortening them.  Another idea would be to shorten
> the hashes.

About shortening the hashes: _usually_ 8. characters is enough... but
sometimes it isn't.  What then?

[...]
>> [Git::Commit:] you should not error out on unknown header in commit object,
> 
> Unless this can actually happen in practice, I'd rather die aggressively
> -- it prevents errors (like cache hiccups) slipping through unnoticed.

If Git::Commit is to be *generic* object oriented interface to git
repository (and not only for gitweb), it has either to skip unknown
commit headers, or save them as-is.

As to checking cache: you can always check if header name is sane...
-- 
Jakub Narebski
Poland

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

[Jakub Narebski]

[back-to-list]

On Mon, 30 Jun 2008, Lea Wiemann wrote:
> [off-list]
[...]
 
> By the way, I found the supercat (spc) colorizer very helpful for the
> mechanize tests, since the output tends to be long and the lines wrap a
> lot.  Here are my regexes:
> 
> $ cat ~/.spcrc/spcrc-test_more
> #  HTML COLOR NAME   COL A N T STRING or REGULAR EXPRESSION
> #################### ### # # # #########################################
> Black                blk     r (.*)
> Green                grn b   r (^ok .*)
> Red                  red b   r (^not ok)
> Yellow               yel b   r (^not ok .* # TODO.*)
> Green                grn     r (^ok .*\[auto\].*)
> Red                  red     r (^not ok .*\[auto\].*)
> Yellow               yel     r (^not ok .*\[auto\].* # TODO.*)
> Black                blk     r ( - .*)
> Green                grn r   r (\* passed all .*)
> Red                  red r   r (\* failed .*)
> 
> Then you can just run
> 
> ./t9503-gitweb-Mechanize.sh -v -l 2>&1 | spc -t test_more
> 
> Hope it's useful!

Hmmm... it would be nice if 'test_external*' functions supported syntax
highlighting of command output if it follows TAP: Test Anything Protocol
http://www.perlfoundation.org/perl5/index.cgi?testing#tap_test_anything_protocol
in a manner similar to coloring native tests (using git_expect_success,
git_expect_failure etc.).

Test::More as far as I know produces TAP-compatibile output.

-- 
Jakub Narebski
Poland