{"thread":{"id":"13950","subject":"[RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","startedAt":"2008-06-14T12:47:27Z","lastAt":"2008-06-30T22:01:42Z","messageCount":41,"participants":["Jakub Narebski","Lea Wiemann","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"79852","messageId":"200806141447.28052.jnareb@gmail.com","threadId":"13950","inReplyTo":null,"subject":"[RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-14T12:47:27Z","receivedAt":"2008-06-14T12:47:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"This test uses Perl module Test::WWW::Mechanize::CGI to check\ngitweb output, using HTML::Lint (if present) to validate HTML.\n\nSigned-off-by: Jakub Narebski \n---\nReview, review, review!\n\nThis test requires test_external_without_stderr in t/test-lib.sh by\nLea Wiemann, send to git mailing list in\n [PATCH] t/test-lib.sh: add test_external and test_external_without_stderr\n http://thread.gmane.org/gmane.comp.version-control.git/83415\n Message-Id: <1212276975-27428-1-git-send-email-LeWiemann@gmail.com>\nalso present in git://repo.or.cz/git/gitweb-caching.git since commit\n(currently) d28d31467db2ec5737948685ade281d5c0704a27.\n\nThis is very much work in progress; it uses Test::WWW::Mechanize::CGI \nPerl module (from CPAN), which makes writing tests checking form (but\nnot requiring exact details) of gitweb output. The test driver, i.e.\nt/t9503-gitweb-Mechanize.sh skips tests if they cannot be done; note\nthat it assumes that if one jas TWM::CGI module it also has all\nmodules it depends on (Test::WWW::Mechanize, HTTP::Request::AsCGI,...)\ninstalled.\n\nThe alternatives would be to redo work of TWM::CGI using standard or\nsimply more common modules (LWP::* modules, HTTP::Request::AsCGI), or\n(proposed during off-line discussion with Lea Wiemann) to create\nexpected output documents and diff (compare) literal output. \n\nThe second solution has two disadvantages I can see. First, it\nfreezes gitweb output format, making improvements more difficult.\nSecond, if there is change that invalidates some or all tests vectors,\nyou would have to regenerate those vectors from actual gitweb output\nwithout having any tests to check this output; it would be quite easy\nto put into test vectors errorneous output.\n\nOn the other hand current approach means much, much more tests.\nIt is also bit more challenging, because you have to find invariants\nof the output and test that they are fullfilled.\n\n\nNOTE: Currently test_external_without_stderr fails because when trying\nto access URL for non-existent commit gitweb writes to STDERR; it is\nnot necessarily a bug because it is not written to web server logs (at\nleast in the case of Apache2).\n\nNOTE!!!: RFC. Work in progress. YMMV. Posted here to have Lea got\nat least _some_ tests to check her GSoC work against, and have some\nexamples how such tests *might* be written (I have nearly next to no\nexperience in writing tests using Perl test infrastructure, and in\nusing Mechanize and Test::WWW::Mechanize::CGI). I would really\nappreciate comments from Perl experts here, or at least people with\nmore Perl experience than me.\n\n t/t9503-gitweb-Mechanize.sh | 127 +++++++++++++++++++++++++++++++++++++++++++\n t/t9503/test.pl | 91 +++++++++++++++++++++++++++++++\n 2 files changed, 218 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..5df22c4\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,127 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+#\n+\n+test_description='gitweb as CGI (using WWW::Mechanize)\n+\n+This test uses Perl module Test::WWW::Mechanize::CGI to\n+check gitweb output, using HTML::Lint to validate HTML.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+perl -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <catfile('..','..','gitweb','gitweb.perl');\n+# the followin two lines of code are workaround for bug in\n+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3\n+# (http://rt.cpan.org/Ticket/Display.html?id=36654)\n+# for pathnames with spaces (because of \"trash directory\")\n+$gitweb = File::Spec->rel2abs($gitweb);\n+$gitweb = Cwd::abs_path($gitweb);\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi_application($gitweb);\n+$mech->env(GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'});\n+\n+# import config, pedeclaring config variables\n+our $site_name = '';\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+my $pagename = '';\n+my $get_ok;\n+SKIP: {\n+\t$pagename = 'project list (implicit)';\n+\tskip \"Could not get $pagename\", 2 + $lint_installed\n+\t\tunless $mech->get_ok('http://localhost/', \"GET $pagename\");\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->title_like(qr!$site_name!,\n+\t\t'title contains $site_name');\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+$mech->get_ok('http://localhost/?p=.git',\n+\t'GET test repository summary (implicit)');\n+$mech->get_ok('http://localhost/.git',\n+\t'GET test repository summary (implicit, pathinfo)');\n+$get_ok = 0;\n+SKIP: {\n+\t$pagename = 'test repository summary (explicit)';\n+\t$get_ok = $mech->get_ok('http://localhost/?p=.git;a=summary',\n+\t\t\"GET $pagename\");\n+\tskip \"Could not get $pagename\", 1 + $lint_installed\n+\t\tunless $get_ok;\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t'title contains $site_name and \".git/summary\"');\n+}\n+\n+SKIP: {\n+\tskip \"Could not get starting page $pagename\", 2 + $lint_installed\n+\t\tunless $get_ok;\n+\t$pagename = 'search test repository (from search form)';\n+\t$get_ok = $mech->submit_form_ok(\n+\t\t{form_number=>1,\n+\t\t fields=> {'s' => 'Initial commit'}\n+\t\t},\n+\t\t\"submit search form (default)\");\n+\tskip \"Could not submit search form\", 1 + $lint_installed\n+\t\tunless $get_ok;\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->content_contains('Initial commit',\n+\t\t'content contains searched text');\n+}\n+\n+$pagename = 'non existent project';\n+$mech->get('http://localhost/?p=non-existent.git');\n+like($mech->status, qr/40[0-9]/, \"40x status response for $pagename\");\n+$mech->html_lint_ok('page validates') if $lint_installed;\n+\n+$pagename = 'non existent commit';\n+$mech->get('http://localhost/?p=.git;a=commit;h=non-existent');\n+like($mech->status, qr/40[0-9]/, \"40x status response for $pagename\");\n+$mech->html_lint_ok('page validates') if $lint_installed;\n+\n+1;\n+__END__\n-- \n1.5.5.3\n"},{"id":"79857","messageId":"4853D84D.5010303@gmail.com","threadId":"13950","inReplyTo":"200806141447.28052.jnareb@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-14T14:40:13Z","receivedAt":"2008-06-14T14:40:13Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> This test requires [PATCH] t/test-lib.sh: add test_external and test_external_without_stderr\n\nFor everyone's convenience, here's the raw patch: \nhttp://article.gmane.org/gmane.comp.version-control.git/83415/raw\n(When repo.or.cz is up again you can also locate a version with fixed \nwhitespace in my repo: )\n\n> This test uses Perl module Test::WWW::Mechanize::CGI to check\n> gitweb output, using HTML::Lint (if present) to validate HTML.\n\nAwesome, thank you so much for making a start here! Here are some quick \ncomments.\n\n> NOTE: Currently test_external_without_stderr fails because when trying\n> to access URL for non-existent commit gitweb writes to STDERR; it is\n> not necessarily a bug because it is not written to web server logs\n\nWithout having looked at the cause of that, I think that gitweb should \nnot be writing stuff to stderr unless an internal or serious error \noccurs; in particular trying to access invalid commits shouldn't cause \nmessages on stderr, only to log files if at all.\n\nThat said, as long as it isn't fixed, here's my workaround to \ntemporarily discard stderr (from my t/t9710/test.pl):\n\nour $old_stderr;\nsub discard_stderr {\n open our $old_stderr, \">&\", STDERR or die \"cannot save STDERR\";\n close STDERR;\n}\nsub restore_stderr {\n open STDERR, \">&\", $old_stderr or die \"cannot restore STDERR\";\n}\n\nIt works on Unix, but I don't know about other platforms.\n\n> +cat >gitweb_config.perl < [...]\n> +our \\$GIT = \"git\";\n\nt9500 seems to be doing the same(?) thing, but this somehow doesn't work \nwith your t9503 test:\n\n$ git # no git in PATH to make sure it picks up the right git binary\nbash: git: command not found\n$ ./t9500-gitweb-standalone-no-errors.sh | grep passed\n* passed all 75 test(s)\n$ ./t9503-gitweb-Mechanize.sh -v\n[...]\n\tgitweb.perl: Can't exec \"git\": No such file or directory at \n/home/lea/source/git/fresh-git/gitweb/gitweb.perl line 380.\n\n-- Lea\n"},{"id":"79867","messageId":"200806142007.35288.jnareb@gmail.com","threadId":"13950","inReplyTo":"4853D84D.5010303@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-14T18:07:34Z","receivedAt":"2008-06-14T18:07:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n> >\n> > NOTE: Currently test_external_without_stderr fails because when trying\n> > to access URL for non-existent commit gitweb writes to STDERR; it is\n> > not necessarily a bug because it is not written to web server logs\n> \n> Without having looked at the cause of that, I think that gitweb should \n> not be writing stuff to stderr unless an internal or serious error \n> occurs; in particular trying to access invalid commits shouldn't cause \n> messages on stderr, only to log files if at all.\n\nActually it isn't gitweb (or Perl) writing to stderr, but git itself.\nSomehow, at least for gitweb run as CGI script (and under legacy\nmod_perl) with Apache 2 as web server this error message:\n fatal: bad revision 'non-existent'\ndoesn't land in web server logs (/var/log/httpd/error_log). So it lands\nin /dev/null when running gitweb as web script, so it was deemed not\nimportant (also fixing this is not very easy, as you can read below).\n\nt/t9500-gitweb-standalone-no-errors.sh considers as errors only those\nerror message which would make it into web server logs, see gitweb_run()\nfunction there. This catches compilation errors.\n\n\nFixing this is not that simple. There is no option to git-rev-list\nto not write any output to stderr (no, '--quiet' is about something\nelse), and I'd rather not lose all advantages of list (shell-less)\nform of magical \"|-\" open only for \"2>/dev/null\" redirection as\nin git_object subroutine in gitweb.\n\nPerhaps it could be solved in Git.pm, and when gitweb is rewritten\nto use \"use Git\" (and global $repo object instead of global $git_dir\nvariable) it would automatically fix it (using Git.pm would have\nthe advantage of making gitweb more portable, I think - to ActiveState\nbroken Perl implementation, with broken magic \"|-\" open).\n\nOr git-rev-list, or even git wrapper itself, could acquire option to\nredirect all stderr to dev null... I think adding it in git wrapper\nwould be even better; simply change \"warning\" and \"die\" to null\nfunctions (I was even thinking about doing that...).\n\n> That said, as long as it isn't fixed, here's my workaround to \n> temporarily discard stderr (from my t/t9710/test.pl):\n> \n> our $old_stderr;\n> sub discard_stderr {\n> open our $old_stderr, \">&\", STDERR or die \"cannot save STDERR\";\n> close STDERR;\n> }\n> sub restore_stderr {\n> open STDERR, \">&\", $old_stderr or die \"cannot restore STDERR\";\n> }\n> \n> It works on Unix, but I don't know about other platforms.\n\nThanks.\n\n> > +cat >gitweb_config.perl < > [...]\n> > +our \\$GIT = \"git\";\n> \n> t9500 seems to be doing the same(?) thing, but this somehow doesn't work \n> with your t9503 test:\n\nIt should work. test-lib.sh sets up $PATH to have 'git' binary (just\ncompiled git binary) in it...\n\n> $ git # no git in PATH to make sure it picks up the right git binary\n> bash: git: command not found\n> $ ./t9500-gitweb-standalone-no-errors.sh | grep passed\n> * passed all 75 test(s)\n> $ ./t9503-gitweb-Mechanize.sh -v\n> [...]\n> \tgitweb.perl: Can't exec \"git\": No such file or directory at \n> /home/lea/source/git/fresh-git/gitweb/gitweb.perl line 380.\n\n...and it would be very strange for t9500 to pass, but t9503 do not\npass. (Of course both tests passes at my computer, otherwise\nI wouldn't send this patch in current form).\n\nHmmm... perhaps $PATH doesn't get passed down... strange.\n\n\nBut thanks to your report I have found bug in gitweb. I have changed\nt/t9503-gitweb-Mechanize.sh... \n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nindex 5df22c4..a5be275 100755\n--- a/t/t9503-gitweb-Mechanize.sh\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -92,7 +92,7 @@ cat >gitweb_config.perl <gitweb_config.perl < +\tskip \"Could not get $pagename\", 2 + $lint_installed\n> +\t\tunless $mech->get_ok('http://localhost/', \"GET $pagename\");\n\nJust noticed this -- here's the cause of the problem I encountered in my \nprevious post.\n\nYou can't really probe for a running gitweb installation on localhost, \nbecause even if there's one, you don't know if you're testing against \nyour tree and your test repo.\n\nSo unless it's (easily) possible to force Mechanize to eat gitweb.cgi's \noutput in lieu of making actual HTTP requests, I'd suggest that we \nsimply use an environment variable (e.g. \nGITWEB_TEST_BASE_URL=http://localhost/cgi-bin/gitweb.cgi) -- if it's \nunset, the tests get skipped, otherwise they're run against that base URL.\n\n*wanders-off-to-write-a-patch*\n\n-- Lea\n"},{"id":"79874","messageId":"48540E70.4030507@gmail.com","threadId":"13950","inReplyTo":"200806142007.35288.jnareb@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-14T18:31:12Z","receivedAt":"2008-06-14T18:31:12Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> Lea Wiemann wrote:\n>> I think that gitweb should not be writing stuff to stderr unless an\n>> internal or serious error occurs\n>\n> There is no option to git-rev-list to not write any output to stderr\n\nOkay, this one will go away with the new API I'm writing, which uses \ncat-file --batch-check instead of rev-list. In the meantime (and in \nother cases) I guess diverting stderr in the test code is fine. (I \nwouldn't want to ignore stderr in all cases, even where you're not \nexpecting any output on stderr, since that might actually indicate an \nerror.)\n\n> [snip] It should work. test-lib.sh sets up $PATH to have 'git' binary\n> (just compiled git binary) in it...\n\nSince you're accessing http://localhost/ URLs, the web server's PATH is \nin effect, which doesn't get overridden by the tests. But anyways, \nusing an environment variable (see my other email) will move the \nresponsibility for this to the developer running the tests.\n\n> I have changed t/t9503-gitweb-Mechanize.sh... \n> \n> -our \\$GIT = \"git\";\n> +our \\$GIT = \"$safe_pwd/../../git\";\n\nFor t9503 this'll go away if the GITWEB_TEST_BASE_URL thing is \nimplemented. For t9500 (which contains $GIT = \"git\" as well), it should \n be fine as is since test-lib.sh sets PATH and thus gitweb picks up the \nright binary.\n\n-- Lea\n"},{"id":"79875","messageId":"200806142031.46556.jnareb@gmail.com","threadId":"13950","inReplyTo":"48540B68.1050108@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-14T18:31:46Z","receivedAt":"2008-06-14T18:31:46Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n> >\n> > +\tskip \"Could not get $pagename\", 2 + $lint_installed\n> > +\t\tunless $mech->get_ok('http://localhost/', \"GET $pagename\");\n> \n> Just noticed this -- here's the cause of the problem I encountered in my \n> previous post.\n> \n> You can't really probe for a running gitweb installation on localhost, \n> because even if there's one, you don't know if you're testing against \n> your tree and your test repo.\n\nErrr... WWW::Mechanize::CGI does not access web server, but runs CGI\nscript (or CGI subroutine) as if it was installed in the root directory\nof localhost. 'http://localhost' simply means installed CGI script.\n\nFor example tests works for me, even though I have gitweb installed\nsomewhere deeper than directly at locahost:\n http://localhost/cgi-bin/gitweb/gitweb.cgi\nBesides you can check output and see that it is consistent with set\nand used gitweb config in t9503* test.\n\nTest::WWW::Mechanize::CGI has the following description:\nhttp://search.cpan.org/~mramberg/Test-WWW-Mechanize-CGI-0.1/lib/Test/WWW/Mechanize/CGI.pm\n\n Provides a convenient way of testing CGI applications without a external\n daemon.\n\nThe documentation of module (0.1 in the case of TWM::CGI, 0.3 in the\ncase of WM::CGI) leaves something to be desired, so I mainly used\nexamples provided as guideline.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"79877","messageId":"200806142059.52373.jnareb@gmail.com","threadId":"13950","inReplyTo":"48540E70.4030507@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-14T18:59:52Z","receivedAt":"2008-06-14T18:59:52Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>> Lea Wiemann wrote:\n>>\n>>> I think that gitweb should not be writing stuff to stderr unless an\n>>> internal or serious error occurs\n>>\n>> There is no option to git-rev-list to not write any output to stderr\n> \n> Okay, this one will go away with the new API I'm writing, which uses \n> cat-file --batch-check instead of rev-list.\n\nWon't work. Well, it would work for p=commit;h=non-existent, but it\nwould not work for p=log;h=non-existent, where git-rev-list\n(or git-log) is really needed. And I'd rather have parse_commit()\n(used in 'commit' view) and parse_commits() (used in log-like views)\nshare the same commit parser, parse_commit_text(), so it's rev-list\nalso for git_commit, not cat-file.\n\nBut using git-cat-file --batch-check would improve other cases.\n\n> In the meantime (and in \n> other cases) I guess diverting stderr in the test code is fine. (I \n> wouldn't want to ignore stderr in all cases, even where you're not \n> expecting any output on stderr, since that might actually indicate an \n> error.)\n\nWell, I'd divert stderr in tests cases (or use simply 'test_external'),\nor better filter stderr, only in those cases where there is spurious\nbut not dangerous thing on stderr.\n\nBut again, I think the solution would be either to add feature to\nGit.pm, something like command_output_pipe_no_stderr, which would\nredirect stderr to /dev/null, or modify git wrapper to redirect\nstderr to /dev/null in scripts / when calling script, and redefine\ndie and warn for built-ins (or close stderr).\n\n>> [snip] It should work. test-lib.sh sets up $PATH to have 'git' binary\n>> (just compiled git binary) in it...\n> \n> Since you're accessing http://localhost/ URLs, the web server's PATH is \n> in effect,...\n\nIt isn't. http://localhost/ is just access convention, and\nWWW::Mechanize::CGI actually calls system()[*1*] on provided path made\nabsolute (hence error when it contains whitespace), setting CGI\nenvironmental variables before calling it.\n\n[*1*] it would be nice to have perl_application in WWW::Mechanize::CGI,\nwhich would simply setup %ENV and use do() instead of system() on\nprovided application. Perhaps it would be better in meantime to\nsimply craft $mech->cgi( sub { ... } ), and do not use generic\n$mech->cgi_application($path); we could do without all the checking,\ntoo.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"79883","messageId":"4854343B.40707@gmail.com","threadId":"13950","inReplyTo":"200806142059.52373.jnareb@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-14T21:12:27Z","receivedAt":"2008-06-14T21:12:27Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> Well, I'd divert stderr in tests cases (or use simply 'test_external'),\n> or better filter stderr, only in those cases where there is spurious\n> but not dangerous thing on stderr.\n\nSince you can only run whole perl scripts with test_external, we'll have \nto resort to using those discard_/restore_stderr functions. I doubt \nfiltering for specific messages is worth the effort.\n\n> But again, I think the solution would be either to add feature to\n> Git.pm, something like command_output_pipe_no_stderr, which would\n> redirect stderr to /dev/null,\n\n(FYI, Git.pm has such a feature in the command method, though it it \nsimply closes STDERR and doesn't properly re-open it, I believe.) The \nGit::Repo API I'm writing doesn't currently generate any spurious output \non stderr, but if it ever does I'll make sure it gets discarded.\n\n>> Since you're accessing http://localhost/ URLs, the web server's PATH is \n>> in effect,...\n> \n> It isn't. http://localhost/ is just access convention,\n\nYup, I should've read TFM. :) On my system, $PATH is empty in \ngitweb.cgi for some strange reason, but since using an absolute path for \n$GIT works, I won't track it down for now.\n\n> [*1*] it would be nice to have perl_application in WWW::Mechanize::CGI,\n> which would simply setup %ENV and use do() instead of system() on\n> provided application.\n\nGitweb and probably CGI::Carp qw(fatalsToBrowser) use 'exit', so we'd \nhave to use Test::Trap (or so) to catch those. I think we should defer \nthis until performance actually becomes an issue.\n\n-- Lea\n"},{"id":"79894","messageId":"1213487844-685-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"200806141447.28052.jnareb@gmail.com","subject":"[RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-14T23:57:24Z","receivedAt":"2008-06-14T23:57:24Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"From: Jakub Narebski \n\nThis test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint (if present) to validate the HTML.\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\nThis requires the following two patches:\n[PATCH] gitweb: Make it work with $GIT containing spaces\n http://article.gmane.org/gmane.comp.version-control.git/85036/raw\n[PATCH] t/test-lib.sh: add test_external and test_external_without_stderr\n http://article.gmane.org/gmane.comp.version-control.git/83415/raw\n\nChanged since v1:\n\n- Use \"$GIT_EXEC_PATH/git\" instead of \"git\" for the git binary path,\n since on my system $PATH gets lost mysteriously.\n\n- Use test_external instead of test_external_without_stderr so that\n the test doesn't fail if there is output on stderr.\n\n Discarding and restoring STDERR (as I mentioned in a recent message)\n doesn't work with Mechanize, as Mechanize somehow seems to save the\n STDERR handle and reuse it, so subsequent calls to Mechanize keep\n using the discarded STDERR. (The discard/restore functions work\n fine in other test modules, it's really just Mechanize here.)\n\n Using test_external means that in verbose mode we get some spurious\n error messages in the test output, but they're not harmful (and it\n should be clear to anyone running the test suite that they're OK).\n\n- Reworded commit message a little for clarity.\n\nThe test works now on my system. If you run this successfully (or\nunsuccessfully), please post a note on the list.\nComments/patches/additions, in particular from people who've used\nMechanize before, is still much appreciated!\n\nThis patch is WIP and not for inclusion. At some point we'll take\nfurther revisions off the list and use a public repository instead\n(once repo.or.cz is up again); we'll post a notice when that happens.\nWhen the test suite is finished we'll post it here again.\n\n t/t9503-gitweb-Mechanize.sh | 127 +++++++++++++++++++++++++++++++++++++++++++\n t/t9503/test.pl | 91 +++++++++++++++++++++++++++++++\n 2 files changed, 218 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..8720aea\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,127 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+#\n+\n+test_description='gitweb as CGI (using WWW::Mechanize)\n+\n+This test uses Perl module Test::WWW::Mechanize::CGI to\n+check gitweb output, using HTML::Lint to validate HTML.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+perl -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <catfile('..','..','gitweb','gitweb.perl');\n+# the followin two lines of code are workaround for bug in\n+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3\n+# (http://rt.cpan.org/Ticket/Display.html?id=36654)\n+# for pathnames with spaces (because of \"trash directory\")\n+$gitweb = File::Spec->rel2abs($gitweb);\n+$gitweb = Cwd::abs_path($gitweb);\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi_application($gitweb);\n+$mech->env(GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'});\n+\n+# import config, pedeclaring config variables\n+our $site_name = '';\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+my $pagename = '';\n+my $get_ok;\n+SKIP: {\n+\t$pagename = 'project list (implicit)';\n+\tskip \"Could not get $pagename\", 2 + $lint_installed\n+\t\tunless $mech->get_ok('http://localhost/', \"GET $pagename\");\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->title_like(qr!$site_name!,\n+\t\t'title contains $site_name');\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+$mech->get_ok('http://localhost/?p=.git',\n+\t'GET test repository summary (implicit)');\n+$mech->get_ok('http://localhost/.git',\n+\t'GET test repository summary (implicit, pathinfo)');\n+$get_ok = 0;\n+SKIP: {\n+\t$pagename = 'test repository summary (explicit)';\n+\t$get_ok = $mech->get_ok('http://localhost/?p=.git;a=summary',\n+\t\t\"GET $pagename\");\n+\tskip \"Could not get $pagename\", 1 + $lint_installed\n+\t\tunless $get_ok;\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t'title contains $site_name and \".git/summary\"');\n+}\n+\n+SKIP: {\n+\tskip \"Could not get starting page $pagename\", 2 + $lint_installed\n+\t\tunless $get_ok;\n+\t$pagename = 'search test repository (from search form)';\n+\t$get_ok = $mech->submit_form_ok(\n+\t\t{form_number=>1,\n+\t\t fields=> {'s' => 'Initial commit'}\n+\t\t},\n+\t\t\"submit search form (default)\");\n+\tskip \"Could not submit search form\", 1 + $lint_installed\n+\t\tunless $get_ok;\n+\t$mech->html_lint_ok('page validates') if $lint_installed;\n+\t$mech->content_contains('Initial commit',\n+\t\t'content contains searched text');\n+}\n+\n+$pagename = 'non existent project';\n+$mech->get('http://localhost/?p=non-existent.git');\n+like($mech->status, qr/40[0-9]/, \"40x status response for $pagename\");\n+$mech->html_lint_ok('page validates') if $lint_installed;\n+\n+$pagename = 'non existent commit';\n+$mech->get('http://localhost/?p=.git;a=commit;h=non-existent');\n+like($mech->status, qr/40[0-9]/, \"40x status response for $pagename\");\n+$mech->html_lint_ok('page validates') if $lint_installed;\n+\n+1;\n+__END__\n-- \n1.5.6.rc2.51.g4d03a.dirty\n"},{"id":"79910","messageId":"200806151036.22205.jnareb@gmail.com","threadId":"13950","inReplyTo":"4854343B.40707@gmail.com","subject":"Re: [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-15T08:36:21Z","receivedAt":"2008-06-15T08:36:21Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>>\n>> [*1*] it would be nice to have perl_application in WWW::Mechanize::CGI,\n>> which would simply setup %ENV and use do() instead of system() on\n>> provided application.\n> \n> Gitweb and probably CGI::Carp qw(fatalsToBrowser) use 'exit', so we'd \n> have to use Test::Trap (or so) to catch those. I think we should defer \n> this until performance actually becomes an issue.\n\nThe idea was more to avoid bug in cgi_application() method of WM::CGI,\nwhich fails on pathname containing embedded spaces (that is why\nworkaround in setting $gitweb path, to avoid 'trash directory' in it),\nthan for performance reasons.\n\nNevertheless naive $mech->cgi( sub { do \"$gitweb\"; } ); doesn't work;\nand after thinking about it a little it simply couldn't work...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"79944","messageId":"200806152001.34350.jnareb@gmail.com","threadId":"13950","inReplyTo":"1213487844-685-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-15T18:01:34Z","receivedAt":"2008-06-15T18:01:34Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n\n> Changed since v1:\n\n> - Use test_external instead of test_external_without_stderr so that\n> the test doesn't fail if there is output on stderr.\n> \n> Discarding and restoring STDERR (proposed in a recent message)\n> doesn't work with Mechanize, as Mechanize somehow seems to save the\n> STDERR handle and reuse it, so subsequent calls to Mechanize keep\n> using the discarded STDERR. (The discard/restore functions work\n> fine in other test modules, it's really just Mechanize here.)\n\nActually I think it is CGI module itself catching and redirecting\nSTDERR from Perl to logfile, and WWW::Mechanize::CGI having to catch\nand redirect all stderr of invoked application, but I'm not sure.\n\n\nWhat should we think about now, I guess, is\n1.) Should we put all tests in one file, or should they be split\n according to what do they test? For example separate tests\n for correct 4xx and 5xx responses for requests for non-exiting\n objects, and for not permitted views.\n\n2.) What invariants should we test, beside obvious HTML validation\n using HTML::Lint (by the way, is there some Perl module for RSS,\n Atom and OPML validation?). Checking for example if all items\n are listed in a 'tree' view, or if all inner links (#link) are\n valid would be a good start... pity that Mechanize modules don't\n have very good documetation.\n\n3.) What invariants you want to test for your caching efforts, e.g.\n checking (not necessary with TWM::CGI) if cached output matches\n non-cached, with exception of marking output as cached?\n\n-- \nJakub Narebski\nPoland\n"},{"id":"79947","messageId":"4855634C.8060606@gmail.com","threadId":"13950","inReplyTo":"200806152001.34350.jnareb@gmail.com","subject":"Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-15T18:45:32Z","receivedAt":"2008-06-15T18:45:32Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> 1.) Should we put all tests in one file, or should they be split\n\nI'd suggest we leave it in a single file until test execution time \nbecomes an issue. Then (when it has become too large) we'll be able to \nfigure out good boundaries along which to split the test suite.\n\n> 2.) What invariants should we test [...] Checking for example if all items\n> are listed in a 'tree' view, or if all inner links (#link) are\n> valid would be a good start... \n\nYup; completeness of item lists is especially relevant for paginated \noutput. Also check for the presence and validity of links (like \n\"parent\" links, etc.), and for the presence of certain elements (like \nthe file modes in the tree view).\n\nAlso, with a $ENV{LONG_GIT_TEST} variable or so, we could automatically \nvalidate all links for each page we're checking -- it takes a long time, \nbut it's still way more efficient than exhaustive spidering of the whole \nsite.\n\n> (by the way, is there some Perl module for RSS, Atom and OPML validation?)\n\nI can't find anything on Google right now, but piping them into external \nvalidators might be just as fine. Also, since those formats are \ngenerated using print statements (which is really error-prone for XML \nformats), I'd say that a good start would be to check for XML validity.\n\n> 3.) What invariants you want to test for your caching efforts, e.g.\n> checking if cached output matches non-cached\n\nHow about this:\n\n1. Run the Mechanize tests (and possibly also the existing t9500 tests) \n*without* caching, recording the URL's and contents of all pages the \ntest suite accesses.\n\n2. Get all those URL's again *with* caching (from a cold cache), and \nassert that the output is identical.\n\n3. Get all those URL's again *with* caching (from a warm cache), and \nassert that the output is identical. Perhaps also assert that no call \nto the git binary is made (i.e. everything has actually been cached). \n(Of course we might need options for the production site to not cache \ncertain things, but let's defer this discussion.)\n\n-- Lea\n"},{"id":"79995","messageId":"200806160240.18539.jnareb@gmail.com","threadId":"13950","inReplyTo":"4855634C.8060606@gmail.com","subject":"Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-16T00:40:17Z","receivedAt":"2008-06-16T00:40:17Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n> >\n> > 1.) Should we put all tests in one file, or should they be split\n> \n> I'd suggest we leave it in a single file until test execution time \n> becomes an issue. Then (when it has become too large) we'll be able to \n> figure out good boundaries along which to split the test suite.\n\nI wanted to split tests mainly not because of performance, but because\nof making it easier to maintain. Although perhaps single driver-test,\nand do()-ing or require()-ing sub-files would be enough.\n\n> > 2.) What invariants should we test [...] Checking for example if all items\n> > are listed in a 'tree' view, or if all inner links (#link) are\n> > valid would be a good start... \n> \n> Yup; completeness of item lists is especially relevant for paginated \n> output. Also check for the presence and validity of links (like \n> \"parent\" links, etc.), and for the presence of certain elements (like \n> the file modes in the tree view).\n\nFor example if \"next\" (and like) views really lead to next page.\n\n> Also, with a $ENV{LONG_GIT_TEST} variable or so, we could automatically \n> validate all links for each page we're checking -- it takes a long time, \n> but it's still way more efficient than exhaustive spidering of the whole \n> site.\n\nGood idea. I would examine how it is done in other tests.\n\n> > (by the way, is there some Perl module for RSS, Atom and OPML validation?)\n> \n> I can't find anything on Google right now, \n\nI usually search CPAN first, not Google...\n\n> but piping them into external \n> validators might be just as fine. Also, since those formats are \n> generated using print statements (which is really error-prone for XML \n> formats), I'd say that a good start would be to check for XML validity.\n\nWe can use Test::XML / Test::XML::Valid / Test::XML::Simple for being\nwell-formed XML. If RSS / Atom / OPML have good DTD / XML Schema /\n/ Relax-NG schema / Sablotron rules, they could be checked using that\nfrom Perl.\n\n> > 3.) What invariants you want to test for your caching efforts, e.g.\n> > checking if cached output matches non-cached\n> \n> How about this:\n> \n> 1. Run the Mechanize tests (and possibly also the existing t9500 tests) \n> *without* caching, recording the URL's and contents of all pages the \n> test suite accesses.\n> \n> 2. Get all those URL's again *with* caching (from a cold cache), and \n> assert that the output is identical.\n\nHow would you ensure cold cache?\n\n> 3. Get all those URL's again *with* caching (from a warm cache), and \n> assert that the output is identical.\n\nWell, it might be identical, but it also might have \"cached output\"\nsomewhere in the output.\n\n> Perhaps also assert that no call \n> to the git binary is made (i.e. everything has actually been cached). \n> (Of course we might need options for the production site to not cache \n> certain things, but let's defer this discussion.)\n\nOr at least (if we don't cache everything, and that could be good idea)\nto check if there are less git binary calls.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"80036","messageId":"48562DFD.4090108@gmail.com","threadId":"13950","inReplyTo":"200806160240.18539.jnareb@gmail.com","subject":"Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-16T09:10:21Z","receivedAt":"2008-06-16T09:10:21Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> How would you ensure cold cache?\n\nUse a unique key prefix, like \"gw:test-$PID:\"\n\n>> 3. Get all those URL's again *with* caching (from a warm cache), and \n>> assert that the output is identical.\n> \n> it might have \"cached output\" somewhere in the output.\n\nWe can make that deactivatable so that the output is actually identical.\n\n-- Lea\n"},{"id":"80065","messageId":"200806162215.05283.jnareb@gmail.com","threadId":"13950","inReplyTo":"48562DFD.4090108@gmail.com","subject":"Re: [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-16T20:15:05Z","receivedAt":"2008-06-16T20:15:05Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>> Lea Wiemann wrote:\n>>\n>> How would you ensure cold cache?\n> \n> Use a unique key prefix, like \"gw:test-$PID:\"\n\nI was thinking rather about dropping cache somewhat.\n \n>>> 3. Get all those URL's again *with* caching (from a warm cache), and \n>>> assert that the output is identical.\n>> \n>> it might have \"cached output\" somewhere in the output.\n> \n> We can make that deactivatable so that the output is actually identical.\n\nGood idea. Or we can just do diff and check the difference.\n\nBTW. another thing worth considering: generating \"Generating page...\"\nor something like that like kernel.org's gitweb does when it has to\nregenerate cache. Nice feature, that is.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"80434","messageId":"1213931911-15860-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"1213487844-685-1-git-send-email-LeWiemann@gmail.com","subject":"[WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-20T03:18:31Z","receivedAt":"2008-06-20T03:18:31Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"From: Jakub Narebski \n\nThis test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint (if present) to validate the HTML.\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\nI haven't gotten around to merging Jakub's recent XML validation patch\nyet, but I think it'd be good to have some review now; I'll merge it\ntomorrow and send another patch.\n\nChanges since v2:\n\nt9503-gitweb-Mechanize.sh has stayed the same, but t9503/test.pl has\nbeen overhauled: extracted common code into helper functions, only\nvalidate if --long-tests if given, added link-checking with\n--long-tests, added some minor tests for page contents, and added\ntests for the summary page at the bottom.\n\nThis runs on top of the *next* branch plus the following patches:\n\n[PATCH] gitweb: respect $GITPERLLIB\n http://article.gmane.org/gmane.comp.version-control.git/85586/raw\n[PATCH 1/2 v3] t/test-lib.sh: add test_external and test_external_without_stderr\n http://article.gmane.org/gmane.comp.version-control.git/85504/raw\n[PATCH v3] gitweb: standarize HTTP status codes\n http://article.gmane.org/gmane.comp.version-control.git/85522/raw\n\n(The latter two patches might be in next already by the time you're\nreading this.)\n\n\n t/t9503-gitweb-Mechanize.sh | 128 +++++++++++++++++++++++++++++++++++++++++++\n t/t9503/test.pl | 128 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 256 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..abcf987\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,128 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+test_description='gitweb as CGI (using WWW::Mechanize)\n+\n+This test uses Perl module Test::WWW::Mechanize::CGI to\n+check gitweb output, using HTML::Lint to validate HTML.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+perl -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <catfile('..','..','gitweb','gitweb.cgi');\n+# the followin two lines of code are workaround for bug in\n+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3\n+# (http://rt.cpan.org/Ticket/Display.html?id=36654)\n+# for pathnames with spaces (because of \"trash directory\")\n+$gitweb = File::Spec->rel2abs($gitweb);\n+$gitweb = Cwd::abs_path($gitweb);\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi_application($gitweb);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV, GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'}, $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name = '';\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+our %checked_pages;\n+# Validate and spider the current page, if --long-tests (-l) is given.\n+sub check_page {\n+\tmy $uri = $mech->uri;\n+\tif (not $checked_pages{$uri}) {\n+\t\t$mech->html_lint_ok('validate') or return 0 if $long_tests && $use_lint;\n+\t\t$mech->page_links_ok(\"check links on $uri\") if $long_tests;\n+\t\t$checked_pages{$uri} = 1;\n+\t}\n+\treturn 1\n+}\n+\n+our $baseurl = \"http://localhost\";\n+our($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page '?p=.git;a=summary', 'repository summary') {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened (and if --long-tests is given that\n+# it validates and has valid links), and return true if it does. Also\n+# set the global variables $params, $pagedesc, and $url for use in the\n+# if block. Optionally pass a third parameter $status to test the\n+# HTTP status code of the page (useful for error pages).\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t$url = \"$baseurl$params\";\n+\tif($status) {\n+\t\t$mech->get($url);\n+\t} else {\n+\t\t$mech->get_ok($url, \"GET \\\"$params\\\" -- $pagedesc\") or return 0;\n+\t}\n+\tcheck_page or return 0;\n+\tif($status) {\n+\t\treturn is($mech->status, $status, \"GET \\\"$params\\\" -- $pagedesc -- yields $status\");\n+\t} else {\n+\t\treturn 1;\n+\t}\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+$mech->get_ok('?p=.git', 'get repository summary');\n+if ($mech->submit_form_ok( { form_number => 1,\n+\t\t\t fields => { 's' => 'Initial' }\n+\t\t\t }, \"submit search form (default)\")) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains searched commit');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+# Summary page.\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+$mech->get_ok('?p=.git', 'get repository summary');\n+for my $revision (@revisions[-3..-1]) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\tok( $mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), \"$link_text link for $revision\" );\n+\t}\n+}\n+\n+\n+1;\n+__END__\n-- \n1.5.6.32.gad63a.dirty\n"},{"id":"80467","messageId":"200806201408.05254.jnareb@gmail.com","threadId":"13950","inReplyTo":"1213931911-15860-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-20T12:08:04Z","receivedAt":"2008-06-20T12:08:04Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 20 Jun 2008, Lea Wiemann wrote:\n\n> From: Jakub Narebski \n\nIf you have rewritten the patch I won't mind if you change authorship\nto you (remove this From: line); signoff is IMHO enough.\n\n> ---\n> I haven't gotten around to merging Jakub's recent XML validation patch\n> yet, but I think it'd be good to have some review now; I'll merge it\n> tomorrow and send another patch.\n\nFirst, it is not XML validation[*1*], but check if XML is well-formed.\nSecond, I think it would be better if adding XML checks (RSS, Atom,\nOPML) would be left as separate commit.\n \n[*1*] There is W3C feed validation service, http://validator.w3.org/feed/\nwhich provides also standalone Python script for feed validation, and\nalso SOAP interface with WebService::Validator::Feed::W3C wrapper (but\nit would require having Internet connection), or we can try validate\nagainst XML Schema Definition (there exists one for RSS, and there is\none for Atom derived from Relax-NG schema).\n\n> diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\n[...]\n> +# set up test repository\n> +test_expect_success 'set up test repository' '\n[cut]\n\nI have created this part as a copy of older t9500 gitweb test, thinking\nabout what we might want to test, but the WIP of Mechanize based t9503\ndoesn't have yet tests for those specific features.\n\n> diff --git a/t/t9503/test.pl b/t/t9503/test.pl\n[...]\n> +# We don't count properly when skipping, so no_plan is necessary.\n> +use Test::More qw(no_plan);\n\nActually it is not that we cannot could properly when skipping, because\nthere are two ways to have skipped tests and test count upfront, both\nimplemented in my initial version (v1) of t/t9503/test.pl.\n\nYou can set number of tests upfront depending if some feature is\navailable, e.g. \"plan tests => - $use_lint* - $check_xml*\"\nor just \"if ($use_lint) { plan tests => } else ...\".\n\nOr you can explicitly skip tests using SKIP: BLOCK (see Test::More(3pm)).\n\n\nWhat we have here is that we don't know number of tests upfront,\nbecause it is complicated, and that's what I'd like to see in the\ncomment instead (but I don't have idea for exact wording).\n\n> +my @revisions = map { substr $_, 0, 10 } split /\\s/, `git-rev-list --first-parent HEAD`;\n\nWhy do you use shortened SHA1 identifier, instead of full SHA1? Links\nin gitweb use full SHA1.\n\n> +my $head = $revisions[-1];\n\nErrr, HEAD would be $revisions[0], $revisions[-1] would be $root.\n\n> +my $gitweb = File::Spec->catfile('..','..','gitweb','gitweb.cgi');\n> +# the followin two lines of code are workaround for bug in\n> +# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3\n> +# (http://rt.cpan.org/Ticket/Display.html?id=36654)\n> +# for pathnames with spaces (because of \"trash directory\")\n> +$gitweb = File::Spec->rel2abs($gitweb);\n> +$gitweb = Cwd::abs_path($gitweb);\n> +\n> +my $mech = new Test::WWW::Mechanize::CGI;\n> +$mech->cgi_application($gitweb);\n\nAnother solution would be to copy relevant parts of cgi_application\n(without all the checks for example), and use $mech->cgi( sub { ... } );\nhere (without the cgi_application bug).\n\n> +our $baseurl = \"http://localhost\";\n> +our($params, $url, $pagedesc, $status);\n\nI think we can use 'my' here; gitweb uses 'our' only to be able to run\nit correctly as \"legacy CGI\" mod_perl script.\n\n> +our($params, $url, $pagedesc, $status);\n\nStyle: I would write \"our (\", with space between keyword and opening\nparenthesis.\n\n> +# test_page ( , , )\n> +# Example:\n> +# if (test_page '?p=.git;a=summary', 'repository summary') {\n> +# $mech->...;\n> +# $mech->...;\n> +# }\n\nStyle: I would use function call form, i.e. \"test_page(...)\", not\ncommand-like (or script-like) form.\n\n\nAs to the rest of the test: I think as preliminary test it is a good\nthing to have. We can think about what tests to add later (unless you\nrather have exhaustive test suite now...).\n\n-- \nJakub Narebski\nPoland\n"},{"id":"80486","messageId":"485BB578.3040605@gmail.com","threadId":"13950","inReplyTo":"200806201408.05254.jnareb@gmail.com","subject":"Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-20T13:49:44Z","receivedAt":"2008-06-20T13:49:44Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> First, it is not XML validation[*1*], but check if XML is well-formed.\n> Second, I think it would be better if adding XML checks (RSS, Atom,\n> OPML) would be left as separate commit.\n\nSure; FWIW I'm generally in favor of having a large initial commit for \nnew independent files (and since we don't need to worry about conflicts \nI'm not fervent about getting this into the next branch ASAP), but we \ncan definitely add XML checking separately.\n\n>> +# set up test repository\n> \n> I have created this part as a copy of older t9500 gitweb test, thinking\n> about what we might want to test, but the WIP of Mechanize based t9503\n> doesn't have yet tests for those specific features.\n\nI was thinking about that. Right now the tests are so generic that you \ncan replace the test repository with anything else as long as it has \nsome commits (and later some tags, etc.). That's kinda nice.\n\n>> +# We don't count properly when skipping, so no_plan is necessary.\n>> +use Test::More qw(no_plan);\n> \n> Actually it is not that we cannot could properly when skipping, because\n> there are two ways to have skipped tests and test count upfront,\n\nWe're skipping tests if a page-load fails to prevent a slew of failure, \nusing constructs like if(test_page '?...') { ... inspect the page ...}. \nIt's my impression that we shouldn't end up with a wrong test count even \nif one test fails; but then we'd have to replace those ifs with \ncumbersome skip blocks.\n\nI'm generally not in favor of maintaining any test count plans; they're \nan unnecessary failure source, and I don't think they buy us much, if \nanything -- correct me if I'm missing something Perl-specific here.\n\n> Why do you use shortened SHA1 identifier, instead of full SHA1?\n> Links in gitweb use full SHA1.\n\nThat's for readability in the test output; links get checked anyway, \ndon't they? If you think we should be testing against full SHA1s, \nthat's fine too.\n\n> Errr, HEAD would be $revisions[0], $revisions[-1] would be $root.\n\nFixed, thanks.\n\n> Another solution would be to copy relevant parts of cgi_application\n> (without all the checks for example), and use $mech->cgi( sub { ... } );\n> here (without the cgi_application bug).\n\nISTR that using cgi(sub ...) gives us problems with untrappable exits in \ngitweb.cgi (and possibly more things), right? I'm fine with the \nworkarounds we have in place, they don't seem brittle.\n\n>> +our $baseurl = \"http://localhost\";\n>> +our($params, $url, $pagedesc, $status);\n> \n> I think we can use 'my' here;\n\nThey're used in subroutines, so I believe 'our' is correct here.\n\n> Style: I would write \"our (\", with space [...]\n\n>> +# if (test_page '?p=.git;a=summary', 'repository summary') {\n> \n> Style: I would use function call form, i.e. \"test_page(...)\", not\n> command-like (or script-like) form.\n\nBoth fixed, thanks.\n\n> As to the rest of the test: I think as preliminary test it is a good\n> thing to have. We can think about what tests to add later (unless you\n> rather have exhaustive test suite now...).\n\nI'll be writing tests as I go and change parts of gitweb. I won't be \nable to write exhaustive tests, but I at least want to make sure that \nthe code I'm changing is covered somehow.\n\n-- Lea\n"},{"id":"80508","messageId":"200806202003.55919.jnareb@gmail.com","threadId":"13950","inReplyTo":"485BB578.3040605@gmail.com","subject":"Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-20T18:03:55Z","receivedAt":"2008-06-20T18:03:55Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Fri, 20 Jun 2008, Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>>\n>> Second, I think it would be better if adding XML checks (RSS, Atom,\n>> OPML) would be left as separate commit.\n> \n> Sure; FWIW I'm generally in favor of having a large initial commit for \n> new independent files [...]\n\nI just think that having this separate could help bisectability in\nthe case of errors.\n\n>>> +# set up test repository\n>> \n>> I have created this part as a copy of older t9500 gitweb test, thinking\n>> about what we might want to test, but the WIP of Mechanize based t9503\n>> doesn't have yet tests for those specific features.\n> \n> I was thinking about that. Right now the tests are so generic that you \n> can replace the test repository with anything else as long as it has \n> some commits (and later some tags, etc.). That's kinda nice.\n\nWell, I've tried to put the cases where something can go wrong, and to\ncover wide range of possibilities. Rename, copy, typechange, merge\ncommit for testing *diff views, symlink to test 'tree' view, different\ntypes of tags and tagged objects...\n\nWhat could be added is different types of stage output: filenames with\n'*', '+', '=', ':', '?', whitespace, etc. Checking if submodules\ndoesn't trip gitweb would be good idea too.\n \n> I'm generally not in favor of maintaining any test count plans; they're \n> an unnecessary failure source, and I don't think they buy us much, if \n> anything -- correct me if I'm missing something Perl-specific here.\n\nWhile they can detect otherwise unnoticed errors, I think most of time\nthey are error in test counting... so I can agree with that.\n \n>> Why do you use shortened SHA1 identifier, instead of full SHA1?\n>> Links in gitweb use full SHA1.\n> \n> That's for readability in the test output; links get checked anyway, \n> don't they? If you think we should be testing against full SHA1s, \n> that's fine too.\n\nThis would reduce number of operations when crawling gitweb output.\n\n \n[about workaround bug in TWM::CGI when path to application contains\n embedded space].\n\n> ISTR that using cgi(sub ...) gives us problems with untrappable exits in \n> gitweb.cgi (and possibly more things), right?\n\nActually ->cgi_application() is implemented using ->cgi()\nin TWM::CGI. The bug is that it uses straight \"system($application)\",\nwithout any quoting, after ensuring that $application is absolute path\n(so quoting it beforehand won't work).\n\n> I'm fine with the workarounds we have in place, they don't seem brittle.\n\nThey work around the fact that we use 'trash directory', but they\nwould fail if you run test from the directory which contains spaces,\nfor example \"/home/Lea Wiemann/git/t\" (I think this has greater\nprobability happening on operating systems other than Linux, for\nexample MS Windows with \"My Documents\" or \"Program Files\").\n\n>>> +our $baseurl = \"http://localhost\";\n>>> +our($params, $url, $pagedesc, $status);\n>> \n>> I think we can use 'my' here;\n> \n> They're used in subroutines, so I believe 'our' is correct here.\n\nActually 'my' would work here too; the problem with gitweb being\nforced to use 'our' to make it work with mod_perl isn't about the\nfact that they are global variables, but with initializing them,\nas mod_perl wraps whole gitweb in a block (processing requests).\n \n>> As to the rest of the test: I think as preliminary test it is a good\n>> thing to have. We can think about what tests to add later (unless you\n>> rather have exhaustive test suite now...).\n> \n> I'll be writing tests as I go and change parts of gitweb. I won't be \n> able to write exhaustive tests, but I at least want to make sure that \n> the code I'm changing is covered somehow.\n\nWrite test when we notice something breaking seems a common theme\nin git development... ;-)\n\n-- \nJakub Narebski\nPoland\n"},{"id":"80518","messageId":"485C2989.6060406@gmail.com","threadId":"13950","inReplyTo":"200806202003.55919.jnareb@gmail.com","subject":"Re: [WIP/PATCH v3] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-20T22:04:57Z","receivedAt":"2008-06-20T22:04:57Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> Lea Wiemann wrote:\n>> Sure; FWIW I'm generally in favor of having a large initial commit for \n>> new independent files [...]\n> \n> I just think that having this separate could help bisectability in\n> the case of errors.\n\nHm... I wouldn't think that bisect could be necessary for a long linear \n(sequential) test script, where the errors are clearly locatable.\n\n> What could be added is different types of stage output: filenames with\n> '*', '+', '=', ':', '?', whitespace, etc. Checking if submodules\n> doesn't trip gitweb would be good idea too.\n\nYup, filenames with ampersands and semicolons would be fun, too. \n(Submodules seem to work, but that should be covered as well of course.)\n\n> [Full SHA1s] would reduce number of operations when crawling gitweb output.\n\nRight; changed.\n\n> Actually ->cgi_application() is implemented using ->cgi()\n> in TWM::CGI. The bug is that it uses straight \"system($application)\",\n> \n> [it] would fail if you run test from the directory which contains spaces,\n\nOK, now I see what you mean. Well, awesome, borked lirbareis. I've put \nthis as a TODO into the commit message; will take care of it later.\n\n>>>> +our $baseurl = \"http://localhost\";\n>>>> +our($params, $url, $pagedesc, $status);\n>>> I think we can use 'my' here;\n\nFixed, and thanks for the offline explanations.\n\n-- Lea\n"},{"id":"80520","messageId":"1214000325-26389-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"485C2989.6060406@gmail.com","subject":"[WIP/PATCH v4] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-20T22:18:45Z","receivedAt":"2008-06-20T22:18:45Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint (if present) to validate the HTML.\n\nTOOD: Make this runnable even when the path to the git tree (i.e. your\nworking copy) contains blanks.\nhttp://mid.gmane.org/200806202003.55919.jnareb@gmail.com\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\nThis for everyone's reference so you have my most recent version to\nbase changes on.\n\nChanged since v3: applied Jakub's suggestions, and added TODO note to\ncommit message so this doesn't get applied accidentally.\n\n t/t9503-gitweb-Mechanize.sh | 128 +++++++++++++++++++++++++++++++++++++++++++\n t/t9503/test.pl | 128 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 256 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..abcf987\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,128 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+test_description='gitweb as CGI (using WWW::Mechanize)\n+\n+This test uses Perl module Test::WWW::Mechanize::CGI to\n+check gitweb output, using HTML::Lint to validate HTML.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+perl -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <catfile('..','..','gitweb','gitweb.cgi');\n+# The following two lines of code are a workaround for a bug in\n+# Test::WWW::Mechanize::CGI::cgi_application version up to 0.3\n+# (http://rt.cpan.org/Ticket/Display.html?id=36654) for pathnames with\n+# spaces (because of \"trash directory\")\n+$gitweb = File::Spec->rel2abs($gitweb);\n+$gitweb = Cwd::abs_path($gitweb);\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi_application($gitweb);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV, GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'}, $mech->env);\n+\n+# import config, predeclaring config variables\n+my $site_name = '';\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+my %checked_pages;\n+# Validate and spider the current page, if --long-tests (-l) is given.\n+sub check_page {\n+\tmy $uri = $mech->uri;\n+\tif (not $checked_pages{$uri}) {\n+\t\t$mech->html_lint_ok('validate') or return 0 if $long_tests && $use_lint;\n+\t\t$mech->page_links_ok(\"check links on $uri\") if $long_tests;\n+\t\t$checked_pages{$uri} = 1;\n+\t}\n+\treturn 1\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened (and if --long-tests is given that\n+# it validates and has valid links), and return true if it does. Also\n+# set the global variables $params, $pagedesc, and $url for use in the\n+# if block. Optionally pass a third parameter $status to test the\n+# HTTP status code of the page (useful for error pages).\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t$url = \"$baseurl$params\";\n+\tif($status) {\n+\t\t$mech->get($url);\n+\t} else {\n+\t\t$mech->get_ok($url, \"GET \\\"$params\\\" -- $pagedesc\") or return 0;\n+\t}\n+\tcheck_page or return 0;\n+\tif($status) {\n+\t\treturn is($mech->status, $status, \"GET \\\"$params\\\" -- $pagedesc -- yields $status\");\n+\t} else {\n+\t\treturn 1;\n+\t}\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+$mech->get_ok('?p=.git', 'get repository summary');\n+if ($mech->submit_form_ok( { form_number => 1,\n+\t\t\t fields => { 's' => 'Initial' }\n+\t\t\t }, \"submit search form (default)\")) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains searched commit');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+# Summary page.\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+$mech->get_ok('?p=.git', 'get repository summary');\n+for my $revision (@revisions[0..2]) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\tok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), \"$link_text link for $revision\");\n+\t}\n+}\n+\n+\n+1;\n+__END__\n-- \n1.5.6.80.g3141.dirty\n"},{"id":"80659","messageId":"1214181904-30265-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"1214000325-26389-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v5] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-23T00:45:04Z","receivedAt":"2008-06-23T00:45:04Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\nto validate the HTML/XML/tgz output, and checks all links on the\ntested pages if --long-tests is given.\n\nAlso add a GITPERL environment variable that allows running Perl-based\ntests with different perl binaries (and thus under different\nversions).\n\nSigned-off-by: Lea Wiemann \nSigned-off-by: Jakub Narebski \n---\nWindows users: Please apply this patch and run the test, I'd love to\nknow if it works on Windows!\n\nThis patch applies against the *next* branch (without any other\npatches, finally).\n\nNote that in order to actually run the test you need the\nTest::WWW::Mechanize::CGI Perl module.\n\nChanges since v4:\n\n- Add a $GITPERL variable so you can choose the perl binary and run\n the test suite with different Perl versions. Note that this affects\n test-lib.sh.\n\n- Add a workaroud for WWW::Mechanize::CGI so that the test suite also\n works if the path to the Git tree contains blanks (which could\n happen on Windows, for instance).\n\n- Integrated all of Jakub's \"[RFC/PATCH (WIP)] gitweb: Check that RSS,\n Atom and OPML output is well formed XML\", which uses XML::Parser.\n\n- Also added snapshot validation using Archive::Tar.\n\n- Spidering (with --long-tests) is now much more efficient, in that it\n doesn't check any page more than once.\n\n- Added checks to ensure that fragments have corresponding name/id\n attributes in the page.\n\n- Added plenty more actual tests: check more elements on the summary\n page, and also test the commit, commitdiff, tree, blame, history,\n blob, and blob_plain views. Some of these tests don't do anything\n more than downloading a few pages with the given action (without\n checking any of the content), but this already helps a lot since it\n checks that gitweb doesn't die, that the HTML is valid, and (if\n --long-tests is given) that all links work.\n\nI've tested the test suite with Perl 5.8, Perl 5.10, and with and\nwithout the optional modules (HTML::Lint, XML::Parser, Archive::Tar).\n\nI'd like some feedback on this (especially from Jakub) -- if other\npeople OK the patch, I think it's ready for inclusion.\n\nApologies for any tyops and last-minute glitches, I'm getting kinda\ntired... :)\n\n-- Lea\n\n t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++\n t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 2 +\n 3 files changed, 488 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..3fe6d8b\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,132 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+# This test supports the --long-tests option.\n+\n+# This test only runs on Perl 5.8 and later versions, since\n+# Test::WWW::Mechanize::CGI requires Perl 5.8.\n+\n+test_description='gitweb tests (using WWW::Mechanize)\n+\n+This test uses Test::WWW::Mechanize::CGI to test gitweb.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+\"$GITPERL\" -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+\"$GITPERL\" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(\"$GITPERL\" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description < /dev/null`);\n+\t$hash or undef;\n+}\n+\n+sub get_type {\n+\tmy $name = shift;\n+\tchomp(my $type = `git cat-file -t $name 2> /dev/null`);\n+\t$type or undef;\n+}\n+\n+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n+\n+# Thus subroutine was copied (and modified to work with blanks in the\n+# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n+# 'under the same terms as perl itself' and thus GPL compatible.\n+my $cgi = sub {\n+\t# Use exec, not the shell, to support blanks in the path.\n+\tmy $status = system $gitweb $gitweb;\n+\tmy $value = $status >> 8;\n+\n+\tcroak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )\n+\t if ( $status == -1 );\n+\tcroak( qq/Application '$gitweb' exited with value: $value/ )\n+\t if ( $value > 0 );\n+};\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi($cgi);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV,\n+\t GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},\n+\t SCRIPT_FILENAME => $gitweb,\n+\t $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name;\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+# Perform non-recursive checks on the current page, but do not check\n+# the status code.\n+my %verified_uris;\n+sub _verify_page {\n+\tmy ($uri, $fragment) = split '#', $mech->uri;\n+\tif (!$verified_uris{$uri}) {\n+\t\t$verified_uris{$uri} = 1;\n+\n+\t\t# Internal errors yield 200, but cause gitweb.cgi to\n+\t\t# exit with non-zero exit code, which Mechanize::CGI\n+\t\t# translates to 500, so we don't really need to check\n+\t\t# for \"Software error\" here, provided that the test\n+\t\t# cases always check the status code.\n+\t\t#$mech->content_lacks('

Software error:

') or return 0;\n+\n+\t\t# Validating is fast, so we can do it even without\n+\t\t# $long_tests.\n+\t\t$mech->html_lint_ok('[auto] validate HTML') or return 0\n+\t\t if $html_lint_installed && $mech->is_html;\n+\t\tmy $content_type = $mech->response->header('Content-Type')\n+\t\t or die \"$uri does not have a Content-Type header\";\n+\t\tif ($xml_parser_installed && $content_type =~ /xml/) {\n+\t\t\teval { XML::Parser->new->parse($mech->content); };\n+\t\t\tok(!$@, \"[auto] check for XML well-formedness ($uri)\") or diag($@);\n+\t\t}\n+\t\tif ($archive_tar_installed && $uri =~ /sf=tgz/) {\n+\t\t\tmy $snapshot_file = File::Temp->new;\n+\t\t\tprint $snapshot_file $mech->content;\n+\t\t\tclose $snapshot_file;\n+\t\t\tmy $t = Archive::Tar->new;\n+\t\t\t$t->read($snapshot_file->filename, 1);\n+\t\t\tok($t->get_files, \"[auto] valid tgz snapshot ($uri)\");\n+\t\t}\n+\t\t# WebService::Validator::Feed::W3C would be nice to\n+\t\t# use, but it doesn't support direct input (as opposed\n+\t\t# to URIs) long enough for our feeds.\n+\t}\n+\t$mech->content_like(qr/(name|id)=\"$fragment\"/,\n+\t\t\t \"[auto] fragment #$fragment exists ($uri)\")\n+\t if $fragment;\n+\treturn 1;\n+}\n+\n+# Verify and spider the current page, the latter only if --long-tests\n+# (-l) is given. Do not check the status code of the current page.\n+my %spidered_uris; # pages whose links have been checked\n+my %status_checked_uris; # verified pages whose status is known to be 2xx\n+sub check_page {\n+\t_verify_page or return 0;\n+\tmy $orig_url = $mech->uri;\n+\tif ($long_tests && !$spidered_uris{$mech->uri} ) {\n+\t\t$spidered_uris{$mech->uri} = 1;\n+\t\tfor my $url (map { $_->url_abs } $mech->followable_links) {\n+\t\t\tif (!$status_checked_uris{$url}) {\n+\t\t\t\t$status_checked_uris{$url} = 1;\n+\t\t\t\t$mech->get_ok($url, \"[auto] check link ($url)\")\n+\t\t\t\t or diag(\"broken link to $url on $orig_url\");\n+\t\t\t\t_verify_page;\n+\t\t\t\t$mech->back;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 1\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened, call _verify_page on it, and\n+# return true if there was no test failure. Also set the global\n+# variables $params, $pagedesc, and $url for use in the if block.\n+# Optionally pass a third parameter $status to test the HTTP status\n+# code of the page (useful for error pages). You can also pass a full\n+# URL instead of just parameters as the first parameter.\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t$pagedesc = $pagedesc ? \" -- $pagedesc\" : '';\n+\tif($params =~ /^$baseurl/) {\n+\t\t$url = \"$params\";\n+\t} else {\n+\t\t$url = \"$baseurl$params\";\n+\t}\n+\tif ($status) {\n+\t\t$mech->get($url);\n+\t} else {\n+\t\t$mech->get_ok($url, \"get $url$pagedesc\") or return 0;\n+\t}\n+\tcheck_page or return 0;\n+\tif ($status) {\n+\t\treturn is($mech->status, $status, \"getting $url$pagedesc -- yields $status\");\n+\t} else {\n+\t\treturn 1;\n+\t}\n+}\n+\n+# follow_link ( \\%parms, $pagedesc )\n+# Example: follow_link( { text => 'commit' }, 'first commit link')\n+# Like test_page, but does not support status code testing.\n+sub follow_link {\n+\t(my $parms, $pagedesc) = @_;\n+\t$mech->follow_link_ok($parms, \"follow link: $pagedesc\") or return 0;\n+\t$url = $mech->uri;\n+\treturn check_page;\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+$mech->get_ok('?p=.git', 'get repository summary');\n+if ($mech->submit_form_ok( { form_number => 1,\n+\t\t\t fields => { 's' => 'Initial' }\n+\t\t\t }, \"submit search form (default)\")) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains searched commit');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+\n+# Summary view\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+$mech->get_ok('?p=.git', 'get repository summary');\n+for my $revision (@revisions[0..2]) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\tok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), \"$link_text link for $revision\");\n+\t}\n+}\n+# Check that branches and tags are highlighted in green and yellow in\n+# the shortlog. We assume here that we are on master, so it should be\n+# at the top.\n+$mech->content_like(qr{]*class=\"head\"[^>]*>master},\n+\t\t 'master branch is highlighted in shortlog');\n+$mech->content_like(qr{]*class=\"tag\"[^>]*>$tags[0]},\n+\t\t \"$tags[0] (most recent tag) is highlighted in shortlog\");\n+\n+# Check heads. (This should be extracted as well.)\n+for my $head (@heads) {\n+\tfor my $link_text qw( shortlog log tree ) {\n+\t\tok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), \"$link_text link for head '$head'\");\n+\t}\n+}\n+\n+# Check tags (assume we only have tags referring to commits).\n+for my $tag (@tags) {\n+\tmy $commit = rev_parse(\"$tag^{commit}\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),\n+\t \"shortlog link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),\n+\t \"log link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),\n+\t \"commit link for tag '$tag'\");\n+\t# To do: Test tag link for tag objects.\n+\t# Why don't we have tree + snapshot links?\n+}\n+\n+\n+# RSS/Atom/OPML view. Simply retrieve and check.\n+{\n+\t# Broken link in Atom/RSS view -- cannot spider:\n+\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n+\tlocal $long_tests = 0;\n+\ttest_page('?p=.git;a=atom', 'Atom feed');\n+\ttest_page('?p=.git;a=rss', 'RSS feed');\n+}\n+test_page('?a=opml', 'OPML outline');\n+\n+\n+# Commit view\n+if (test_page('?p=.git;a=commit;h=master')) {\n+\tok($mech->find_link(url_abs_regex => qr/a=tree/),\n+\t \"tree link on commit page ($url)\");\n+\t$mech->content_like(qr/A U Thor/, \"author mentioned on commit page ($url)\");\n+}\n+\n+\n+# Commitdiff view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\t$mech->content_like(qr/new file with mode/, \"commitdiff has diffstat ($url)\");\n+\t$mech->content_like(qr/new file mode/, \"commitdiff has diff ($url)\");\n+}\n+\n+\n+# Tree view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $file (@files) {\n+\t\tmy $file_hash = rev_parse(\"HEAD:$file\");\n+\t\tok($mech->find_link(text => $file), \"'$file' listed (and linked) in tree view ($url)\");\n+\t\tif (get_type(\"HEAD:$file\") eq 'blob') {\n+\t\t\tfor my $link_text qw( blob blame history raw ) {\n+\t\t\t\tmy $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,\n+\t\t\t\t\t\t\t text => $link_text);\n+\t\t\t\tok($link, \"'$file' file has $link_text link in tree view ($url)\");\n+\t\t\t}\n+\t\t} else {\n+\t\t\t# Subtree -- to do: write tests. (Need to set\n+\t\t\t# up a subtree in t9503-gitweb-Mechanize.sh.)\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blame view\n+{\n+\t# Broken link in blame view -- cannot spider:\n+\t# http://mid.gmane.org/485EC621.7090101@gmail.com\n+\tlocal $long_tests = 0;\n+\tif ($mech->get_ok('?p=.git', 'get repository summary') &&\n+\t follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\t\tfor my $blame_link ($mech->find_all_links(text => 'blame')) {\n+\t\t\ttest_page($blame_link->url, \"follow blame link from tree view\");\n+\t\t\t$mech->content_like(qr/A U Thor/,\n+\t\t\t\t\t \"author mentioned on blame page\");\n+\t\t}\n+\t}\n+}\n+\n+\n+# History view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $history_link ($mech->find_all_links(text => 'history')) {\n+\t\ttest_page($history_link->url, \"follow history link from tree view\");\n+\t\t# To do: Expand.\n+\t}\n+}\n+\n+\n+# Blob view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $blob_link ($mech->find_all_links(text => 'blob')) {\n+\t\ttest_page($blob_link->url, \"follow blob link from tree view\");\n+\t\t# To do: Expand beyond standard tests.\n+\t}\n+}\n+\n+\n+# Raw view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $raw_link ($mech->find_all_links(text => 'raw')) {\n+\t\ttest_page($raw_link->url, \"follow raw link from tree view\");\n+\t}\n+}\n+\n+\n+1;\n+__END__\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a9fc621..504c0bb 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB\n \n GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n export GITPERLLIB\n+GITPERL=${GITPERL:-perl}\n+export GITPERL\n test -d ../templates/blt || {\n \terror \"You haven't built things yet, have you?\"\n }\n-- \n1.5.6.86.g5139f.dirty\n"},{"id":"80662","messageId":"1214183688-8544-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"1214181904-30265-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-23T01:14:48Z","receivedAt":"2008-06-23T01:14:48Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\nto validate the HTML/XML/tgz output, and checks all links on the\ntested pages if --long-tests is given.\n\nAlso add a GITPERL environment variable that allows running Perl-based\ntests with different perl binaries (and thus under different\nversions).\n\nSigned-off-by: Lea Wiemann \nSigned-off-by: Jakub Narebski \n---\nChanged since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not\njust test execution:\n\n diff --git a/t/t9503/test.pl b/t/t9503/test.pl\n index 28894c5..947e2e8 100755\n --- a/t/t9503/test.pl\n +++ b/t/t9503/test.pl\n @@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n my $cgi = sub {\n # Use exec, not the shell, to support blanks in the path.\n -\tmy $status = system $gitweb $gitweb;\n +\tmy $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;\n my $value = $status >> 8;\n\n\n t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++\n t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++\n t/test-lib.sh | 2 +\n 3 files changed, 488 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..3fe6d8b\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,132 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+# This test supports the --long-tests option.\n+\n+# This test only runs on Perl 5.8 and later versions, since\n+# Test::WWW::Mechanize::CGI requires Perl 5.8.\n+\n+test_description='gitweb tests (using WWW::Mechanize)\n+\n+This test uses Test::WWW::Mechanize::CGI to test gitweb.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+\"$GITPERL\" -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+\"$GITPERL\" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(\"$GITPERL\" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description < /dev/null`);\n+\t$hash or undef;\n+}\n+\n+sub get_type {\n+\tmy $name = shift;\n+\tchomp(my $type = `git cat-file -t $name 2> /dev/null`);\n+\t$type or undef;\n+}\n+\n+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n+\n+# Thus subroutine was copied (and modified to work with blanks in the\n+# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n+# 'under the same terms as perl itself' and thus GPL compatible.\n+my $cgi = sub {\n+\t# Use exec, not the shell, to support blanks in the path.\n+\tmy $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;\n+\tmy $value = $status >> 8;\n+\n+\tcroak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )\n+\t if ( $status == -1 );\n+\tcroak( qq/Application '$gitweb' exited with value: $value/ )\n+\t if ( $value > 0 );\n+};\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi($cgi);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV,\n+\t GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},\n+\t SCRIPT_FILENAME => $gitweb,\n+\t $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name;\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+# Perform non-recursive checks on the current page, but do not check\n+# the status code.\n+my %verified_uris;\n+sub _verify_page {\n+\tmy ($uri, $fragment) = split '#', $mech->uri;\n+\tif (!$verified_uris{$uri}) {\n+\t\t$verified_uris{$uri} = 1;\n+\n+\t\t# Internal errors yield 200, but cause gitweb.cgi to\n+\t\t# exit with non-zero exit code, which Mechanize::CGI\n+\t\t# translates to 500, so we don't really need to check\n+\t\t# for \"Software error\" here, provided that the test\n+\t\t# cases always check the status code.\n+\t\t#$mech->content_lacks('

Software error:

') or return 0;\n+\n+\t\t# Validating is fast, so we can do it even without\n+\t\t# $long_tests.\n+\t\t$mech->html_lint_ok('[auto] validate HTML') or return 0\n+\t\t if $html_lint_installed && $mech->is_html;\n+\t\tmy $content_type = $mech->response->header('Content-Type')\n+\t\t or die \"$uri does not have a Content-Type header\";\n+\t\tif ($xml_parser_installed && $content_type =~ /xml/) {\n+\t\t\teval { XML::Parser->new->parse($mech->content); };\n+\t\t\tok(!$@, \"[auto] check for XML well-formedness ($uri)\") or diag($@);\n+\t\t}\n+\t\tif ($archive_tar_installed && $uri =~ /sf=tgz/) {\n+\t\t\tmy $snapshot_file = File::Temp->new;\n+\t\t\tprint $snapshot_file $mech->content;\n+\t\t\tclose $snapshot_file;\n+\t\t\tmy $t = Archive::Tar->new;\n+\t\t\t$t->read($snapshot_file->filename, 1);\n+\t\t\tok($t->get_files, \"[auto] valid tgz snapshot ($uri)\");\n+\t\t}\n+\t\t# WebService::Validator::Feed::W3C would be nice to\n+\t\t# use, but it doesn't support direct input (as opposed\n+\t\t# to URIs) long enough for our feeds.\n+\t}\n+\t$mech->content_like(qr/(name|id)=\"$fragment\"/,\n+\t\t\t \"[auto] fragment #$fragment exists ($uri)\")\n+\t if $fragment;\n+\treturn 1;\n+}\n+\n+# Verify and spider the current page, the latter only if --long-tests\n+# (-l) is given. Do not check the status code of the current page.\n+my %spidered_uris; # pages whose links have been checked\n+my %status_checked_uris; # verified pages whose status is known to be 2xx\n+sub check_page {\n+\t_verify_page or return 0;\n+\tmy $orig_url = $mech->uri;\n+\tif ($long_tests && !$spidered_uris{$mech->uri} ) {\n+\t\t$spidered_uris{$mech->uri} = 1;\n+\t\tfor my $url (map { $_->url_abs } $mech->followable_links) {\n+\t\t\tif (!$status_checked_uris{$url}) {\n+\t\t\t\t$status_checked_uris{$url} = 1;\n+\t\t\t\t$mech->get_ok($url, \"[auto] check link ($url)\")\n+\t\t\t\t or diag(\"broken link to $url on $orig_url\");\n+\t\t\t\t_verify_page;\n+\t\t\t\t$mech->back;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 1\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened, call _verify_page on it, and\n+# return true if there was no test failure. Also set the global\n+# variables $params, $pagedesc, and $url for use in the if block.\n+# Optionally pass a third parameter $status to test the HTTP status\n+# code of the page (useful for error pages). You can also pass a full\n+# URL instead of just parameters as the first parameter.\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t$pagedesc = $pagedesc ? \" -- $pagedesc\" : '';\n+\tif($params =~ /^$baseurl/) {\n+\t\t$url = \"$params\";\n+\t} else {\n+\t\t$url = \"$baseurl$params\";\n+\t}\n+\tif ($status) {\n+\t\t$mech->get($url);\n+\t} else {\n+\t\t$mech->get_ok($url, \"get $url$pagedesc\") or return 0;\n+\t}\n+\tcheck_page or return 0;\n+\tif ($status) {\n+\t\treturn is($mech->status, $status, \"getting $url$pagedesc -- yields $status\");\n+\t} else {\n+\t\treturn 1;\n+\t}\n+}\n+\n+# follow_link ( \\%parms, $pagedesc )\n+# Example: follow_link( { text => 'commit' }, 'first commit link')\n+# Like test_page, but does not support status code testing.\n+sub follow_link {\n+\t(my $parms, $pagedesc) = @_;\n+\t$mech->follow_link_ok($parms, \"follow link: $pagedesc\") or return 0;\n+\t$url = $mech->uri;\n+\treturn check_page;\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+$mech->get_ok('?p=.git', 'get repository summary');\n+if ($mech->submit_form_ok( { form_number => 1,\n+\t\t\t fields => { 's' => 'Initial' }\n+\t\t\t }, \"submit search form (default)\")) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains searched commit');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+\n+# Summary view\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+$mech->get_ok('?p=.git', 'get repository summary');\n+for my $revision (@revisions[0..2]) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\tok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), \"$link_text link for $revision\");\n+\t}\n+}\n+# Check that branches and tags are highlighted in green and yellow in\n+# the shortlog. We assume here that we are on master, so it should be\n+# at the top.\n+$mech->content_like(qr{]*class=\"head\"[^>]*>master},\n+\t\t 'master branch is highlighted in shortlog');\n+$mech->content_like(qr{]*class=\"tag\"[^>]*>$tags[0]},\n+\t\t \"$tags[0] (most recent tag) is highlighted in shortlog\");\n+\n+# Check heads. (This should be extracted as well.)\n+for my $head (@heads) {\n+\tfor my $link_text qw( shortlog log tree ) {\n+\t\tok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), \"$link_text link for head '$head'\");\n+\t}\n+}\n+\n+# Check tags (assume we only have tags referring to commits).\n+for my $tag (@tags) {\n+\tmy $commit = rev_parse(\"$tag^{commit}\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),\n+\t \"shortlog link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),\n+\t \"log link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),\n+\t \"commit link for tag '$tag'\");\n+\t# To do: Test tag link for tag objects.\n+\t# Why don't we have tree + snapshot links?\n+}\n+\n+\n+# RSS/Atom/OPML view. Simply retrieve and check.\n+{\n+\t# Broken link in Atom/RSS view -- cannot spider:\n+\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n+\tlocal $long_tests = 0;\n+\ttest_page('?p=.git;a=atom', 'Atom feed');\n+\ttest_page('?p=.git;a=rss', 'RSS feed');\n+}\n+test_page('?a=opml', 'OPML outline');\n+\n+\n+# Commit view\n+if (test_page('?p=.git;a=commit;h=master')) {\n+\tok($mech->find_link(url_abs_regex => qr/a=tree/),\n+\t \"tree link on commit page ($url)\");\n+\t$mech->content_like(qr/A U Thor/, \"author mentioned on commit page ($url)\");\n+}\n+\n+\n+# Commitdiff view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\t$mech->content_like(qr/new file with mode/, \"commitdiff has diffstat ($url)\");\n+\t$mech->content_like(qr/new file mode/, \"commitdiff has diff ($url)\");\n+}\n+\n+\n+# Tree view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $file (@files) {\n+\t\tmy $file_hash = rev_parse(\"HEAD:$file\");\n+\t\tok($mech->find_link(text => $file), \"'$file' listed (and linked) in tree view ($url)\");\n+\t\tif (get_type(\"HEAD:$file\") eq 'blob') {\n+\t\t\tfor my $link_text qw( blob blame history raw ) {\n+\t\t\t\tmy $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,\n+\t\t\t\t\t\t\t text => $link_text);\n+\t\t\t\tok($link, \"'$file' file has $link_text link in tree view ($url)\");\n+\t\t\t}\n+\t\t} else {\n+\t\t\t# Subtree -- to do: write tests. (Need to set\n+\t\t\t# up a subtree in t9503-gitweb-Mechanize.sh.)\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blame view\n+{\n+\t# Broken link in blame view -- cannot spider:\n+\t# http://mid.gmane.org/485EC621.7090101@gmail.com\n+\tlocal $long_tests = 0;\n+\tif ($mech->get_ok('?p=.git', 'get repository summary') &&\n+\t follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\t\tfor my $blame_link ($mech->find_all_links(text => 'blame')) {\n+\t\t\ttest_page($blame_link->url, \"follow blame link from tree view\");\n+\t\t\t$mech->content_like(qr/A U Thor/,\n+\t\t\t\t\t \"author mentioned on blame page\");\n+\t\t}\n+\t}\n+}\n+\n+\n+# History view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $history_link ($mech->find_all_links(text => 'history')) {\n+\t\ttest_page($history_link->url, \"follow history link from tree view\");\n+\t\t# To do: Expand.\n+\t}\n+}\n+\n+\n+# Blob view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $blob_link ($mech->find_all_links(text => 'blob')) {\n+\t\ttest_page($blob_link->url, \"follow blob link from tree view\");\n+\t\t# To do: Expand beyond standard tests.\n+\t}\n+}\n+\n+\n+# Raw view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'follow first tree link on page')) {\n+\tfor my $raw_link ($mech->find_all_links(text => 'raw')) {\n+\t\ttest_page($raw_link->url, \"follow raw link from tree view\");\n+\t}\n+}\n+\n+\n+1;\n+__END__\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex a9fc621..504c0bb 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -472,6 +472,8 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB\n \n GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git\n export GITPERLLIB\n+GITPERL=${GITPERL:-perl}\n+export GITPERL\n test -d ../templates/blt || {\n \terror \"You haven't built things yet, have you?\"\n }\n-- \n1.5.6.86.g5139f.dirty\n"},{"id":"80669","messageId":"7vd4m8khmb.fsf@gitster.siamese.dyndns.org","threadId":"13950","inReplyTo":"1214183688-8544-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-23T02:30:36Z","receivedAt":"2008-06-23T02:30:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann writes:\n\n> Changed since v5: honor $ENV{GITPERL} for gitweb.cgi execution, not\n> just test execution:\n>\n> diff --git a/t/t9503/test.pl b/t/t9503/test.pl\n> index 28894c5..947e2e8 100755\n> --- a/t/t9503/test.pl\n> +++ b/t/t9503/test.pl\n> @@ -54,5 +54,5 @@ my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n> my $cgi = sub {\n> # Use exec, not the shell, to support blanks in the path.\n> -\tmy $status = system $gitweb $gitweb;\n> +\tmy $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;\n> my $value = $status >> 8;\n\nMy Perl must be getting rusty. I had to look up this tricky syntax in\nperlfunc.pod, under \"=item exec\", with examples like:\n\n $shell = '/bin/csh';\n exec $shell '-sh';\t\t# pretend it's a login shell\n\n or, more directly,\n\n exec {'/bin/csh'} '-sh';\t# pretend it's a login shell\n\n> t/t9503-gitweb-Mechanize.sh | 132 ++++++++++++++++\n> t/t9503/test.pl | 354 +++++++++++++++++++++++++++++++++++++++++++\n> t/test-lib.sh | 2 +\n> 3 files changed, 488 insertions(+), 0 deletions(-)\n> create mode 100755 t/t9503-gitweb-Mechanize.sh\n> create mode 100755 t/t9503/test.pl\n>\n> diff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\n> new file mode 100755\n> index 0000000..3fe6d8b\n> --- /dev/null\n> +++ b/t/t9503-gitweb-Mechanize.sh\n> @@ -0,0 +1,132 @@\n> +#!/bin/sh\n> +#\n> +# Copyright (c) 2008 Jakub Narebski\n> +# Copyright (c) 2008 Lea Wiemann\n\nIf you mean by this that originally Jakub started and then Lea continued\nextending the work, probably the order of Sign-off should match that order\nto express the patch flow trail better.\n\n> +# check if test can be run\n> +\"$GITPERL\" -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n> +\ttest_expect_success \\\n> +\t\t'skipping gitweb tests, perl version is too old' :\n> +\ttest_done\n> +\texit\n> +}\n> +\n> +\"$GITPERL\" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n> +\ttest_expect_success \\\n> +\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n> +\ttest_done\n> +\texit\n> +}\n\nNice touch.\n"},{"id":"80679","messageId":"485F4A13.2060904@gmail.com","threadId":"13950","inReplyTo":"7vd4m8khmb.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-23T07:00:35Z","receivedAt":"2008-06-23T07:00:35Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> My Perl must be getting rusty. I had to look up this tricky syntax in\n> perlfunc.pod,\n\nI looked in up in #perl. ;-)\n\n>> +# Copyright (c) 2008 Jakub Narebski\n>> +# Copyright (c) 2008 Lea Wiemann\n> \n> If you mean by this that originally Jakub started and then Lea continued\n> extending the work, probably the order of Sign-off should match that order\n> to express the patch flow trail better.\n\nSure; I'll change it if I post another version, or feel free to change \nit when you apply the patch.\n\n-- Lea\n"},{"id":"80702","messageId":"200806231531.13082.jnareb@gmail.com","threadId":"13950","inReplyTo":"1214183688-8544-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-23T13:31:12Z","receivedAt":"2008-06-23T13:31:12Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Lea Wiemann wrote:\n\n> +# We don't count properly when skipping, so no_plan is necessary.\n> +use Test::More qw(no_plan);\n\nI'd rather either skip this comment all together, or change it to\nread something like that (I don't have good solution):\n\n+# Counting tests upfront is difficult, as number of tests depends\n+# on existence of several Perl modules, and is next to impossible\n+# when spidering gitweb output (for --long-test). Additionally,\n+# having number of tests planned stated at beginning is not necessary,\n+# as this test is to be run from git test suite, and not to be\n+# processed further by TAP (Test Anything Protocol) Perl modules.\n\nSee http://www.perlfoundation.org/perl5/index.cgi?testing for\nexplanation why (and when) 'plan' is useful:\n\n TAP output usually starts with a plan line:\n \n 1..9\n \n which specifies how many tests are to follow. The above example\n specifies 9 tests.\n \n This line is optional, but recommended. Should a test suite die\n part-way through, the plan allows the testing framework to recognise\n this situation, rather than assuming that all tests were completed.\n\n> +our $long_tests = $ENV{GIT_TEST_LONG};\n\nHere also it could be \"my $long_tests\"; but I am not a Perl hacker,\nand do not know which is preferred.\n\n> +eval { require Archive::Tar; };\n> +my $archive_tar_installed = !$@\n> + or diag('Archive::Tar is not installed; no tests for valid snapshots');\n\n> +chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);\n> +chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);\n\nWouldn't be easier to use '--format=%(refname)' in git-for-each-ref\ninvocation? Besides, gitweb now uses in link _full_ refname, i.e.\n\"refs/heads/\" and \"refs/tags/\" to avoid branch / tag\nambiguity (when there are both branch and head with the same name).\n\n> +# files and directories in HEAD root:\n> +chomp(my @files = map { (split(\"\\t\", $_))[1] } `git-ls-tree HEAD`);\n\nWouldn't it be easier to use \"git ls-tree --names-only HEAD\"?\n\n> +sub rev_parse (...)\n> +sub get_type (...)\n\nNice.\n\n> +my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n> +\n> +# Thus subroutine was copied (and modified to work with blanks in the\n> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n> +# 'under the same terms as perl itself' and thus GPL compatible.\n\nErrr... I think that without my comment you removed it is hard to\nunderstand what this comment talks about. \"Thus ...\" without any\npreceding sentence?\n\n> +my $cgi = sub {\n> +\t# Use exec, not the shell, to support blanks in the path.\n\nblanks = embedded whitespace?\npath = path to gitweb?\n\n> +\tmy $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;\n> +\tmy $value = $status >> 8;\n> +\n> +\tcroak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )\n> +\t if ( $status == -1 );\n> +\tcroak( qq/Application '$gitweb' exited with value: $value/ )\n> +\t if ( $value > 0 );\n> +};\n> +\n> +my $mech = new Test::WWW::Mechanize::CGI;\n> +$mech->cgi($cgi);\n\nLooks good. Thanks.\n\n> +\t\t# WebService::Validator::Feed::W3C would be nice to\n> +\t\t# use, but it doesn't support direct input (as opposed\n> +\t\t# to URIs) long enough for our feeds.\n\nCould you tell us here what are the limitations? Are those limits\nof W3C SOAP interface for web validation, or the CPAN package mentioned?\n\n> +\treturn 1\n\nStyle: \"return 1;\"\n\n> +# RSS/Atom/OPML view. Simply retrieve and check.\n> +{\n> +\t# Broken link in Atom/RSS view -- cannot spider:\n> +\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n> +\tlocal $long_tests = 0;\n> +\ttest_page('?p=.git;a=atom', 'Atom feed');\n> +\ttest_page('?p=.git;a=rss', 'RSS feed');\n> +}\n> +test_page('?a=opml', 'OPML outline');\n\nThis I think should be written using Test::More equivalent of\ntest_expect_failure in t/test-lib.sh, namely TODO block, either\nas \n\n TODO: {\n local $TODO = \"why\";\n\n ...normal testing code...\n }\n\nor if it causes problem with t9503-gitweb-Mechanize.sh failing, perhaps\nas\n\n TODO: {\n todo_skip \"why\", if ;\n\n ...normal testing code...\n }\n\n\n(And similarly for other pages).\n\n> +# Blob view\n\nI like those comments.\n\n\n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n[...]\n> +GITPERL=${GITPERL:-perl}\n> +export GITPERL\n\nHow it is different from PERL_PATH?\n\n-- \nJakub Narebski\nShadeHawk on #git\nThorn, Poland\n"},{"id":"80734","messageId":"485FE3F7.4040102@gmail.com","threadId":"13950","inReplyTo":"200806231531.13082.jnareb@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-23T17:57:11Z","receivedAt":"2008-06-23T17:57:11Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"[Open issue about PERL_PATH at the bottom!]\n\nJakub Narebski wrote:\n> Lea Wiemann wrote:\n> \n>> +# We don't count properly when skipping, so no_plan is necessary.\n>> +use Test::More qw(no_plan);\n> \n> I'd rather either skip this comment all together, or change it [...]\n\nI'll just remove it. Anybody trying to add a test count will give up \nwithin 30 seconds. ;-)\n\n>> +our $long_tests = $ENV{GIT_TEST_LONG};\n> \n> Here also it could be \"my $long_tests\";\n\nThis one is to make \"local $long_tests = 0\" work -- it wouldn't work \nwith a 'my' declaration.\n\n>> +chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);\n> \n> Wouldn't be easier to use '--format=%(refname)' in git-for-each-ref\n\n*checks* No, since I want to strip the leading refs/heads/ anyway (in \norder to test the page text, which mentions heads and tags without the \ndirectory prefix) -- hence the 'split' wouldn't go away with \n--format=%(refname).\n\n>> +chomp(my @files = map { (split(\"\\t\", $_))[1] } `git-ls-tree HEAD`);\n> \n> Wouldn't it be easier to use \"git ls-tree --names-only HEAD\"?\n\nYup ('--name-only' though). Wonder why I didn't use it -- fixed.\n\n>> +# Thus subroutine was copied (and modified to work with blanks in the\n>> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n>> +# 'under the same terms as perl itself' and thus GPL compatible.\n\nThat's a tyop: s/Thus/This/ (fixed). It refers to the following \"my \n$cgi = sub\". Does the comment make sense now?\n\n>> +my $cgi = sub {\n>> +\t# Use exec, not the shell, to support blanks in the path.\n> \n> blanks = embedded whitespace? path = path to gitweb?\n\nYup. Improved comment, and simplified system call:\n\n-\t# Use exec, not the shell, to support blanks in the path.\n-\tmy $status = system { $ENV{GITPERL} } $ENV{GITPERL}, $gitweb;\n+\t# Use exec, not the shell, to support embedded whitespace in\n+\t# the path to gitweb.cgi.\n+\tmy $status = system $ENV{GITPERL}, $gitweb;\n\n>> +\t\t# WebService::Validator::Feed::W3C would be nice to\n>> +\t\t# use, but it doesn't support direct input (as opposed\n>> +\t\t# to URIs) long enough for our feeds.\n> \n> Could you tell us here what are the limitations? Are those limits\n> of W3C SOAP interface for web validation, or the CPAN package mentioned?\n\nNo idea. I tried it and it told me that the URL was too long -- I \nsuspect it's the W3C server that rejected it. I wouldn't spend more \neffort trying to get online validation to work; it's probably easier to \njust occasionally validate manually. ;) XML well-formedness tests \nshould catch most occasional breakages just fine.\n\n>> +\t# Broken link in Atom/RSS view -- cannot spider:\n>> +\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n>> +\tlocal $long_tests = 0;\n>> +\ttest_page('?p=.git;a=atom', 'Atom feed');\n> \n> This I think should be written using Test::More equivalent of\n> test_expect_failure in t/test-lib.sh, namely TODO block,\n\nRight -- here's my new version (which still fails if the feeds die or \nare not well-formed XML -- I'll want that when I change gitweb):\n\n# RSS/Atom/OPML view\n# Simply retrieve and verify well-formedness, but don't spider.\n$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;\n$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;\nTODO: {\n\t# Now spider -- but there are broken links.\n\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n\tlocal $TODO = \"fix broken links in Atom/RSS feeds\";\n\ttest_page('?p=.git;a=atom', 'Atom feed');\n\ttest_page('?p=.git;a=rss', 'RSS feed');\n}\ntest_page('?a=opml', 'OPML outline');\n\n>> [in t/test-lib.sh:]\n>> +GITPERL=${GITPERL:-perl}\n>> +export GITPERL\n>> [The idea is to easily run the test suite with different perl versions.]\n> \n> How it is different from PERL_PATH?\n\nRight, I didn't think of that. PERL_PATH isn't available in the tests \nthough, it's only used internally by the Makefile to generate (among \nother things) gitweb.cgi. This means that while we can control under \nwhich Perl version gitweb.cgi runs, we cannot control under which Perl \nversion the test suite runs (at least without $PATH trickery). Does \nthis bother us?\n\nIf yes, I'd suggest we keep GITPERL but rename it to GIT_TEST_PERL, \nbecause that's what it's about. If not, I'll rip it out and simply call \n'perl' in the test shell script, whatever version it may be.\n\n-- Lea\n"},{"id":"80783","messageId":"200806240018.20820.jnareb@gmail.com","threadId":"13950","inReplyTo":"485FE3F7.4040102@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-23T22:18:20Z","receivedAt":"2008-06-23T22:18:20Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 23 Jun 2008, Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>> Lea Wiemann wrote:\n>> \n>>> +# We don't count properly when skipping, so no_plan is necessary.\n>>> +use Test::More qw(no_plan);\n>> \n>> I'd rather either skip this comment all together, or change it [...]\n> \n> I'll just remove it. Anybody trying to add a test count will give up \n> within 30 seconds. ;-)\n\nI just think that not having comment is better than have comment which\nis not fully true, or five lines of explanations which could be\nexpanded further.\n\nSimply: planning (coming with number of tests upfront) is hard, and for\nthis situation it doesn't give us anything.\n \n>>> +our $long_tests = $ENV{GIT_TEST_LONG};\n>> \n>> Here also it could be \"my $long_tests\";\n> \n> This one is to make \"local $long_tests = 0\" work -- it wouldn't work \n> with a 'my' declaration.\n\nHmmmm...\n\nSiednote: some time ago I though that I understood difference between\nlexical scoping (my) and dynamical scoping (local)...\n \n>>> +chomp(my @heads = map { (split('/', $_))[2] } \n>>> +\t`git-for-each-ref --sort=-committerdate refs/heads`); \n>> \n>> Wouldn't be easier to use '--format=%(refname)' in git-for-each-ref\n> \n> *checks* No, since I want to strip the leading refs/heads/ anyway (in \n> order to test the page text, which mentions heads and tags without the \n> directory prefix) -- hence the 'split' wouldn't go away with \n> --format=%(refname).\n\nWell, you could have replaced 'split' by s!^refs/!! ;-)))\n\n>>> +# Thus subroutine was copied (and modified to work with blanks in the\n>>> +# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n>>> +# 'under the same terms as perl itself' and thus GPL compatible.\n> \n> That's a tyop: s/Thus/This/ (fixed). It refers to the following \"my \n> $cgi = sub\". Does the comment make sense now?\n\nIt makes sense now, thanks.\n\nAlthough I guess it would be nice to have link to the ticket to the bug\n(report) in WWW::Mechanize::CGI, so we could use cgi_application()\n\"again\" if it gets fixed...\n\n> +\tmy $status = system $ENV{GITPERL}, $gitweb;\n\n...if not for the trick with explicitly calling Perl (GITPERL), to\nallow testing different Perl versions with gitweb.\n \n>>> +\t\t# WebService::Validator::Feed::W3C would be nice to\n>>> +\t\t# use, but it doesn't support direct input (as opposed\n>>> +\t\t# to URIs) long enough for our feeds.\n>> \n>> Could you tell us here what are the limitations? Are those limits\n>> of W3C SOAP interface for web validation, or the CPAN package mentioned?\n> \n> No idea. I tried it and it told me that the URL was too long -- I \n> suspect it's the W3C server that rejected it. I wouldn't spend more \n> effort trying to get online validation to work; it's probably easier to \n> just occasionally validate manually. ;) XML well-formedness tests \n> should catch most occasional breakages just fine.\n\nRight.\n \nWell, W3C feed validator provides SOAP interface we could use, using\nPOST of course and not GET (but it requires net connection), and also\nstandalone CLI validator (unfortunately in Python).\n\n>>> +\t# Broken link in Atom/RSS view -- cannot spider:\n>>> +\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n>>> +\tlocal $long_tests = 0;\n>>> +\ttest_page('?p=.git;a=atom', 'Atom feed');\n>> \n>> This I think should be written using Test::More equivalent of\n>> test_expect_failure in t/test-lib.sh, namely TODO block,\n> \n> Right -- here's my new version (which still fails if the feeds die or \n> are not well-formed XML -- I'll want that when I change gitweb):\n> \n> # RSS/Atom/OPML view\n> # Simply retrieve and verify well-formedness, but don't spider.\n> $mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;\n> $mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;\n> TODO: {\n> \t# Now spider -- but there are broken links.\n> \t# http://mid.gmane.org/485EB333.5070108@gmail.com\n> \tlocal $TODO = \"fix broken links in Atom/RSS feeds\";\n> \ttest_page('?p=.git;a=atom', 'Atom feed');\n> \ttest_page('?p=.git;a=rss', 'RSS feed');\n> }\n> test_page('?a=opml', 'OPML outline');\n\nDoes it make t9503-gitweb.Mechanize.sh fail?\n \n>>> [in t/test-lib.sh:]\n>>> +GITPERL=${GITPERL:-perl}\n>>> +export GITPERL\n>>> [The idea is to easily run the test suite with different perl versions.]\n>> \n>> How it is different from PERL_PATH?\n> \n> Right, I didn't think of that. PERL_PATH isn't available in the tests \n> though, it's only used internally by the Makefile to generate (among \n> other things) gitweb.cgi. [...]\n\nThat's what I was asking about.\n\n-- \nJakub Narebski\nPoland\n"},{"id":"80804","messageId":"4860556F.5060206@gmail.com","threadId":"13950","inReplyTo":"200806240018.20820.jnareb@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-24T02:01:19Z","receivedAt":"2008-06-24T02:01:19Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> Although I guess it would be nice to have link to the ticket to the bug\n> (report) in WWW::Mechanize::CGI, so we could use cgi_application()\n> \"again\" if it gets fixed...\n\nSure, I've added a link to \nhttp://rt.cpan.org/Ticket/Display.html?id=36654 for completeness, though \nwe won't be able to use it if it gets fixed, since we can't break \ncompatibility to older versions. Btw, you may want to add a reply to \nthe bug report (if that's possible -- haven't used CPAN's ticket system \nbefore) that \"system $application $application\" would be a better \nsolution, since it doesn't suffer from quoting issues.\n\n>> Right -- here's my new version (which still fails if the feeds die or \n>> are not well-formed XML -- I'll want that when I change gitweb):\n>>\n>> [snip code]\n> \n> Does it make t9503-gitweb.Mechanize.sh fail?\n\nNo, my point was that it will fail *in case* it breaks (which is good). \n Putting everything into a TODO block would silently ignore issues with \nthe feeds.\n\n>>>> +GITPERL=${GITPERL:-perl}\n>>>> +export GITPERL\n\nBreaking the indecisiveness, I've ripped it out. ;-) We can still test \ngitweb under different Perl versions by make'ing it with different \nPERL_PATHs, so I think it'll be fine.\n\n-- Lea\n"},{"id":"80820","messageId":"1214273933-3925-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"4860556F.5060206@gmail.com","subject":"[PATCH v7] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-24T02:18:53Z","receivedAt":"2008-06-24T02:18:53Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\nto validate the HTML/XML/tgz output, and checks all links on the\ntested pages if --long-tests is given.\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\nChanges since v6:\n\n- Removed GITPERL again.\n\n- Added some basic tests for the page contents of blob and blob_plain\n views.\n\n- Incorporated Jakub's suggestions (see parent posts); in particular\n blame and feed views *do* get spidered now in --long-tests mode, but\n the spider tests are marked as TODO and thus don't cause the test\n suite to fail.\n\n- Some minor (internal) changes: @files only contains blobs, not\n trees; improved the logic that avoids checking pages twice; die if\n test.pl is run directly rather than from the shell script.\n\n t/t9503-gitweb-Mechanize.sh | 132 +++++++++++++++\n t/t9503/test.pl | 378 +++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 510 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..6f7168e\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,132 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+# This test supports the --long-tests option.\n+\n+# This test only runs on Perl 5.8 and later versions, since\n+# Test::WWW::Mechanize::CGI requires Perl 5.8.\n+\n+test_description='gitweb tests (using WWW::Mechanize)\n+\n+This test uses Test::WWW::Mechanize::CGI to test gitweb.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+perl -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+perl -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(perl -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <cwd !~ /trash directory$/;\n+\n+our $long_tests = $ENV{GIT_TEST_LONG}; # \"our\" so we can use \"local $long_tests\"\n+\n+eval { require Archive::Tar; };\n+my $archive_tar_installed = !$@\n+ or diag('Archive::Tar is not installed; no tests for valid snapshots');\n+\n+eval { require HTML::Lint; };\n+my $html_lint_installed = !$@\n+ or diag('HTML::Lint is not installed; no HTML validation tests');\n+\n+eval { require XML::Parser; };\n+my $xml_parser_installed = !$@\n+ or diag('XML::Parser is not installed; no tests for well-formed XML');\n+\n+sub rev_parse {\n+\tmy $name = shift;\n+\tchomp(my $hash = `git rev-parse $name 2> /dev/null`);\n+\t$hash or undef;\n+}\n+\n+sub get_type {\n+\tmy $name = shift;\n+\tchomp(my $type = `git cat-file -t $name 2> /dev/null`);\n+\t$type or undef;\n+}\n+\n+my @revisions = split /\\s/, `git-rev-list --first-parent HEAD`;\n+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);\n+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);\n+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);\n+my @files = grep { get_type(\"HEAD:$_\") eq 'blob' } @root_entries or die;\n+\n+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n+\n+# This subroutine was copied (and modified to work with blanks in the\n+# application path) from WWW::Mechanize::CGI 0.3, which is licensed\n+# 'under the same terms as perl itself' and thus GPL compatible.\n+# http://rt.cpan.org/Ticket/Display.html?id=36654\n+my $cgi = sub {\n+\t# Use exec, not the shell, to support embedded whitespace in\n+\t# the path to gitweb.cgi.\n+\tmy $status = system $gitweb $gitweb;\n+\tmy $value = $status >> 8;\n+\n+\tcroak( qq/Failed to execute application '$gitweb'. Reason: '$!'/ )\n+\t if ( $status == -1 );\n+\tcroak( qq/Application '$gitweb' exited with value: $value/ )\n+\t if ( $value > 0 );\n+};\n+\n+my $mech = new Test::WWW::Mechanize::CGI;\n+$mech->cgi($cgi);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV,\n+\t GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},\n+\t SCRIPT_FILENAME => $gitweb,\n+\t $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name;\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+# Perform non-recursive checks on the current page, but do not check\n+# the status code.\n+my %verified_uris;\n+sub _verify_page {\n+\tmy ($uri, $fragment) = split '#', $mech->uri;\n+\t$mech->content_like(qr/(name|id)=\"$fragment\"/,\n+\t\t\t \"[auto] fragment #$fragment exists ($uri)\")\n+\t if $fragment;\n+\n+\treturn 1 if $verified_uris{$uri};\n+\t$verified_uris{$uri} = 1;\n+\n+\t# Internal errors yield 200 but cause gitweb.cgi to exit with\n+\t# non-zero exit code, which Mechanize::CGI translates to 500,\n+\t# so we don't really need to check for \"Software error\" here,\n+\t# provided that the test cases always check the status code.\n+\t#$mech->content_lacks('

Software error:

') or return 0;\n+\n+\t# Validate. This is fast, so we can do it even without\n+\t# $long_tests.\n+\t$mech->html_lint_ok('[auto] validate HTML') or return 0\n+\t if $html_lint_installed && $mech->is_html;\n+\tmy $content_type = $mech->response->header('Content-Type')\n+\t or die \"$uri does not have a Content-Type header\";\n+\tif ($xml_parser_installed && $content_type =~ /xml/) {\n+\t\teval { XML::Parser->new->parse($mech->content); };\n+\t\tok(!$@, \"[auto] check for XML well-formedness ($uri)\") or diag($@);\n+\t}\n+\tif ($archive_tar_installed && $uri =~ /sf=tgz/) {\n+\t\tmy $snapshot_file = File::Temp->new;\n+\t\tprint $snapshot_file $mech->content;\n+\t\tclose $snapshot_file;\n+\t\tmy $t = Archive::Tar->new;\n+\t\t$t->read($snapshot_file->filename, 1);\n+\t\tok($t->get_files, \"[auto] valid tgz snapshot ($uri)\");\n+\t}\n+\t# WebService::Validator::Feed::W3C would be nice to\n+\t# use, but it doesn't support direct input (as opposed\n+\t# to URIs) long enough for our feeds.\n+\n+\treturn 1;\n+}\n+\n+# Verify and spider the current page, the latter only if --long-tests\n+# (-l) is given. Do not check the status code of the current page.\n+my %spidered_uris; # pages whose links have been checked\n+my %status_checked_uris; # verified pages whose status is known to be 2xx\n+sub check_page {\n+\t_verify_page or return 0;\n+\tmy $orig_url = $mech->uri;\n+\tif ($long_tests && !$spidered_uris{$mech->uri} ) {\n+\t\t$spidered_uris{$mech->uri} = 1;\n+\t\tfor my $url (map { $_->url_abs } $mech->followable_links) {\n+\t\t\tif (!$status_checked_uris{$url}) {\n+\t\t\t\t$status_checked_uris{$url} = 1;\n+\t\t\t\t$mech->get_ok($url, \"[auto] check link ($url)\")\n+\t\t\t\t or diag(\"broken link to $url on $orig_url\");\n+\t\t\t\t_verify_page;\n+\t\t\t\t$mech->back;\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened, call _verify_page on it, and\n+# return true if there was no test failure. Also set the global\n+# variables $params, $pagedesc, and $url for use in the if block.\n+# Optionally pass a third parameter $status to test the HTTP status\n+# code of the page (useful for error pages). You can also pass a full\n+# URL instead of just parameters as the first parameter.\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t$pagedesc = $pagedesc ? \" -- $pagedesc\" : '';\n+\tif($params =~ /^$baseurl/) {\n+\t\t$url = \"$params\";\n+\t} else {\n+\t\t$url = \"$baseurl$params\";\n+\t}\n+\tif ($status) {\n+\t\t$mech->get($url);\n+\t} else {\n+\t\t$mech->get_ok($url, \"get $url$pagedesc\") or return 0;\n+\t}\n+\tcheck_page or return 0;\n+\tif ($status) {\n+\t\treturn is($mech->status, $status, \"getting $url$pagedesc -- yields $status\");\n+\t} else {\n+\t\treturn 1;\n+\t}\n+}\n+\n+# follow_link ( \\%parms, $pagedesc )\n+# Example: follow_link( { text => 'commit' }, 'first commit link')\n+# Like test_page, but does not support status code testing.\n+sub follow_link {\n+\t(my $parms, $pagedesc) = @_;\n+\tunless ($mech->follow_link_ok($parms, \"follow link: $pagedesc\")) {\n+\t\tdiag(\"cannot follow link ($pagedesc) on page \" . $mech->uri);\n+\t\treturn 0;\n+\t}\n+\t$url = $mech->uri;\n+\treturn check_page;\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('./t9503-gitweb-Mechanize.sh test repository', \n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+$mech->get_ok('?p=.git', 'get repository summary');\n+if ($mech->submit_form_ok( { form_number => 1,\n+\t\t\t fields => { 's' => 'Initial' }\n+\t\t\t }, \"submit search form (default)\")) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains searched commit');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+\n+# Summary view\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+$mech->get_ok('?p=.git', 'get repository summary');\n+for my $revision (@revisions[0..2]) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\tok($mech->find_link(url_abs_regex => qr/h=$revision/, text => $link_text), \"$link_text link for $revision\");\n+\t}\n+}\n+# Check that branches and tags are highlighted in green and yellow in\n+# the shortlog. We assume here that we are on master, so it should be\n+# at the top.\n+$mech->content_like(qr{]*class=\"head\"[^>]*>master},\n+\t\t 'master branch is highlighted in shortlog');\n+$mech->content_like(qr{]*class=\"tag\"[^>]*>$tags[0]},\n+\t\t \"$tags[0] (most recent tag) is highlighted in shortlog\");\n+\n+# Check heads. (This should be extracted as well.)\n+for my $head (@heads) {\n+\tfor my $link_text qw( shortlog log tree ) {\n+\t\tok($mech->find_link(url_abs_regex => qr{h=refs/heads/$head}, text => $link_text), \"$link_text link for head '$head'\");\n+\t}\n+}\n+\n+# Check tags (assume we only have tags referring to commits).\n+for my $tag (@tags) {\n+\tmy $commit = rev_parse(\"$tag^{commit}\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog'),\n+\t \"shortlog link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=refs/tags/$tag}, text => 'log'),\n+\t \"log link for tag '$tag'\");\n+\tok($mech->find_link(url_abs_regex => qr{h=$commit}, text => 'commit'),\n+\t \"commit link for tag '$tag'\");\n+\t# To do: Test tag link for tag objects.\n+\t# Why don't we have tree + snapshot links?\n+}\n+\n+\n+# RSS/Atom/OPML view\n+# Simply retrieve and verify well-formedness, but don't spider.\n+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;\n+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;\n+TODO: {\n+\t# Now spider -- but there are broken links.\n+\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n+\tlocal $TODO = \"fix broken links in Atom/RSS feeds\";\n+\ttest_page('?p=.git;a=atom', 'Atom feed');\n+\ttest_page('?p=.git;a=rss', 'RSS feed');\n+}\n+test_page('?a=opml', 'OPML outline');\n+\n+\n+# Commit view\n+if (test_page('?p=.git;a=commit;h=master')) {\n+\tok($mech->find_link(url_abs_regex => qr/a=tree/),\n+\t \"tree link on commit page ($url)\");\n+\t$mech->content_like(qr/A U Thor/, \"author mentioned on commit page ($url)\");\n+}\n+\n+\n+# Commitdiff view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\t$mech->content_like(qr/new file with mode/, \"commitdiff has diffstat ($url)\");\n+\t$mech->content_like(qr/new file mode/, \"commitdiff has diff ($url)\");\n+}\n+\n+\n+# Tree view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'first tree link on page')) {\n+\tfor my $file (@files) {\n+\t\tmy $file_hash = rev_parse(\"HEAD:$file\");\n+\t\tok($mech->find_link(text => $file), \"'$file' listed (and linked) in tree view ($url)\");\n+\t\tfor my $link_text qw( blob blame history raw ) {\n+\t\t\tmy $link = $mech->find_link(url_abs_regex => qr/[^a-z]f=$file(;|$)/,\n+\t\t\t\t\t\t text => $link_text);\n+\t\t\tok($link, \"'$file' file has $link_text link in tree view ($url)\");\n+\t\t}\n+\t}\n+\t# To do: write tests for subtrees. (Need to set up subtrees\n+\t# in t9503-gitweb-Mechanize.sh.)\n+}\n+\n+\n+# Blame view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'first tree link on page')) {\n+\tfor my $blame_link ($mech->find_all_links(text => 'blame')) {\n+\t\tmy $url = $blame_link->url;\n+\t\t$mech->get_ok($url, \"get $url -- blame link on tree view\")\n+\t\t and _verify_page;\n+\t\t$mech->content_like(qr/A U Thor/,\n+\t\t\t\t \"author mentioned on blame page\");\n+\t\tTODO: {\n+\t\t\t# Now spider -- but there are broken links.\n+\t\t\t# http://mid.gmane.org/485EC621.7090101@gmail.com\n+\t\t\tlocal $TODO = \"fix broken links in certain blame views\";\n+\t\t\tcheck_page;\n+\t\t}\n+\t}\n+}\n+\n+\n+# History view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'first tree link on page')) {\n+\tfor my $history_link ($mech->find_all_links(text => 'history')) {\n+\t\ttest_page($history_link->url, \"follow history link from tree view\");\n+\t\t# To do: Expand.\n+\t}\n+}\n+\n+\n+# Blob view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'first tree link on page')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },\n+\t\t\t\t \"\\\"$file\\\" (blob) entry on tree view\")) {\n+\t\t\tchomp(my $first_line_regex = (`cat \"$file\"`)[0]);\n+\t\t\t$first_line_regex =~ s/ / | /g;\n+\t\t\t# Hope that the first line doesn't contain any\n+\t\t\t# HTML-escapable character.\n+\t\t\t$mech->content_like(qr/$first_line_regex/,\n+\t\t\t\t\t \"blob view contains first line of file ($url)\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Raw (blob_plain) view\n+if ($mech->get_ok('?p=.git', 'get repository summary') &&\n+ follow_link( { text => 'tree' }, 'first tree link on page')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => 'raw', url_abs_regex => qr/[^a-z]f=$file(;|$)/ },\n+\t\t\t\t \"raw (blob_plain) entry for \\\"$file\\\" in tree view\")) {\n+\t\t\tchomp(my $last_line = (`cat \"$file\"`)[-1]);\n+\t\t\t$mech->content_contains(\n+\t\t\t\t$last_line, \"blob_plain view contains last line of file\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+1;\n+__END__\n-- \n1.5.6.109.g0c85.dirty\n"},{"id":"80825","messageId":"7vtzfjea64.fsf@gitster.siamese.dyndns.org","threadId":"13950","inReplyTo":"485FE3F7.4040102@gmail.com","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-06-24T04:20:19Z","receivedAt":"2008-06-24T04:20:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lea Wiemann writes:\n\n>> How it is different from PERL_PATH?\n>\n> Right, I didn't think of that. PERL_PATH isn't available in the tests\n> though, it's only used internally by the Makefile to generate (among\n> other things) gitweb.cgi. This means that while we can control under\n> which Perl version gitweb.cgi runs, we cannot control under which Perl\n> version the test suite runs (at least without $PATH trickery). Does\n> this bother us?\n>\n> If yes, I'd suggest we keep GITPERL but rename it to GIT_TEST_PERL,\n> because that's what it's about. If not, I'll rip it out and simply\n> call 'perl' in the test shell script, whatever version it may be.\n\nThat sounds wrong, as the point of tests would be to make sure the stuff\nyou are going to install would work with what you thought will be used\nfrom the system.\n\nIf \"isn't available in the tests\" is the problem, is it possible to make\nit available? We are passing down SHELL_PATH from primary Makefile to t/\nand you should be able to do the same for Perl path...\n\nAbout the Test::WWW:Mechanize::CGI thing, how widely available is it? I\ndo not think it is packaged for Debian nor Ubuntu, for example.\n"},{"id":"80856","messageId":"4860B255.7010909@gmail.com","threadId":"13950","inReplyTo":"7vtzfjea64.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-24T08:37:41Z","receivedAt":"2008-06-24T08:37:41Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> We are passing down SHELL_PATH from primary Makefile to t/\n> and you should be able to do the same for Perl path...\n\nI see; that'll work, thanks! Will send v8 soon.\n\n> About the Test::WWW:Mechanize::CGI thing, how widely available is it?\n\nNot very, you basically have to install it from CPAN. If it's not \ninstalled, the only message you get from the test is:\n\nok 1: skipping gitweb tests, Test::WWW::Mechanize::CGI not found\n\nShould optional test dependencies like Test::WWW::Mechnanize::CGI be \ndocumented in INSTALL?\n\nBest,\n\n Lea\n"},{"id":"80866","messageId":"200806241123.42456.jnareb@gmail.com","threadId":"13950","inReplyTo":"7vtzfjea64.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH v6] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-24T09:23:41Z","receivedAt":"2008-06-24T09:23:41Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Tue, 24 Jun 2008, Junio C Hamano wrote:\n\n> About the Test::WWW::Mechanize::CGI thing, how widely available is it?\n> I do not think it is packaged for Debian nor Ubuntu, for example.\n\nNot very widely; practically you have to install it from CPAN. \nTest::WWW::Mechanize::CGI is at v0.1, WWW::Mechanize::CGI is at v0.3.\n\nBut if Test::WWW::Mechanize::CGI is not installed, test would be not \nrun. If you are gitweb developer, then installing locally in $HOME \nfrom CPAN is I guess viable option; if you are not gitweb developer, we \nstill have t/t9500-gitweb-standalone-no-errors.sh\n\n\nThe whole point of using those packages was that it makes it _easy_\nto write those tests. This consist of two parts: \n\n1). running gitweb as if it was CGI application, which otherwise would \neither require deep knowledge of how CGI application is invoked (what \nit is in WWW::Mechanize::CGI and required dependence \nHTTP::Request::AsCGI which I think does all the work) or working\nweb server, done a la \"git instaweb\",\n\n2.) accessing and testing gitweb output (what Test::WWW::Mechanize,\nWWW::Mechanize does; we could do the same with LWP* modules from \nlibwww-perl / perl-libwww-perl package and Test::More from Perl, but\nit would be repeating Test::WWW::Mechanize work).\n-- \nJakub Narebski\nPoland\n"},{"id":"81290","messageId":"1214488071-6721-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"1214273933-3925-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-26T13:47:51Z","receivedAt":"2008-06-26T13:47:51Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\nto validate the HTML/XML/tgz output, and checks all links on the\ntested pages if --long-tests is given.\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\nThis patch applies on next.\n\nChanges since v7:\n\n- In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets\n source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.\n\n- .git/description in the test repository no longer depends on $0\n (this would e.g. cause 'cd t; make' to fail).\n\n- Add test_link subroutine and use it everywhere in place of\n ok(find_link...) so that links whose presence get tested get checked\n and spidered in --long-tests mode.\n\n- Add simple page caching (reduces execution time without --long-tests\n by 25%). That's *really* helpful when you have to run those tests\n on a regular basis. ;) (WWW::Mechanize::Cached won't work with\n TWM::CGI, so we have to implement it ourselves; but it's easier\n anyway.)\n\n- Follow redirects rather than failing.\n\n- Test subdirectories in tree view.\n\n- Test error handling for non-existent hashes or hashes of wrong type.\n\n- Test commitdiff_plain view.\n\n- Expand test for history view.\n\n- Test tag objects (not just symbolic tags) in tag list.\n\n- Test a specific bug (under \"diff formatting\", marked TODO).\n\n- Do not test correctness of line number fragments (#l[0-9]+); they're\n broken too often right now.\n\n- Probably some more minor improvements I've forgotten about. :)\n\n Makefile | 1 +\n t/t9503-gitweb-Mechanize.sh | 138 +++++++++++\n t/t9503/test.pl | 553 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 692 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/Makefile b/Makefile\nindex a129491..3bf4f14 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1251,6 +1251,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS\n \n GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS\n \t@echo SHELL_PATH=\\''$(SHELL_PATH_SQ)'\\' >$@\n+\t@echo PERL_PATH=\\''$(PERL_PATH_SQ)'\\' >>$@\n \n ### Detect Tck/Tk interpreter path changes\n ifndef NO_TCLTK\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..4585cea\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,138 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+# This test supports the --long-tests option.\n+\n+# This test only runs on Perl 5.8 and later versions, since\n+# Test::WWW::Mechanize::CGI requires Perl 5.8.\n+\n+test_description='gitweb tests (using WWW::Mechanize)\n+\n+This test uses Test::WWW::Mechanize::CGI to test gitweb.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+\"$PERL_PATH\" -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+\"$PERL_PATH\" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tmkdir dir1 &&\n+\techo \"New file\" >> dir1/file1 &&\n+\tgit add dir1/file1 &&\n+\ttest_tick && git commit -a -m \"File added in subdirectory.\" &&\n+\tgit tag -m \"creating a tag object\" tag-object\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(\"$PERL_PATH\" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <cwd !~ /trash directory$/;\n+\n+our $long_tests = $ENV{GIT_TEST_LONG}; # \"our\" so we can use \"local $long_tests\"\n+\n+eval { require Archive::Tar; };\n+my $archive_tar_installed = !$@\n+ or diag('Archive::Tar is not installed; no tests for valid snapshots');\n+\n+eval { require HTML::Lint; };\n+my $html_lint_installed = !$@\n+ or diag('HTML::Lint is not installed; no HTML validation tests');\n+\n+eval { require XML::Parser; };\n+my $xml_parser_installed = !$@\n+ or diag('XML::Parser is not installed; no tests for well-formed XML');\n+\n+sub rev_parse {\n+\tmy $name = shift;\n+\tchomp(my $hash = `git rev-parse $name 2> /dev/null`);\n+\t$hash or undef;\n+}\n+\n+sub get_type {\n+\tmy $name = shift;\n+\tchomp(my $type = `git cat-file -t $name 2> /dev/null`);\n+\t$type or undef;\n+}\n+\n+\n+package OurMechanize;\n+\n+use base qw( Test::WWW::Mechanize::CGI );\n+\n+my %page_cache;\n+# Cache requests.\n+sub _make_request {\n+\tmy ($self, $request) = (shift, shift);\n+\n+\tmy $response;\n+\tunless ($response = Storable::thaw($page_cache{$request->uri})) {\n+\t\t$response = $self->SUPER::_make_request($request, @_);\n+\t\t$page_cache{$request->uri} = Storable::freeze($response);\n+\t}\n+\treturn $response;\n+}\n+\n+# Fix whitespace problem.\n+sub cgi_application {\n+\tmy ($self, $application) = @_;\n+\n+\t# This subroutine was copied (and modified) from\n+\t# WWW::Mechanize::CGI 0.3, which is licensed 'under the same\n+\t# terms as perl itself' and thus GPL compatible.\n+\tmy $cgi = sub {\n+\t\t# Use exec, not the shell, to support embedded\n+\t\t# whitespace in the path to $application.\n+\t\t# http://rt.cpan.org/Ticket/Display.html?id=36654\n+\t\tmy $status = system $application $application;\n+\t\tmy $value = $status >> 8;\n+\n+\t\tcroak( qq/Failed to execute application '$application'. Reason: '$!'/ )\n+\t\t if ( $status == -1 );\n+\t\tcroak( qq/Application '$application' exited with value: $value/ )\n+\t\t if ( $value > 0 );\n+\t};\n+\n+\t$self->cgi($cgi);\n+}\n+\n+package main;\n+\n+\n+my @revisions = split /\\s/, `git-rev-list --first-parent HEAD`;\n+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);\n+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);\n+my @tag_objects = grep { get_type($_) eq 'tag' } @tags;\n+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);\n+my @files = grep { get_type(\"HEAD:$_\") eq 'blob' } @root_entries or die;\n+my @directories = grep { get_type(\"HEAD:$_\") eq 'tree' } @root_entries or die;\n+# Only test one file and directory each.\n+@files = $files[0] unless $long_tests;\n+@directories = $directories[0] unless $long_tests;\n+\n+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n+\n+my $mech = OurMechanize->new;\n+$mech->cgi_application($gitweb);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV,\n+\t GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},\n+\t SCRIPT_FILENAME => $gitweb,\n+\t $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name;\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+# Perform non-recursive checks on the current page, but do not check\n+# the status code.\n+my %verified_uris;\n+sub _verify_page {\n+\tmy ($uri, $fragment) = split '#', $mech->uri;\n+\tTODO: {\n+\t\tlocal $TODO = 'line number fragments can be broken for diffs and blames'\n+\t\t if $fragment && $fragment =~ /^l[0-9]+$/;\n+\t\t$mech->content_like(qr/(name|id)=\"$fragment\"/,\n+\t\t\t\t \"[auto] fragment #$fragment exists ($uri)\")\n+\t\t if $fragment;\n+\t}\n+\n+\treturn 1 if $verified_uris{$uri};\n+\t$verified_uris{$uri} = 1;\n+\n+\t# Internal errors yield 200 but cause gitweb.cgi to exit with\n+\t# non-zero exit code, which Mechanize::CGI translates to 500,\n+\t# so we don't really need to check for \"Software error\" here,\n+\t# provided that the test cases always check the status code.\n+\t#$mech->content_lacks('

Software error:

') or return 0;\n+\n+\t# Validate. This is fast, so we can do it even without\n+\t# $long_tests.\n+\t$mech->html_lint_ok('[auto] validate HTML') or return 0\n+\t if $html_lint_installed && $mech->is_html;\n+\tmy $content_type = $mech->response->header('Content-Type')\n+\t or die \"$uri does not have a Content-Type header\";\n+\tif ($xml_parser_installed && $content_type =~ /xml/) {\n+\t\teval { XML::Parser->new->parse($mech->content); };\n+\t\tok(!$@, \"[auto] check for XML well-formedness ($uri)\") or diag($@);\n+\t}\n+\tif ($archive_tar_installed && $uri =~ /sf=tgz/) {\n+\t\tmy $snapshot_file = File::Temp->new;\n+\t\tprint $snapshot_file $mech->content;\n+\t\tclose $snapshot_file;\n+\t\tmy $t = Archive::Tar->new;\n+\t\t$t->read($snapshot_file->filename, 1);\n+\t\tok($t->get_files, \"[auto] valid tgz snapshot ($uri)\");\n+\t}\n+\t# WebService::Validator::Feed::W3C would be nice to\n+\t# use, but it doesn't support direct input (as opposed\n+\t# to URIs) long enough for our feeds.\n+\n+\treturn 1;\n+}\n+\n+# Verify and spider the current page, the latter only if --long-tests\n+# (-l) is given. Do not check the status code of the current page.\n+my %spidered_uris; # pages whose links have been checked\n+my %status_checked_uris; # verified pages whose status is known to be 2xx\n+sub check_page {\n+\t_verify_page or return 0;\n+\tif ($long_tests && !$spidered_uris{$mech->uri} ) {\n+\t\t$spidered_uris{$mech->uri} = 1;\n+\t\tmy $orig_url = $mech->uri;\n+\t\tTODO: {\n+\t\t\tlocal $TODO = \"blame links can be broken sometimes\"\n+\t\t\t if $orig_url =~ /a=blame/;\n+\t\t\tfor my $url (map { $_->url_abs } $mech->followable_links) {\n+\t\t\t\tif (!$status_checked_uris{$url}) {\n+\t\t\t\t\t$status_checked_uris{$url} = 1;\n+\t\t\t\t\tlocal $long_tests = 0; # stop recursing\n+\t\t\t\t\ttest_page($url, \"[auto] check link ($url)\")\n+\t\t\t\t\t or diag(\"broken link to $url on $orig_url\");\n+\t\t\t\t\t$mech->back;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened, call _verify_page on it, and\n+# return true if there was no test failure. Also set the global\n+# variables $params, $pagedesc, and $url for use in the if block.\n+# Optionally pass a third parameter $status to test the HTTP status\n+# code of the page (useful for error pages). You can also pass a full\n+# URL instead of just parameters as the first parameter.\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t# missing $pagedesc is usually accidental\n+\tdie \"$params: no pagedesc given\" unless defined $pagedesc;\n+\tif($params =~ /^$baseurl/) {\n+\t\t$url = \"$params\";\n+\t} else {\n+\t\t$url = \"$baseurl$params\";\n+\t}\n+\t$mech->get($url);\n+\tlike($mech->status, $status ? qr/$status/ : qr/^[23][0-9][0-9]$/,\n+\t \"$pagedesc: $url\" . ($status ? \" -- yields $status\" : \"\"))\n+\t or return 0;\n+\tif ($mech->status =~ /^3/) {\n+\t\t# Don't check 3xx, they tend to look funny.\n+\t\tmy $location = $mech->response->header('Location');\n+\t\t$mech->back; # compensate for history\n+\t\treturn test_page($location, \"follow redirect from $url\");\n+\t} else {\n+\t\treturn check_page;\n+\t}\n+}\n+\n+# follow_link ( \\%parms, $pagedesc )\n+# Example:\n+# if (follow_link( { text => 'commit' }, 'first commit link')) {\n+# $mech->...;\n+# $mech->back;\n+# }\n+# Like test_page, but does not support status code testing, and\n+# returns true if there was a link at all, regardless of whether it\n+# was [23]xx or not.\n+sub follow_link {\n+\t(my $parms, $pagedesc) = @_;\n+\tmy $link = $mech->find_link(%$parms);\n+\tmy $current_url = $mech->uri;\n+\tok($link, \"link exists: $pagedesc (on page $current_url)\") or return 0;\n+\ttest_page($link->url, \"follow link: $pagedesc (on page $current_url)\");\n+\treturn 1;\n+}\n+\n+# like follow_link, except that only checks and goes back immediately;\n+# use this instead of ok(find_link...).\n+sub test_link {\n+\tmy ($parms, $pagedesc) = @_;\n+\tmy $current_url = $mech->uri;\n+\tif($long_tests) {\n+\t\t# Check status, validate, spider.\n+\t\treturn follow_link($parms, $pagedesc) && $mech->back;\n+\t} else {\n+\t\t# Only check presence of the link (much faster).\n+\t\treturn ok($mech->find_link(%$parms),\n+\t\t\t \"link exists: $pagedesc (on page $current_url)\");\n+\t}\n+}\n+\n+sub get_summary {\n+\ttest_page('?p=.git', 'repository summary')\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('gitweb test repository',\n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+if (get_summary && $mech->submit_form_ok(\n+\t { form_number => 1, fields => { 's' => 'Initial' } },\n+\t 'submit search form (default)')) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains commit we searched for');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+\n+# Summary view\n+\n+get_summary or die 'summary page failed; no point in running the remaining tests';\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+for my $revision (@revisions) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\ttest_link( { url_abs_regex => qr/h=$revision/, text => $link_text },\n+\t\t\t \"$link_text link for $revision\");\n+\t}\n+}\n+\n+# Check that branches and tags are highlighted in green and yellow in\n+# the shortlog. We assume here that we are on master, so it should be\n+# at the top.\n+$mech->content_like(qr{]*class=\"head\"[^>]*>master},\n+\t\t 'master branch is highlighted in shortlog');\n+$mech->content_like(qr{]*class=\"tag\"[^>]*>$tags[0]},\n+\t\t \"$tags[0] (most recent tag) is highlighted in shortlog\");\n+\n+# Check heads. (This should be extracted as well.)\n+for my $head (@heads) {\n+\tfor my $link_text qw( shortlog log tree ) {\n+\t\ttest_link( { url_abs_regex => qr{h=refs/heads/$head}, text => $link_text },\n+\t\t\t \"$link_text link for head '$head'\");\n+\t}\n+}\n+\n+# Check tags (assume we only have tags referring to commits, not to\n+# blobs or trees).\n+for my $tag (@tags) {\n+\tmy $commit = rev_parse(\"$tag^{commit}\");\n+\ttest_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog' },\n+\t\t \"shortlog link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'log' },\n+\t\t \"log link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=$commit}, text => 'commit' },\n+\t\t \"commit link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=$commit}, text => $tag },\n+\t \"'$tag' links to the commit as well\");\n+\t# To do: Test tag link for tag objects.\n+\t# Why don't we have tree + snapshot links?\n+}\n+for my $tag (@tag_objects) {\n+\tmy $tag_sha1 = rev_parse($tag);\n+\ttest_link( { url_abs_regex => qr{h=$tag_sha1}, text => 'tag' },\n+\t\t \"tag link for tag object '$tag'\" );\n+}\n+\n+\n+# RSS/Atom/OPML view\n+# Simply retrieve and verify well-formedness, but don't spider.\n+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;\n+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;\n+TODO: {\n+\t# Now spider -- but there are broken links.\n+\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n+\tlocal $TODO = \"fix broken links in Atom/RSS feeds\";\n+\ttest_page('?p=.git;a=atom', 'Atom feed');\n+\ttest_page('?p=.git;a=rss', 'RSS feed');\n+}\n+test_page('?a=opml', 'OPML outline');\n+\n+\n+# Commit view\n+if (test_page('?p=.git;a=commit;h=master', 'view HEAD commit')) {\n+\tmy $tree_sha1 = rev_parse('master:');\n+\ttest_link( { url_abs_regex => qr/a=tree/, text => rev_parse('master:') },\n+\t\t \"SHA1 link to tree on commit page ($url)\");\n+\ttest_link( { url_abs_regex => qr/h=$tree_sha1/, text => 'tree' },\n+\t\t \"'tree' link to tree on commit page ($url)\");\n+\t$mech->content_like(qr/A U Thor/, \"author mentioned on commit page ($url)\");\n+}\n+\n+\n+# Commitdiff view\n+if (get_summary &&\n+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\t$mech->content_like(qr/new file with mode/, \"commitdiff has diffstat ($url)\");\n+\t$mech->content_like(qr/new file mode/, \"commitdiff has diff ($url)\");\n+}\n+test_page(\"?p=.git;a=commitdiff;h=$revisions[-1]\",\n+\t 'commitdiff without parent');\n+\n+# Diff formattting problem.\n+if (get_summary &&\n+ follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\tTODO: {\n+\t\tlocal $TODO = \"bad a/* link in diff\";\n+\t\tif (follow_link( { text_regex => qr!^a/! },\n+\t\t\t\t 'a/* link (probably wrong)')) {\n+\t\t\t# The page we land on here is broken already.\n+\t\t\tfollow_link( { url_abs_regex => qr/a=blob_plain/ },\n+\t\t\t\t 'linked file name'); # bang\n+\t\t}\n+\t}\n+}\n+\n+\n+# Raw commitdiff (commitdiff_plain) view\n+if (test_page('?p=.git;a=commit;h=refs/tags/tag-object',\n+\t 'commit view of tags/tag-object') &&\n+ follow_link( { text => 'commitdiff' }, \"'commitdiff'\") &&\n+ follow_link( { text => 'raw' }, \"'raw' (commitdiff_plain)\")) {\n+\t$mech->content_like(qr/^From: A U Thor $/m,\n+\t\t\t 'commitdiff_plain: From header');\n+\tTODO: {\n+\t\tlocal $TODO = 'date header mangles timezone';\n+\t\t$mech->content_like(qr/^Date: Thu, 7 Apr 2005 15:..:13 -0700$/m,\n+\t\t\t\t 'commitdiff_plain: Date header (correct)');\n+\t}\n+\t$mech->content_like(qr/^Date: Thu, 7 Apr 2005 22:..:13 \\+0000 \\(-0700\\)$/m,\n+\t\t\t 'commitdiff_plain: Date header (UTC, wrong)');\n+\t$mech->content_like(qr/^Subject: .+$/m,\n+\t\t\t 'commitdiff_plain: Subject header');\n+\t# $ markers inexplicably don't work here if we use like(...)\n+\t# or $mech->content_like().\n+\tok($mech->content =~ /^X-Git-Tag: tag-object\\^0$/m,\n+\t 'commitdiff_plain: X-Git-Tag header');\n+\tok($mech->content =~ /^X-Git-Url: $baseurl\\?p=\\.git;a=commitdiff_plain;h=refs%2Ftags%2Ftag-object$/m,\n+\t 'commitdiff_plain: X-Git-Url header');\n+\tok($mech->content =~ /^---$/m, 'commitdiff_plain: separator$');\n+\t$mech->content_like(qr/^diff --git /m, 'commitdiff_plain: diff$');\n+}\n+\n+\n+# Tree view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tmy $sha1 = rev_parse(\"HEAD:$file\");\n+\t\ttest_link( { text => $file, url_abs_regex => qr/h=$sha1/ },\n+\t\t\t \"'$file' is listed and linked\");\n+\t\ttest_link({ url_abs_regex => qr/f=$file/, text => $_ },\n+\t\t\t \"'$_' link\") foreach qw( blame blob history raw );\n+\t}\n+\tfor my $directory (@directories) {\n+\t\tmy $sha1 = rev_parse(\"HEAD:$directory\");\n+\t\ttest_link({ url_abs_regex => qr/f=$directory/, text => $_ },\n+\t\t\t \"'$_' link\") foreach qw( tree history );\n+\t\tif(follow_link( { text => $directory, url_abs_regex => qr/h=$sha1/ },\n+\t\t\t\t\"'$directory is listed and linked\" )) {\n+\t\t\tif(follow_link( { text => '..' }, 'parent directory')) {\n+\t\t\t\ttest_link({ url_abs_regex => qr/h=$sha1/,\n+\t\t\t\t\t text => $directory },\n+\t\t\t\t\t 'back to original tree view');\n+\t\t\t\t$mech->back;\n+\t\t\t}\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blame view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $blame_link ($mech->find_all_links(text => 'blame')) {\n+\t\tmy $url = $blame_link->url;\n+\t\t$mech->get_ok($url, \"get $url -- blame link on tree view\")\n+\t\t and _verify_page;\n+\t\t$mech->content_like(qr/A U Thor/,\n+\t\t\t\t \"author mentioned on blame page\");\n+\t\tTODO: {\n+\t\t\t# Now spider -- but there are broken links.\n+\t\t\t# http://mid.gmane.org/485EC621.7090101@gmail.com\n+\t\t\tlocal $TODO = \"fix broken links in certain blame views\";\n+\t\t\tcheck_page;\n+\t\t}\n+\t\tlast unless $long_tests; # only test first blame link\n+\t}\n+}\n+\n+\n+# History view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files, @directories) {\n+\t\tmy $type = get_type(\"HEAD:$file\"); # blob or tree\n+\t\tif (follow_link( { text => 'history', url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"history link for '$file'\")) {\n+\t\t\t# There is at least one commit, so A U Thor is mentioned.\n+\t\t\t$mech->content_contains('A U Thor', 'A U Thor mentioned');\n+\t\t\t# The following tests test for at least *one*\n+\t\t\t# link of each type and are weak since we\n+\t\t\t# don't have any knowledge of commit hashes.\n+\t\t\ttest_link( { text => $type, url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"$type\");\n+\t\t\ttest_link( { text => 'commitdiff' },\n+\t\t\t\t \"commitdiff\");\n+\t\t\ttest_link( { url_abs_regex => qr/a=commit;.*h=[a-f0-9]{40}/ },\n+\t\t\t\t \"subject links to commit\"); # weak, brittle\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blob view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },\n+\t\t\t\t \"\\\"$file\\\" (blob) entry on tree view\")) {\n+\t\t\tchomp(my $first_line_regex = (`cat \"$file\"`)[0]);\n+\t\t\t$first_line_regex =~ s/ / | /g;\n+\t\t\t# Hope that the first line doesn't contain any\n+\t\t\t# HTML-escapable character.\n+\t\t\t$mech->content_like(qr/$first_line_regex/,\n+\t\t\t\t\t \"blob view contains first line of file ($url)\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Raw (blob_plain) view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => 'raw', url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"raw (blob_plain) entry for \\\"$file\\\" in tree view\")) {\n+\t\t\tchomp(my $first_line = (`cat \"$file\"`)[0]);\n+\t\t\t$mech->content_contains(\n+\t\t\t\t$first_line, \"blob_plain view contains first line of file\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Error handling\n+# Pass valid and invalid paths to various file-based actions\n+for my $action qw( blame blob blob_plain blame ) {\n+\ttest_page(\"?p=.git;a=$action;f=$files[0];hb=HEAD\",\n+\t\t \"$action: look up existent file\");\n+\ttest_page(\"?p=.git;a=$action;f=does_not_exist;hb=HEAD\",\n+\t\t \"$action: look up non-existent file\", 404);\n+\tTODO: {\n+\t\tlocal $TODO = 'wrong error code (but using Git::Repo will fix this)';\n+\t\ttest_page(\"?p=.git;a=$action;f=$directories[0];hb=HEAD\",\n+\t\t\t \"$action: look up directory\", 400);\n+\t}\n+}\n+TODO: {\n+\tlocal $TODO = 'wrong error code (but using Git::Repo will fix this)';\n+\ttest_page(\"?p=.git;a=tree;f=$files[0];hb=HEAD\",\n+\t\t 'tree: look up existent file', 400);\n+}\n+# Pass valid and invalid paths to tree action\n+test_page(\"?p=.git;a=tree;f=does_not_exist;hb=HEAD\",\n+\t 'tree: look up non-existent file', 404);\n+test_page(\"?p=.git;a=tree;f=$directories[0];hb=HEAD\",\n+\t 'tree: look up directory');\n+TODO: {\n+\tlocal $TODO = 'cannot use f=/ or f= for trees';\n+\ttest_page(\"?p=.git;a=tree;f=/;hb=HEAD\", 'tree: look up directory');\n+}\n+\n+\n+1;\n+__END__\n-- \n1.5.6.109.g33ded.dirty\n"},{"id":"81291","messageId":"1214488126-6783-1-git-send-email-LeWiemann@gmail.com","threadId":"13950","inReplyTo":"1214273933-3925-1-git-send-email-LeWiemann@gmail.com","subject":"[PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-26T13:48:46Z","receivedAt":"2008-06-26T13:48:46Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\nalso uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\nto validate the HTML/XML/tgz output, and checks all links on the\ntested pages if --long-tests is given.\n\nSigned-off-by: Jakub Narebski \nSigned-off-by: Lea Wiemann \n---\n[Resent with correct subject.]\n\nThis patch applies on next.\n\nChanges since v7:\n\n- In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets\n source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.\n\n- .git/description in the test repository no longer depends on $0\n (this would e.g. cause 'cd t; make' to fail).\n\n- Add test_link subroutine and use it everywhere in place of\n ok(find_link...) so that links whose presence get tested get checked\n and spidered in --long-tests mode.\n\n- Add simple page caching (reduces execution time without --long-tests\n by 25%). That's *really* helpful when you have to run those tests\n on a regular basis. ;) (WWW::Mechanize::Cached won't work with\n TWM::CGI, so we have to implement it ourselves; but it's easier\n anyway.)\n\n- Follow redirects rather than failing.\n\n- Test subdirectories in tree view.\n\n- Test error handling for non-existent hashes or hashes of wrong type.\n\n- Test commitdiff_plain view.\n\n- Expand test for history view.\n\n- Test tag objects (not just symbolic tags) in tag list.\n\n- Test a specific bug (under \"diff formatting\", marked TODO).\n\n- Do not test correctness of line number fragments (#l[0-9]+); they're\n broken too often right now.\n\n- Probably some more minor improvements I've forgotten about. :)\n\n Makefile | 1 +\n t/t9503-gitweb-Mechanize.sh | 138 +++++++++++\n t/t9503/test.pl | 553 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 692 insertions(+), 0 deletions(-)\n create mode 100755 t/t9503-gitweb-Mechanize.sh\n create mode 100755 t/t9503/test.pl\n\ndiff --git a/Makefile b/Makefile\nindex a129491..3bf4f14 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1251,6 +1251,7 @@ GIT-CFLAGS: .FORCE-GIT-CFLAGS\n \n GIT-BUILD-OPTIONS: .FORCE-GIT-BUILD-OPTIONS\n \t@echo SHELL_PATH=\\''$(SHELL_PATH_SQ)'\\' >$@\n+\t@echo PERL_PATH=\\''$(PERL_PATH_SQ)'\\' >>$@\n \n ### Detect Tck/Tk interpreter path changes\n ifndef NO_TCLTK\ndiff --git a/t/t9503-gitweb-Mechanize.sh b/t/t9503-gitweb-Mechanize.sh\nnew file mode 100755\nindex 0000000..4585cea\n--- /dev/null\n+++ b/t/t9503-gitweb-Mechanize.sh\n@@ -0,0 +1,138 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2008 Jakub Narebski\n+# Copyright (c) 2008 Lea Wiemann\n+#\n+\n+# This test supports the --long-tests option.\n+\n+# This test only runs on Perl 5.8 and later versions, since\n+# Test::WWW::Mechanize::CGI requires Perl 5.8.\n+\n+test_description='gitweb tests (using WWW::Mechanize)\n+\n+This test uses Test::WWW::Mechanize::CGI to test gitweb.'\n+\n+# helper functions\n+\n+safe_chmod () {\n+\tchmod \"$1\" \"$2\" &&\n+\tif [ \"$(git config --get core.filemode)\" = false ]\n+\tthen\n+\t\tgit update-index --chmod=\"$1\" \"$2\"\n+\tfi\n+}\n+\n+. ./test-lib.sh\n+\n+# check if test can be run\n+\"$PERL_PATH\" -MEncode -e 'decode_utf8(\"\", Encode::FB_CROAK)' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, perl version is too old' :\n+\ttest_done\n+\texit\n+}\n+\n+\"$PERL_PATH\" -MTest::WWW::Mechanize::CGI -e '' >/dev/null 2>&1 || {\n+\ttest_expect_success \\\n+\t\t'skipping gitweb tests, Test::WWW::Mechanize::CGI not found' :\n+\ttest_done\n+\texit\n+}\n+\n+# set up test repository\n+test_expect_success 'set up test repository' '\n+\n+\techo \"Not an empty file.\" > file &&\n+\tgit add file &&\n+\ttest_tick && git commit -a -m \"Initial commit.\" &&\n+\tgit branch b &&\n+\n+\techo \"New file\" > new_file &&\n+\tgit add new_file &&\n+\ttest_tick && git commit -a -m \"File added.\" &&\n+\n+\tsafe_chmod +x new_file &&\n+\ttest_tick && git commit -a -m \"Mode changed.\" &&\n+\n+\tgit mv new_file renamed_file &&\n+\ttest_tick && git commit -a -m \"File renamed.\" &&\n+\n+\trm renamed_file &&\n+\tln -s file renamed_file &&\n+\ttest_tick && git commit -a -m \"File to symlink.\" &&\n+\tgit tag with-symlink &&\n+\n+\tgit rm renamed_file &&\n+\trm -f renamed_file &&\n+\ttest_tick && git commit -a -m \"File removed.\" &&\n+\n+\tcp file file2 &&\n+\tgit add file2 &&\n+\ttest_tick && git commit -a -m \"File copied.\" &&\n+\n+\techo \"New line\" >> file2 &&\n+\tsafe_chmod +x file2 &&\n+\ttest_tick && git commit -a -m \"Mode change and modification.\" &&\n+\n+\tmkdir dir1 &&\n+\techo \"New file\" >> dir1/file1 &&\n+\tgit add dir1/file1 &&\n+\ttest_tick && git commit -a -m \"File added in subdirectory.\" &&\n+\tgit tag -m \"creating a tag object\" tag-object\n+\n+\tgit checkout b &&\n+\techo \"Branch\" >> b &&\n+\tgit add b &&\n+\ttest_tick && git commit -a -m \"On branch\" &&\n+\tgit checkout master &&\n+\ttest_tick && git pull . b\n+'\n+\n+# set up empty repository\n+# TODO!\n+\n+# set up repositories for gitweb\n+# TODO!\n+\n+# set up gitweb configuration\n+safe_pwd=\"$(\"$PERL_PATH\" -MPOSIX=getcwd -e 'print quotemeta(getcwd)')\"\n+cat >gitweb_config.perl <.git/description <cwd !~ /trash directory$/;\n+\n+our $long_tests = $ENV{GIT_TEST_LONG}; # \"our\" so we can use \"local $long_tests\"\n+\n+eval { require Archive::Tar; };\n+my $archive_tar_installed = !$@\n+ or diag('Archive::Tar is not installed; no tests for valid snapshots');\n+\n+eval { require HTML::Lint; };\n+my $html_lint_installed = !$@\n+ or diag('HTML::Lint is not installed; no HTML validation tests');\n+\n+eval { require XML::Parser; };\n+my $xml_parser_installed = !$@\n+ or diag('XML::Parser is not installed; no tests for well-formed XML');\n+\n+sub rev_parse {\n+\tmy $name = shift;\n+\tchomp(my $hash = `git rev-parse $name 2> /dev/null`);\n+\t$hash or undef;\n+}\n+\n+sub get_type {\n+\tmy $name = shift;\n+\tchomp(my $type = `git cat-file -t $name 2> /dev/null`);\n+\t$type or undef;\n+}\n+\n+\n+package OurMechanize;\n+\n+use base qw( Test::WWW::Mechanize::CGI );\n+\n+my %page_cache;\n+# Cache requests.\n+sub _make_request {\n+\tmy ($self, $request) = (shift, shift);\n+\n+\tmy $response;\n+\tunless ($response = Storable::thaw($page_cache{$request->uri})) {\n+\t\t$response = $self->SUPER::_make_request($request, @_);\n+\t\t$page_cache{$request->uri} = Storable::freeze($response);\n+\t}\n+\treturn $response;\n+}\n+\n+# Fix whitespace problem.\n+sub cgi_application {\n+\tmy ($self, $application) = @_;\n+\n+\t# This subroutine was copied (and modified) from\n+\t# WWW::Mechanize::CGI 0.3, which is licensed 'under the same\n+\t# terms as perl itself' and thus GPL compatible.\n+\tmy $cgi = sub {\n+\t\t# Use exec, not the shell, to support embedded\n+\t\t# whitespace in the path to $application.\n+\t\t# http://rt.cpan.org/Ticket/Display.html?id=36654\n+\t\tmy $status = system $application $application;\n+\t\tmy $value = $status >> 8;\n+\n+\t\tcroak( qq/Failed to execute application '$application'. Reason: '$!'/ )\n+\t\t if ( $status == -1 );\n+\t\tcroak( qq/Application '$application' exited with value: $value/ )\n+\t\t if ( $value > 0 );\n+\t};\n+\n+\t$self->cgi($cgi);\n+}\n+\n+package main;\n+\n+\n+my @revisions = split /\\s/, `git-rev-list --first-parent HEAD`;\n+chomp(my @heads = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/heads`);\n+chomp(my @tags = map { (split('/', $_))[2] } `git-for-each-ref --sort=-committerdate refs/tags`);\n+my @tag_objects = grep { get_type($_) eq 'tag' } @tags;\n+chomp(my @root_entries = `git-ls-tree --name-only HEAD`);\n+my @files = grep { get_type(\"HEAD:$_\") eq 'blob' } @root_entries or die;\n+my @directories = grep { get_type(\"HEAD:$_\") eq 'tree' } @root_entries or die;\n+# Only test one file and directory each.\n+@files = $files[0] unless $long_tests;\n+@directories = $directories[0] unless $long_tests;\n+\n+my $gitweb = abs_path(File::Spec->catfile('..', '..', 'gitweb', 'gitweb.cgi'));\n+\n+my $mech = OurMechanize->new;\n+$mech->cgi_application($gitweb);\n+# On some systems(?) it's necessary to have %ENV here, otherwise the\n+# CGI process won't get *any* of the current environment variables\n+# (not even PATH, etc.)\n+$mech->env(%ENV,\n+\t GITWEB_CONFIG => $ENV{'GITWEB_CONFIG'},\n+\t SCRIPT_FILENAME => $gitweb,\n+\t $mech->env);\n+\n+# import config, predeclaring config variables\n+our $site_name;\n+require_ok($ENV{'GITWEB_CONFIG'})\n+\tor diag('Could not load gitweb config; some tests would fail');\n+\n+# Perform non-recursive checks on the current page, but do not check\n+# the status code.\n+my %verified_uris;\n+sub _verify_page {\n+\tmy ($uri, $fragment) = split '#', $mech->uri;\n+\tTODO: {\n+\t\tlocal $TODO = 'line number fragments can be broken for diffs and blames'\n+\t\t if $fragment && $fragment =~ /^l[0-9]+$/;\n+\t\t$mech->content_like(qr/(name|id)=\"$fragment\"/,\n+\t\t\t\t \"[auto] fragment #$fragment exists ($uri)\")\n+\t\t if $fragment;\n+\t}\n+\n+\treturn 1 if $verified_uris{$uri};\n+\t$verified_uris{$uri} = 1;\n+\n+\t# Internal errors yield 200 but cause gitweb.cgi to exit with\n+\t# non-zero exit code, which Mechanize::CGI translates to 500,\n+\t# so we don't really need to check for \"Software error\" here,\n+\t# provided that the test cases always check the status code.\n+\t#$mech->content_lacks('

Software error:

') or return 0;\n+\n+\t# Validate. This is fast, so we can do it even without\n+\t# $long_tests.\n+\t$mech->html_lint_ok('[auto] validate HTML') or return 0\n+\t if $html_lint_installed && $mech->is_html;\n+\tmy $content_type = $mech->response->header('Content-Type')\n+\t or die \"$uri does not have a Content-Type header\";\n+\tif ($xml_parser_installed && $content_type =~ /xml/) {\n+\t\teval { XML::Parser->new->parse($mech->content); };\n+\t\tok(!$@, \"[auto] check for XML well-formedness ($uri)\") or diag($@);\n+\t}\n+\tif ($archive_tar_installed && $uri =~ /sf=tgz/) {\n+\t\tmy $snapshot_file = File::Temp->new;\n+\t\tprint $snapshot_file $mech->content;\n+\t\tclose $snapshot_file;\n+\t\tmy $t = Archive::Tar->new;\n+\t\t$t->read($snapshot_file->filename, 1);\n+\t\tok($t->get_files, \"[auto] valid tgz snapshot ($uri)\");\n+\t}\n+\t# WebService::Validator::Feed::W3C would be nice to\n+\t# use, but it doesn't support direct input (as opposed\n+\t# to URIs) long enough for our feeds.\n+\n+\treturn 1;\n+}\n+\n+# Verify and spider the current page, the latter only if --long-tests\n+# (-l) is given. Do not check the status code of the current page.\n+my %spidered_uris; # pages whose links have been checked\n+my %status_checked_uris; # verified pages whose status is known to be 2xx\n+sub check_page {\n+\t_verify_page or return 0;\n+\tif ($long_tests && !$spidered_uris{$mech->uri} ) {\n+\t\t$spidered_uris{$mech->uri} = 1;\n+\t\tmy $orig_url = $mech->uri;\n+\t\tTODO: {\n+\t\t\tlocal $TODO = \"blame links can be broken sometimes\"\n+\t\t\t if $orig_url =~ /a=blame/;\n+\t\t\tfor my $url (map { $_->url_abs } $mech->followable_links) {\n+\t\t\t\tif (!$status_checked_uris{$url}) {\n+\t\t\t\t\t$status_checked_uris{$url} = 1;\n+\t\t\t\t\tlocal $long_tests = 0; # stop recursing\n+\t\t\t\t\ttest_page($url, \"[auto] check link ($url)\")\n+\t\t\t\t\t or diag(\"broken link to $url on $orig_url\");\n+\t\t\t\t\t$mech->back;\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn 1;\n+}\n+\n+my $baseurl = \"http://localhost\";\n+my ($params, $url, $pagedesc, $status);\n+\n+# test_page ( , , )\n+# Example:\n+# if (test_page('?p=.git;a=summary', 'repository summary')) {\n+# $mech->...;\n+# $mech->...;\n+# }\n+#\n+# Test that the page can be opened, call _verify_page on it, and\n+# return true if there was no test failure. Also set the global\n+# variables $params, $pagedesc, and $url for use in the if block.\n+# Optionally pass a third parameter $status to test the HTTP status\n+# code of the page (useful for error pages). You can also pass a full\n+# URL instead of just parameters as the first parameter.\n+sub test_page {\n+\t($params, $pagedesc, $status) = @_;\n+\t# missing $pagedesc is usually accidental\n+\tdie \"$params: no pagedesc given\" unless defined $pagedesc;\n+\tif($params =~ /^$baseurl/) {\n+\t\t$url = \"$params\";\n+\t} else {\n+\t\t$url = \"$baseurl$params\";\n+\t}\n+\t$mech->get($url);\n+\tlike($mech->status, $status ? qr/$status/ : qr/^[23][0-9][0-9]$/,\n+\t \"$pagedesc: $url\" . ($status ? \" -- yields $status\" : \"\"))\n+\t or return 0;\n+\tif ($mech->status =~ /^3/) {\n+\t\t# Don't check 3xx, they tend to look funny.\n+\t\tmy $location = $mech->response->header('Location');\n+\t\t$mech->back; # compensate for history\n+\t\treturn test_page($location, \"follow redirect from $url\");\n+\t} else {\n+\t\treturn check_page;\n+\t}\n+}\n+\n+# follow_link ( \\%parms, $pagedesc )\n+# Example:\n+# if (follow_link( { text => 'commit' }, 'first commit link')) {\n+# $mech->...;\n+# $mech->back;\n+# }\n+# Like test_page, but does not support status code testing, and\n+# returns true if there was a link at all, regardless of whether it\n+# was [23]xx or not.\n+sub follow_link {\n+\t(my $parms, $pagedesc) = @_;\n+\tmy $link = $mech->find_link(%$parms);\n+\tmy $current_url = $mech->uri;\n+\tok($link, \"link exists: $pagedesc (on page $current_url)\") or return 0;\n+\ttest_page($link->url, \"follow link: $pagedesc (on page $current_url)\");\n+\treturn 1;\n+}\n+\n+# like follow_link, except that only checks and goes back immediately;\n+# use this instead of ok(find_link...).\n+sub test_link {\n+\tmy ($parms, $pagedesc) = @_;\n+\tmy $current_url = $mech->uri;\n+\tif($long_tests) {\n+\t\t# Check status, validate, spider.\n+\t\treturn follow_link($parms, $pagedesc) && $mech->back;\n+\t} else {\n+\t\t# Only check presence of the link (much faster).\n+\t\treturn ok($mech->find_link(%$parms),\n+\t\t\t \"link exists: $pagedesc (on page $current_url)\");\n+\t}\n+}\n+\n+sub get_summary {\n+\ttest_page('?p=.git', 'repository summary')\n+}\n+\n+if (test_page '', 'project list (implicit)') {\n+\t$mech->title_like(qr!$site_name!,\n+\t\t\"title contains $site_name\");\n+\t$mech->content_contains('gitweb test repository',\n+\t\t'lists test repository (by description)');\n+}\n+\n+# Test repository summary: implicit, implicit with pathinfo, explicit.\n+for my $sumparams ('?p=.git', '/.git', '?p=.git;a=summary') {\n+\tif (test_page $sumparams, 'repository summary') {\n+\t\t$mech->title_like(qr!$site_name.*\\.git/summary!,\n+\t\t\t\t \"title contains $site_name and \\\".git/summary\\\"\");\n+\t}\n+}\n+\n+# Search form (on summary page).\n+if (get_summary && $mech->submit_form_ok(\n+\t { form_number => 1, fields => { 's' => 'Initial' } },\n+\t 'submit search form (default)')) {\n+\tcheck_page;\n+\t$mech->content_contains('Initial commit',\n+\t\t\t\t'content contains commit we searched for');\n+}\n+\n+test_page('?p=non-existent.git', 'non-existent project', 404);\n+test_page('?p=.git;a=commit;h=non-existent', 'non-existent commit', 404);\n+\n+\n+# Summary view\n+\n+get_summary or die 'summary page failed; no point in running the remaining tests';\n+\n+# Check short log. To do: Extract into separate test_short_log\n+# function since the short log occurs on several pages.\n+for my $revision (@revisions) {\n+\tfor my $link_text qw( commit commitdiff tree snapshot ) {\n+\t\ttest_link( { url_abs_regex => qr/h=$revision/, text => $link_text },\n+\t\t\t \"$link_text link for $revision\");\n+\t}\n+}\n+\n+# Check that branches and tags are highlighted in green and yellow in\n+# the shortlog. We assume here that we are on master, so it should be\n+# at the top.\n+$mech->content_like(qr{]*class=\"head\"[^>]*>master},\n+\t\t 'master branch is highlighted in shortlog');\n+$mech->content_like(qr{]*class=\"tag\"[^>]*>$tags[0]},\n+\t\t \"$tags[0] (most recent tag) is highlighted in shortlog\");\n+\n+# Check heads. (This should be extracted as well.)\n+for my $head (@heads) {\n+\tfor my $link_text qw( shortlog log tree ) {\n+\t\ttest_link( { url_abs_regex => qr{h=refs/heads/$head}, text => $link_text },\n+\t\t\t \"$link_text link for head '$head'\");\n+\t}\n+}\n+\n+# Check tags (assume we only have tags referring to commits, not to\n+# blobs or trees).\n+for my $tag (@tags) {\n+\tmy $commit = rev_parse(\"$tag^{commit}\");\n+\ttest_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'shortlog' },\n+\t\t \"shortlog link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=refs/tags/$tag}, text => 'log' },\n+\t\t \"log link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=$commit}, text => 'commit' },\n+\t\t \"commit link for tag '$tag'\");\n+\ttest_link( { url_abs_regex => qr{h=$commit}, text => $tag },\n+\t \"'$tag' links to the commit as well\");\n+\t# To do: Test tag link for tag objects.\n+\t# Why don't we have tree + snapshot links?\n+}\n+for my $tag (@tag_objects) {\n+\tmy $tag_sha1 = rev_parse($tag);\n+\ttest_link( { url_abs_regex => qr{h=$tag_sha1}, text => 'tag' },\n+\t\t \"tag link for tag object '$tag'\" );\n+}\n+\n+\n+# RSS/Atom/OPML view\n+# Simply retrieve and verify well-formedness, but don't spider.\n+$mech->get_ok('?p=.git;a=atom', 'Atom feed') and _verify_page;\n+$mech->get_ok('?p=.git;a=rss', 'RSS feed') and _verify_page;\n+TODO: {\n+\t# Now spider -- but there are broken links.\n+\t# http://mid.gmane.org/485EB333.5070108@gmail.com\n+\tlocal $TODO = \"fix broken links in Atom/RSS feeds\";\n+\ttest_page('?p=.git;a=atom', 'Atom feed');\n+\ttest_page('?p=.git;a=rss', 'RSS feed');\n+}\n+test_page('?a=opml', 'OPML outline');\n+\n+\n+# Commit view\n+if (test_page('?p=.git;a=commit;h=master', 'view HEAD commit')) {\n+\tmy $tree_sha1 = rev_parse('master:');\n+\ttest_link( { url_abs_regex => qr/a=tree/, text => rev_parse('master:') },\n+\t\t \"SHA1 link to tree on commit page ($url)\");\n+\ttest_link( { url_abs_regex => qr/h=$tree_sha1/, text => 'tree' },\n+\t\t \"'tree' link to tree on commit page ($url)\");\n+\t$mech->content_like(qr/A U Thor/, \"author mentioned on commit page ($url)\");\n+}\n+\n+\n+# Commitdiff view\n+if (get_summary &&\n+ follow_link( { text_regex => qr/file added/i }, 'commit with added file') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\t$mech->content_like(qr/new file with mode/, \"commitdiff has diffstat ($url)\");\n+\t$mech->content_like(qr/new file mode/, \"commitdiff has diff ($url)\");\n+}\n+test_page(\"?p=.git;a=commitdiff;h=$revisions[-1]\",\n+\t 'commitdiff without parent');\n+\n+# Diff formattting problem.\n+if (get_summary &&\n+ follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&\n+ follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n+\tTODO: {\n+\t\tlocal $TODO = \"bad a/* link in diff\";\n+\t\tif (follow_link( { text_regex => qr!^a/! },\n+\t\t\t\t 'a/* link (probably wrong)')) {\n+\t\t\t# The page we land on here is broken already.\n+\t\t\tfollow_link( { url_abs_regex => qr/a=blob_plain/ },\n+\t\t\t\t 'linked file name'); # bang\n+\t\t}\n+\t}\n+}\n+\n+\n+# Raw commitdiff (commitdiff_plain) view\n+if (test_page('?p=.git;a=commit;h=refs/tags/tag-object',\n+\t 'commit view of tags/tag-object') &&\n+ follow_link( { text => 'commitdiff' }, \"'commitdiff'\") &&\n+ follow_link( { text => 'raw' }, \"'raw' (commitdiff_plain)\")) {\n+\t$mech->content_like(qr/^From: A U Thor $/m,\n+\t\t\t 'commitdiff_plain: From header');\n+\tTODO: {\n+\t\tlocal $TODO = 'date header mangles timezone';\n+\t\t$mech->content_like(qr/^Date: Thu, 7 Apr 2005 15:..:13 -0700$/m,\n+\t\t\t\t 'commitdiff_plain: Date header (correct)');\n+\t}\n+\t$mech->content_like(qr/^Date: Thu, 7 Apr 2005 22:..:13 \\+0000 \\(-0700\\)$/m,\n+\t\t\t 'commitdiff_plain: Date header (UTC, wrong)');\n+\t$mech->content_like(qr/^Subject: .+$/m,\n+\t\t\t 'commitdiff_plain: Subject header');\n+\t# $ markers inexplicably don't work here if we use like(...)\n+\t# or $mech->content_like().\n+\tok($mech->content =~ /^X-Git-Tag: tag-object\\^0$/m,\n+\t 'commitdiff_plain: X-Git-Tag header');\n+\tok($mech->content =~ /^X-Git-Url: $baseurl\\?p=\\.git;a=commitdiff_plain;h=refs%2Ftags%2Ftag-object$/m,\n+\t 'commitdiff_plain: X-Git-Url header');\n+\tok($mech->content =~ /^---$/m, 'commitdiff_plain: separator$');\n+\t$mech->content_like(qr/^diff --git /m, 'commitdiff_plain: diff$');\n+}\n+\n+\n+# Tree view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tmy $sha1 = rev_parse(\"HEAD:$file\");\n+\t\ttest_link( { text => $file, url_abs_regex => qr/h=$sha1/ },\n+\t\t\t \"'$file' is listed and linked\");\n+\t\ttest_link({ url_abs_regex => qr/f=$file/, text => $_ },\n+\t\t\t \"'$_' link\") foreach qw( blame blob history raw );\n+\t}\n+\tfor my $directory (@directories) {\n+\t\tmy $sha1 = rev_parse(\"HEAD:$directory\");\n+\t\ttest_link({ url_abs_regex => qr/f=$directory/, text => $_ },\n+\t\t\t \"'$_' link\") foreach qw( tree history );\n+\t\tif(follow_link( { text => $directory, url_abs_regex => qr/h=$sha1/ },\n+\t\t\t\t\"'$directory is listed and linked\" )) {\n+\t\t\tif(follow_link( { text => '..' }, 'parent directory')) {\n+\t\t\t\ttest_link({ url_abs_regex => qr/h=$sha1/,\n+\t\t\t\t\t text => $directory },\n+\t\t\t\t\t 'back to original tree view');\n+\t\t\t\t$mech->back;\n+\t\t\t}\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blame view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $blame_link ($mech->find_all_links(text => 'blame')) {\n+\t\tmy $url = $blame_link->url;\n+\t\t$mech->get_ok($url, \"get $url -- blame link on tree view\")\n+\t\t and _verify_page;\n+\t\t$mech->content_like(qr/A U Thor/,\n+\t\t\t\t \"author mentioned on blame page\");\n+\t\tTODO: {\n+\t\t\t# Now spider -- but there are broken links.\n+\t\t\t# http://mid.gmane.org/485EC621.7090101@gmail.com\n+\t\t\tlocal $TODO = \"fix broken links in certain blame views\";\n+\t\t\tcheck_page;\n+\t\t}\n+\t\tlast unless $long_tests; # only test first blame link\n+\t}\n+}\n+\n+\n+# History view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files, @directories) {\n+\t\tmy $type = get_type(\"HEAD:$file\"); # blob or tree\n+\t\tif (follow_link( { text => 'history', url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"history link for '$file'\")) {\n+\t\t\t# There is at least one commit, so A U Thor is mentioned.\n+\t\t\t$mech->content_contains('A U Thor', 'A U Thor mentioned');\n+\t\t\t# The following tests test for at least *one*\n+\t\t\t# link of each type and are weak since we\n+\t\t\t# don't have any knowledge of commit hashes.\n+\t\t\ttest_link( { text => $type, url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"$type\");\n+\t\t\ttest_link( { text => 'commitdiff' },\n+\t\t\t\t \"commitdiff\");\n+\t\t\ttest_link( { url_abs_regex => qr/a=commit;.*h=[a-f0-9]{40}/ },\n+\t\t\t\t \"subject links to commit\"); # weak, brittle\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Blob view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => $file, url_abs_regex => qr/a=blob/ },\n+\t\t\t\t \"\\\"$file\\\" (blob) entry on tree view\")) {\n+\t\t\tchomp(my $first_line_regex = (`cat \"$file\"`)[0]);\n+\t\t\t$first_line_regex =~ s/ / | /g;\n+\t\t\t# Hope that the first line doesn't contain any\n+\t\t\t# HTML-escapable character.\n+\t\t\t$mech->content_like(qr/$first_line_regex/,\n+\t\t\t\t\t \"blob view contains first line of file ($url)\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Raw (blob_plain) view\n+if (get_summary && follow_link( { text => 'tree' }, 'first tree link')) {\n+\tfor my $file (@files) {\n+\t\tif (follow_link( { text => 'raw', url_abs_regex => qr/f=$file/ },\n+\t\t\t\t \"raw (blob_plain) entry for \\\"$file\\\" in tree view\")) {\n+\t\t\tchomp(my $first_line = (`cat \"$file\"`)[0]);\n+\t\t\t$mech->content_contains(\n+\t\t\t\t$first_line, \"blob_plain view contains first line of file\");\n+\t\t\t$mech->back;\n+\t\t}\n+\t}\n+}\n+\n+\n+# Error handling\n+# Pass valid and invalid paths to various file-based actions\n+for my $action qw( blame blob blob_plain blame ) {\n+\ttest_page(\"?p=.git;a=$action;f=$files[0];hb=HEAD\",\n+\t\t \"$action: look up existent file\");\n+\ttest_page(\"?p=.git;a=$action;f=does_not_exist;hb=HEAD\",\n+\t\t \"$action: look up non-existent file\", 404);\n+\tTODO: {\n+\t\tlocal $TODO = 'wrong error code (but using Git::Repo will fix this)';\n+\t\ttest_page(\"?p=.git;a=$action;f=$directories[0];hb=HEAD\",\n+\t\t\t \"$action: look up directory\", 400);\n+\t}\n+}\n+TODO: {\n+\tlocal $TODO = 'wrong error code (but using Git::Repo will fix this)';\n+\ttest_page(\"?p=.git;a=tree;f=$files[0];hb=HEAD\",\n+\t\t 'tree: look up existent file', 400);\n+}\n+# Pass valid and invalid paths to tree action\n+test_page(\"?p=.git;a=tree;f=does_not_exist;hb=HEAD\",\n+\t 'tree: look up non-existent file', 404);\n+test_page(\"?p=.git;a=tree;f=$directories[0];hb=HEAD\",\n+\t 'tree: look up directory');\n+TODO: {\n+\tlocal $TODO = 'cannot use f=/ or f= for trees';\n+\ttest_page(\"?p=.git;a=tree;f=/;hb=HEAD\", 'tree: look up directory');\n+}\n+\n+\n+1;\n+__END__\n-- \n1.5.6.109.g33ded.dirty\n"},{"id":"81682","messageId":"200806300047.12224.jnareb@gmail.com","threadId":"13950","inReplyTo":"1214488126-6783-1-git-send-email-LeWiemann@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-29T22:47:11Z","receivedAt":"2008-06-29T22:47:11Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Thu, 26 June 2008, Lea Wiemann wrote:\n\n> This test uses Test::WWW::Mechanize::CGI to check gitweb's output. It\n> also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\n\nWouldn't it be better to use \"(each if present)\"?\n\nI am not a native English speaker, though.\n\n[...]\n> Changes since v7:\n> \n> - In Makefile, dump $PERL_PATH to GIT-BUILD-OPTIONS (which gets\n> source'd by test-lib.sh), and use it in t9710-perl-git-repo.sh.\n\nGood idea.\n\n> - .git/description in the test repository no longer depends on $0\n> (this would e.g. cause 'cd t; make' to fail).\n\n> +cat >.git/description < +gitweb test repository\n> +EOF\n\nI'd rather use more descriptive name, so when you look it up in gitweb\nto manually (visually?) checks its output you would know of which test\nis it. Something like \"gitweb Mechanize test repository\", or \"test\nrepository for gitweb's Mechanize test\".\n\n> - Add test_link subroutine and use it everywhere in place of\n> ok(find_link...) so that links whose presence get tested get checked\n> and spidered in --long-tests mode.\n\nNice.\n\n> - Add simple page caching (reduces execution time without --long-tests\n> by 25%). That's *really* helpful when you have to run those tests\n> on a regular basis. ;)\n\nWhat do you need caching for? You check if page was already accessed\nwhen spidering...\n\nIf it is about checking links, alternate solution would be to replace\nsimple $mech->page_links_ok( [ $desc ] ) by finding all the links\neither using $mech->followable_links() or $mech->find_all_links( ... ),\nor just $mech->links, then filtering out links which you have checked\nalready, then checking selected links using $mech->links_ok( $links [, $desc ] )\n\n> (WWW::Mechanize::Cached won't work with\n> TWM::CGI, so we have to implement it ourselves; but it's easier\n> anyway.)\n\n> +package OurMechanize;\n> +\n> +use base qw( Test::WWW::Mechanize::CGI );\n\nWhy this package is not named WWW::Mechanize::CGI::Cached, I wonder?\nIs it because of corrected cgi_application method?\n\n> +\n> +my %page_cache;\n> +# Cache requests.\n> +sub _make_request {\n> +\tmy ($self, $request) = (shift, shift);\n> +\n> +\tmy $response;\n> +\tunless ($response = Storable::thaw($page_cache{$request->uri})) {\n> +\t\t$response = $self->SUPER::_make_request($request, @_);\n> +\t\t$page_cache{$request->uri} = Storable::freeze($response);\n> +\t}\n> +\treturn $response;\n> +}\n> +\n> +# Fix whitespace problem.\n> +sub cgi_application {\n> +\tmy ($self, $application) = @_;\n> +\n> +\t# This subroutine was copied (and modified) from\n> +\t# WWW::Mechanize::CGI 0.3, which is licensed 'under the same\n> +\t# terms as perl itself' and thus GPL compatible.\n> +\tmy $cgi = sub {\n> +\t\t# Use exec, not the shell, to support embedded\n> +\t\t# whitespace in the path to $application.\n> +\t\t# http://rt.cpan.org/Ticket/Display.html?id=36654\n> +\t\tmy $status = system $application $application;\n> +\t\tmy $value = $status >> 8;\n> +\n> +\t\tcroak( qq/Failed to execute application '$application'. Reason: '$!'/ )\n> +\t\t if ( $status == -1 );\n> +\t\tcroak( qq/Application '$application' exited with value: $value/ )\n> +\t\t if ( $value > 0 );\n> +\t};\n> +\n> +\t$self->cgi($cgi);\n> +}\n> +\n\nBy the way, if you want to add a comment to mentioned WM::CGI ticket\nhttp://rt.cpan.org/Ticket/Display.html?id=36654 you have to either\nregister, or send comment via mail with the following info\n\n>From \"Bugs in WWW-Mechanize-CGI via RT\" :\n|\n| Please include the string:\n|\n| [rt.cpan.org #36654]\n|\n| in the subject line of all future correspondence about this issue. To do so, \n| you may reply to this message.\n\n> - Follow redirects rather than failing.\n\nNice. Where it is?\n \n> - Test subdirectories in tree view.\n> - Test error handling for non-existent hashes or hashes of wrong type.\n> - Test commitdiff_plain view.\n> - Expand test for history view.\n> - Test tag objects (not just symbolic tags) in tag list.\n\nMore tests are always nice to have, although I begin to wonder if\nsplitting this test into smaller part wouldn't be a good idea...\n\n> - Test a specific bug (under \"diff formatting\", marked TODO).\n\n> +# Diff formattting problem.\n\n(One 't' too many: should be \"Diff formatting problem\")\n\n> +if (get_summary &&\n> + follow_link( { text_regex => qr/renamed/ }, 'commit with rename') &&\n> + follow_link( { text => 'commitdiff' }, 'commitdiff')) {\n> +\tTODO: {\n> +\t\tlocal $TODO = \"bad a/* link in diff\";\n> +\t\tif (follow_link( { text_regex => qr!^a/! },\n> +\t\t\t\t 'a/* link (probably wrong)')) {\n> +\t\t\t# The page we land on here is broken already.\n> +\t\t\tfollow_link( { url_abs_regex => qr/a=blob_plain/ },\n> +\t\t\t\t 'linked file name'); # bang\n> +\t\t}\n> +\t}\n> +}\n \nI'll try to investigate; I guess this uses wrong name or wrong hash\nfor preimage.\n\n> - Do not test correctness of line number fragments (#l[0-9]+); they're\n> broken too often right now.\n\nWhat do you mean by broken?\n\n> - Probably some more minor improvements I've forgotten about. :)\n\nFor example\n\n> +die \"this must be run by calling the t/t*.sh shell script(s)\\n\"\n> + if Cwd->cwd !~ /trash directory$/;\n\n \nGood work!\n\n-- \nJakub Narebski\nPoland\n"},{"id":"81686","messageId":"48681D21.1040302@gmail.com","threadId":"13950","inReplyTo":"200806300047.12224.jnareb@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-29T23:39:13Z","receivedAt":"2008-06-29T23:39:13Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> On Thu, 26 June 2008, Lea Wiemann wrote:\n>> It also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)\n> \n> Wouldn't it be better to use \"(each if present)\"?\n> I am not a native English speaker, though.\n\nMe neither, but I prefer \"if present, each\" off the top of my head...\n\n>> - Add simple page caching (reduces execution time without --long-tests\n>> by 25%). That's *really* helpful when you have to run those tests\n>> on a regular basis. ;)\n> \n> What do you need caching for? You check if page was already accessed\n> when spidering...\n\nThis is actually about the short tests (I don't think it gains much for\nspidering, percentage-wise). The test suite uses some repetitive set-up\ncode (like get_summary && follow_link 'tree' && ...), so the second and\nthird times these pages are loaded we can save some time.\n\n> replace $mech->page_links_ok [with] finding all the links [...], \n> then filtering out links which you have checked already, then checking\n> selected links using $mech->links_ok\n\nThat's basically what it does right now. :)\n\n>> +package OurMechanize;\n>> +use base qw( Test::WWW::Mechanize::CGI );\n> \n> Why this package is not named WWW::Mechanize::CGI::Cached, I wonder?\n> Is it because of corrected cgi_application method?\n\nYeah, basically. :) Plus, it's not to be confused with a proper\nimplementation of a TWM::CGI::Cached package. (For instance, it uses\nthe URL rather than the complete request as cache key. Hence it ignores\nheaders like Referer; but that's great for the Gitweb tests since TWM\napparently sets the Referer, but Gitweb doesn't check it, so there's no\nneed to refetch a page because the Referer has changed.)\n\n> By the way, if you want to add a comment to mentioned WM::CGI ticket [...]\n\nThanks; done.\n\n>> - Follow redirects rather than failing.\n> \n> Nice. Where it is?\n\ntest_page doesn't use $mech->get_ok anymore, but rather calls $mech->get\nand checks that the status is [23][0-9][0-9]. If it's 3xx, it also\nfollows the redirect.\n\n> I begin to wonder if\n> splitting this test into smaller part wouldn't be a good idea...\n\nIt's not yet painful (or long-running) enough for me to care. :) I'm\nfine with a 500-lines test module.\n\n>> +# Diff formattting problem.\n> \n> (One 't' too many:\n\nFixed.\n\n>> +\t\t\tfollow_link( { url_abs_regex => qr/a=blob_plain/ },\n>> +\t\t\t\t 'linked file name'); # bang\n> \n> I'll try to investigate; I guess this uses wrong name or wrong hash\n> for preimage.\n\nThanks!\n\n>> - Do not test correctness of line number fragments (#l[0-9]+); they're\n>> broken too often right now.\n> \n> What do you mean by broken?\n\nThis is only visible with --long-tests -- there are links to line\nnumbers that don't exist (IOW the fragment doesn't exist in a name or id\nattribute on the target page). I've even seen links to #l0. Bug fixes\nwelcome. ;-)\n\n(There are also some other [mostly minor] issues marked with \"TODO:\" in\nthe test code; I didn't want to swamp the list with half a dozen bug\nreports though, apart from time constraints on my end.)\n\n> Good work!\n\nThanks! :-) I'll send v9 in (hopefully) 2-3 days, together with the\nfirst working version of the caching code.\n\n-- Lea\n"},{"id":"81687","messageId":"200806300156.28908.jnareb@gmail.com","threadId":"13950","inReplyTo":"48681D21.1040302@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-29T23:56:27Z","receivedAt":"2008-06-29T23:56:27Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 30 June 2008, Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>> On Thu, 26 June 2008, Lea Wiemann wrote:\n>>>\n>>> - Follow redirects rather than failing.\n>> \n>> Nice. Where it is?\n> \n> test_page doesn't use $mech->get_ok anymore, but rather calls $mech->get\n> and checks that the status is [23][0-9][0-9]. If it's 3xx, it also\n> follows the redirect.\n\nThanks for the info.\n\nBy the way, some time ago I have send a patch (dropped, perhaps because\nof it being feature freeze, or just lost) which converted support for\nlinks with hash and without action (introduced in 7f9778b by Gerrit\nPape) to use redirect (like for 'object' action) instead of silently\nfilling correct action based on type of object (given by hash). IMHO\nit is better as it should prevent bookmarking \"expensive\" URL.\n\nSo this is useful, and could/would be even more useful.\n\n>>> - Do not test correctness of line number fragments (#l[0-9]+); they're\n>>> broken too often right now.\n>> \n>> What do you mean by broken?\n> \n> This is only visible with --long-tests -- there are links to line\n> numbers that don't exist (IOW the fragment doesn't exist in a name or id\n> attribute on the target page). I've even seen links to #l0. Bug fixes\n> welcome. ;-)\n\nI wonder if those are intentional (or at least known) breaking, to form\napproximate blame file history browsing; there was discussion about it\non git mailing list some time ago (it resulted in adding \"parent\" line\nto blame porcelain/incremental output... or was it only in 'pu'?).\n \n>> Good work!\n> \n> Thanks! :-) I'll send v9 in (hopefully) 2-3 days, together with the\n> first working version of the caching code.\n\nOne issue of note, after brief peek at Git::Repo code: you should not\nerror out on unknown header in commit object, but either save its value\nunder its name, or just skip it.\n\nUnless this has changed...\n\n-- \nJakub Narebski\nPoland\n"},{"id":"81688","messageId":"48682934.6090406@gmail.com","threadId":"13950","inReplyTo":"200806300156.28908.jnareb@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Lea Wiemann","fromEmail":"lewiemann@gmail.com","sentAt":"2008-06-30T00:30:44Z","receivedAt":"2008-06-30T00:30:44Z","isPatch":true,"sender":{"key":"lewiemann@gmail.com","avatar":null},"body":"Jakub Narebski wrote:\n> some time ago I have send a patch which converted support for\n> links with hash and without action to use redirect instead of\n> silently filling correct action based on type of object [...]\n> IMHO it is better as it should prevent bookmarking \"expensive\" URL.\n\nHaha, I'd actually suggest the opposite. ;-) Figuring out the right\naction is almost free since you have to fetch the object anyways, so I\ndoubt it'll make any difference performance-wise (though it'd be\ninteresting to benchmark this). However, gitweb's URLs are\nprohibitively long -- so that nobody uses them in email --, and\n(automatically?) dropping the action parameter where possible would be a\ngood first step to shortening them. Another idea would be to shorten\nthe hashes.\n\n>> there are links to line numbers that don't exist\n> \n> I wonder if those are intentional (or at least known) breaking, to form\n> approximate blame file history browsing;\n\n*nods* I wasn't following this in detail; if it turns out to be\nunfixable, we could also remove the fragment checks for line numbers\n(rather than running them as \"TODO:\" tests).\n\n> [Git::Commit:] you should not error out on unknown header in commit object,\n\nUnless this can actually happen in practice, I'd rather die aggressively\n-- it prevents errors (like cache hiccups) slipping through unnoticed.\n"},{"id":"81777","messageId":"200806302355.50560.jnareb@gmail.com","threadId":"13950","inReplyTo":"48682934.6090406@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-30T21:55:49Z","receivedAt":"2008-06-30T21:55:49Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On Mon, 30 Jun 2008, Lea Wiemann wrote:\n> Jakub Narebski wrote:\n>>\n>> some time ago I have send a patch which converted support for\n>> links with hash and without action to use redirect instead of\n>> silently filling correct action based on type of object [...]\n>> IMHO it is better as it should prevent bookmarking \"expensive\" URL.\n> \n> Haha, I'd actually suggest the opposite. ;-) Figuring out the right\n> action is almost free since you have to fetch the object anyways, so I\n> doubt it'll make any difference performance-wise (though it'd be\n> interesting to benchmark this).\n\nIt is one fork more[*1*], which doesn't matter on operating systems\nlike Linux where forking is fairly cheap.\n\nBut perhaps avoiding redirect would be better solution... well, yet\nanother solution would be to leave it up to gitweb configuration ;-)\n\n[*1*] Or pre-fetching object (together with its type) which might be\nlarge, into memory.\n\n> However, gitweb's URLs are \n> prohibitively long -- so that nobody uses them in email --, and\n> (automatically?) dropping the action parameter where possible would be a\n> good first step to shortening them. Another idea would be to shorten\n> the hashes.\n\nAbout shortening the hashes: _usually_ 8. characters is enough... but\nsometimes it isn't. What then?\n\n[...]\n>> [Git::Commit:] you should not error out on unknown header in commit object,\n> \n> Unless this can actually happen in practice, I'd rather die aggressively\n> -- it prevents errors (like cache hiccups) slipping through unnoticed.\n\nIf Git::Commit is to be *generic* object oriented interface to git\nrepository (and not only for gitweb), it has either to skip unknown\ncommit headers, or save them as-is.\n\nAs to checking cache: you can always check if header name is sane...\n-- \nJakub Narebski\nPoland\n"},{"id":"81779","messageId":"200807010001.42373.jnareb@gmail.com","threadId":"13950","inReplyTo":"48681EC8.8000606@gmail.com","subject":"Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2008-06-30T22:01:42Z","receivedAt":"2008-06-30T22:01:42Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"[back-to-list]\n\nOn Mon, 30 Jun 2008, Lea Wiemann wrote:\n> [off-list]\n[...]\n \n> By the way, I found the supercat (spc) colorizer very helpful for the\n> mechanize tests, since the output tends to be long and the lines wrap a\n> lot. Here are my regexes:\n> \n> $ cat ~/.spcrc/spcrc-test_more\n> # HTML COLOR NAME COL A N T STRING or REGULAR EXPRESSION\n> #################### ### # # # #########################################\n> Black blk r (.*)\n> Green grn b r (^ok .*)\n> Red red b r (^not ok)\n> Yellow yel b r (^not ok .* # TODO.*)\n> Green grn r (^ok .*\\[auto\\].*)\n> Red red r (^not ok .*\\[auto\\].*)\n> Yellow yel r (^not ok .*\\[auto\\].* # TODO.*)\n> Black blk r ( - .*)\n> Green grn r r (\\* passed all .*)\n> Red red r r (\\* failed .*)\n> \n> Then you can just run\n> \n> ./t9503-gitweb-Mechanize.sh -v -l 2>&1 | spc -t test_more\n> \n> Hope it's useful!\n\nHmmm... it would be nice if 'test_external*' functions supported syntax\nhighlighting of command output if it follows TAP: Test Anything Protocol\nhttp://www.perlfoundation.org/perl5/index.cgi?testing#tap_test_anything_protocol\nin a manner similar to coloring native tests (using git_expect_success,\ngit_expect_failure etc.).\n\nTest::More as far as I know produces TAP-compatibile output.\n\n-- \nJakub Narebski\nPoland\n"}]}