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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
