From: Lea Wiemann <lewiemann@gmail.com>
To: Jakub Narebski <jnareb@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v8] gitweb: add test suite with Test::WWW::Mechanize::CGI
Date: Mon, 30 Jun 2008 01:39:13 +0200 [thread overview]
Message-ID: <48681D21.1040302@gmail.com> (raw)
In-Reply-To: <200806300047.12224.jnareb@gmail.com>
Jakub Narebski wrote:
> On Thu, 26 June 2008, Lea Wiemann wrote:
>> It also uses HTML::Lint, XML::Parser, and Archive::Tar (if present, each)
>
> Wouldn't it be better to use "(each if present)"?
> I am not a native English speaker, though.
Me neither, but I prefer "if present, each" off the top of my head...
>> - Add simple page caching (reduces execution time without --long-tests
>> by 25%). That's *really* helpful when you have to run those tests
>> on a regular basis. ;)
>
> What do you need caching for? You check if page was already accessed
> when spidering...
This is actually about the short tests (I don't think it gains much for
spidering, percentage-wise). The test suite uses some repetitive set-up
code (like get_summary && follow_link 'tree' && ...), so the second and
third times these pages are loaded we can save some time.
> replace $mech->page_links_ok [with] finding all the links [...],
> then filtering out links which you have checked already, then checking
> selected links using $mech->links_ok
That's basically what it does right now. :)
>> +package OurMechanize;
>> +use base qw( Test::WWW::Mechanize::CGI );
>
> Why this package is not named WWW::Mechanize::CGI::Cached, I wonder?
> Is it because of corrected cgi_application method?
Yeah, basically. :) Plus, it's not to be confused with a proper
implementation of a TWM::CGI::Cached package. (For instance, it uses
the URL rather than the complete request as cache key. Hence it ignores
headers like Referer; but that's great for the Gitweb tests since TWM
apparently sets the Referer, but Gitweb doesn't check it, so there's no
need to refetch a page because the Referer has changed.)
> By the way, if you want to add a comment to mentioned WM::CGI ticket [...]
Thanks; done.
>> - Follow redirects rather than failing.
>
> Nice. Where it is?
test_page doesn't use $mech->get_ok anymore, but rather calls $mech->get
and checks that the status is [23][0-9][0-9]. If it's 3xx, it also
follows the redirect.
> I begin to wonder if
> splitting this test into smaller part wouldn't be a good idea...
It's not yet painful (or long-running) enough for me to care. :) I'm
fine with a 500-lines test module.
>> +# Diff formattting problem.
>
> (One 't' too many:
Fixed.
>> + follow_link( { url_abs_regex => qr/a=blob_plain/ },
>> + 'linked file name'); # bang
>
> I'll try to investigate; I guess this uses wrong name or wrong hash
> for preimage.
Thanks!
>> - Do not test correctness of line number fragments (#l[0-9]+); they're
>> broken too often right now.
>
> What do you mean by broken?
This is only visible with --long-tests -- there are links to line
numbers that don't exist (IOW the fragment doesn't exist in a name or id
attribute on the target page). I've even seen links to #l0. Bug fixes
welcome. ;-)
(There are also some other [mostly minor] issues marked with "TODO:" in
the test code; I didn't want to swamp the list with half a dozen bug
reports though, apart from time constraints on my end.)
> Good work!
Thanks! :-) I'll send v9 in (hopefully) 2-3 days, together with the
first working version of the caching code.
-- Lea
next prev parent reply other threads:[~2008-06-29 23:40 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-06-14 12:47 [RFC/PATCH (WIP)] gitweb: Use Test::WWW::Mechanize::CGI to test gitweb output Jakub Narebski
2008-06-14 14:40 ` Lea Wiemann
2008-06-14 18:07 ` Jakub Narebski
2008-06-14 18:31 ` Lea Wiemann
2008-06-14 18:59 ` Jakub Narebski
2008-06-14 21:12 ` Lea Wiemann
2008-06-15 8:36 ` Jakub Narebski
2008-06-14 18:18 ` Lea Wiemann
2008-06-14 18:31 ` Jakub Narebski
2008-06-14 23:57 ` [RFC/WIP/PATCH v2] gitweb: add test suite with Test::WWW::Mechanize::CGI Lea Wiemann
2008-06-15 18:01 ` Jakub Narebski
2008-06-15 18:45 ` Lea Wiemann
2008-06-16 0:40 ` Jakub Narebski
2008-06-16 9:10 ` Lea Wiemann
2008-06-16 20:15 ` Jakub Narebski
2008-06-20 3:18 ` [WIP/PATCH v3] " Lea Wiemann
2008-06-20 12:08 ` Jakub Narebski
2008-06-20 13:49 ` Lea Wiemann
2008-06-20 18:03 ` Jakub Narebski
2008-06-20 22:04 ` Lea Wiemann
2008-06-20 22:18 ` [WIP/PATCH v4] " Lea Wiemann
2008-06-23 0:45 ` [PATCH v5] " Lea Wiemann
2008-06-23 1:14 ` [PATCH v6] " Lea Wiemann
2008-06-23 2:30 ` Junio C Hamano
2008-06-23 7:00 ` Lea Wiemann
2008-06-23 13:31 ` Jakub Narebski
2008-06-23 17:57 ` Lea Wiemann
2008-06-23 22:18 ` Jakub Narebski
2008-06-24 2:01 ` Lea Wiemann
2008-06-24 2:18 ` [PATCH v7] " Lea Wiemann
2008-06-26 13:47 ` [PATCH] " Lea Wiemann
2008-06-26 13:48 ` [PATCH v8] " Lea Wiemann
2008-06-29 22:47 ` Jakub Narebski
2008-06-29 23:39 ` Lea Wiemann [this message]
2008-06-29 23:56 ` Jakub Narebski
2008-06-30 0:30 ` Lea Wiemann
2008-06-30 21:55 ` Jakub Narebski
[not found] ` <48681EC8.8000606@gmail.com>
2008-06-30 22:01 ` Jakub Narebski
2008-06-24 4:20 ` [PATCH v6] " Junio C Hamano
2008-06-24 8:37 ` Lea Wiemann
2008-06-24 9:23 ` Jakub Narebski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=48681D21.1040302@gmail.com \
--to=lewiemann@gmail.com \
--cc=git@vger.kernel.org \
--cc=jnareb@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).