threads / rfc / 13950

RFC patchgitweb: Use Test::WWW::Mechanize::CGI to test gitweb output

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

## tl;dr

41 messages between Jun 14, 2008 and Jun 30, 2008. Diffs are folded; open one to read it.

replies: 40people: 3as markdown or json

Jakub Narebski· Jun 14, 2008, 12:47 UTC · lore

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
Show changes to 2 files +218 −0

t/t9503-gitweb-Mechanize.sh, 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
Lea Wiemann· Jun 14, 2008, 14:40 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Jun 14, 2008, 18:07 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 10 quoted lines
> 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...).

Show 13 quoted lines
> 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.
Show 6 quoted lines
> > +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...

Show 8 quoted lines
> $ 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...

Show changes to t/t9503-gitweb-Mechanize.sh +2 −2
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
Lea Wiemann· Jun 14, 2008, 18:31 UTC · re: Jakub Narebski · lore

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

Jakub Narebski wrote:
Show 5 quoted lines
> 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
Jakub Narebski· Jun 14, 2008, 18:59 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 10 quoted lines
> 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.
Show 5 quoted lines
>                                             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).

Show 5 quoted lines
>> [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
Lea Wiemann· Jun 14, 2008, 21:12 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Jun 15, 2008, 08:36 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 9 quoted lines
> 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
Lea Wiemann· Jun 14, 2008, 18:18 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Jun 14, 2008, 18:31 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 11 quoted lines
> 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
Lea Wiemann· Jun 14, 2008, 23:57 UTC · re: Jakub Narebski · lore

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

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
Show changes to 2 files +218 −0

t/t9503-gitweb-Mechanize.sh, 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
Jakub Narebski· Jun 15, 2008, 18:01 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
> Changed since v1:
Show 8 quoted lines
> - 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
Lea Wiemann· Jun 15, 2008, 18:45 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Jun 16, 2008, 00:40 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 7 quoted lines
> 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.

Show 8 quoted lines
> > 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.

Show 11 quoted lines
> > 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
Lea Wiemann· Jun 16, 2008, 09:10 UTC · re: Jakub Narebski · lore

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

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
Jakub Narebski· Jun 16, 2008, 20:15 UTC · re: Lea Wiemann · lore

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

Lea Wiemann wrote:
Show 6 quoted lines
> 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.
 
Show 6 quoted lines
>>> 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
Lea Wiemann· Jun 20, 2008, 03:18 UTC · re: Lea Wiemann · lore

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

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
Show changes to 2 files +256 −0

t/t9503-gitweb-Mechanize.sh, 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
Jakub Narebski· Jun 20, 2008, 12:08 UTC · re: Lea Wiemann · lore

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

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.
Show 10 quoted lines
> +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.
Show 6 quoted lines
> +# 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
Lea Wiemann· Jun 20, 2008, 13:49 UTC · re: Jakub Narebski · lore

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

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.

Show 5 quoted lines
>> +# 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.

Show 5 quoted lines
>> +# 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
Jakub Narebski· Jun 20, 2008, 18:03 UTC · re: Lea Wiemann · lore

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

On Fri, 20 Jun 2008, Lea Wiemann wrote:
Show 7 quoted lines
> 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.

Show 9 quoted lines
>>> +# 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.
 
Show 6 quoted lines
>> 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").

Show 6 quoted lines
>>> +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).
 
Show 7 quoted lines
>> 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
Lea Wiemann· Jun 20, 2008, 22:04 UTC · re: Jakub Narebski · lore

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

Jakub Narebski wrote:
Show 6 quoted lines
> 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
Lea Wiemann· Jun 20, 2008, 22:18 UTC · re: Lea Wiemann · lore

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

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
Show changes to 2 files +256 −0

t/t9503-gitweb-Mechanize.sh, 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
Lea Wiemann· Jun 23, 2008, 00:45 UTC · re: Lea Wiemann · lore

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

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
Show changes to 3 files +488 −0

t/t9503-gitweb-Mechanize.sh, t/t9503/test.pl, t/test-lib.sh

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
Lea Wiemann· Jun 23, 2008, 01:14 UTC · re: Lea Wiemann · lore

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

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
Show changes to 3 files +488 −0

t/t9503-gitweb-Mechanize.sh, t/t9503/test.pl, t/test-lib.sh

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
Junio C Hamano· Jun 23, 2008, 02:30 UTC · re: Lea Wiemann · lore

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

Lea Wiemann <lewiemann@gmail.com> writes:
Show 13 quoted lines
> 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
Show 17 quoted lines
>  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.

Show 14 quoted lines
> +# 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.
Lea Wiemann· Jun 23, 2008, 07:00 UTC · re: Junio C Hamano · lore

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

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. ;-)
Show 6 quoted lines
>> +# 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
Jakub Narebski· Jun 23, 2008, 13:31 UTC · re: Lea Wiemann · lore

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

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.
Show 5 quoted lines
> +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?

Show 11 quoted lines
> +	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;"
Show 9 quoted lines
> +# 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
Lea Wiemann· Jun 23, 2008, 17:57 UTC · re: Jakub Narebski · lore

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

[Open issue about PERL_PATH at the bottom!]
Jakub Narebski wrote:
Show 6 quoted lines
> 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;

Show 6 quoted lines
>> +		# 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.

Show 7 quoted lines
>> +	# 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');
Show 6 quoted lines
>> [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
Jakub Narebski· Jun 23, 2008, 22:18 UTC · re: Lea Wiemann · lore

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

On Mon, 23 Jun 2008, Lea Wiemann wrote:
Show 10 quoted lines
> 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.
 
Show 6 quoted lines
>>> +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)...
 
Show 9 quoted lines
>>> +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/!! ;-)))
Show 6 quoted lines
>>> +# 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.
 
Show 12 quoted lines
>>> +		# 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).
Show 23 quoted lines
>>> +	# 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?
 
Show 10 quoted lines
>>> [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
Lea Wiemann· Jun 24, 2008, 02:01 UTC · re: Jakub Narebski · lore

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

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.

Show 6 quoted lines
>> 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
Lea Wiemann· Jun 24, 2008, 02:18 UTC · re: Lea Wiemann · lore

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

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
Show changes to 2 files +510 −0

t/t9503-gitweb-Mechanize.sh, 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
Lea Wiemann· Jun 26, 2008, 13:47 UTC · re: Lea Wiemann · lore

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

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
Show changes to 3 files +692 −0

Makefile, t/t9503-gitweb-Mechanize.sh, 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
Lea Wiemann· Jun 26, 2008, 13:48 UTC · re: Lea Wiemann · lore

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

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
Show changes to 3 files +692 −0

Makefile, t/t9503-gitweb-Mechanize.sh, 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
Jakub Narebski· Jun 29, 2008, 22:47 UTC · re: Lea Wiemann · lore

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

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?

Show 37 quoted lines
> +
> +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?
 
Show 5 quoted lines
> - 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")
Show 13 quoted lines
> +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
Lea Wiemann· Jun 29, 2008, 23:39 UTC · re: Jakub Narebski · lore

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

Jakub Narebski wrote:
Show 5 quoted lines
> 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...
Show 6 quoted lines
>> - 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. :)
Show 5 quoted lines
>> +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.
Show 5 quoted lines
>> +			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
Jakub Narebski· Jun 29, 2008, 23:56 UTC · re: Lea Wiemann · lore

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

On Mon, 30 June 2008, Lea Wiemann wrote:
Show 10 quoted lines
> 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.
Show 9 quoted lines
>>> - 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
Lea Wiemann· Jun 30, 2008, 00:30 UTC · re: Jakub Narebski · lore

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

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.

Jakub Narebski· Jun 30, 2008, 21:55 UTC · re: Lea Wiemann · lore

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

On Mon, 30 Jun 2008, Lea Wiemann wrote:
Show 11 quoted lines
> 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.

Show 5 quoted lines
> 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
Junio C Hamano· Jun 24, 2008, 04:20 UTC · re: Lea Wiemann · lore

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

Lea Wiemann <lewiemann@gmail.com> writes:
Show 12 quoted lines
>> 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.

Lea Wiemann· Jun 24, 2008, 08:37 UTC · re: Junio C Hamano · lore

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

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
Jakub Narebski· Jun 24, 2008, 09:23 UTC · re: Junio C Hamano · lore

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

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
Jakub Narebski· Jun 30, 2008, 22:01 UTC · lore

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

[back-to-list]
On Mon, 30 Jun 2008, Lea Wiemann wrote:
> [off-list]
[...]
 
Show 23 quoted lines
> 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

← back to recent threads