Bug #4467 for WWW-Mechanize: Fwd: [WWW::Mechanize test-patch and Qs: jcromie@divsol.com]

-- Andy Lester => andy@petdance.com => www.petdance.com => AIM:petdance

Return-Path: <jcromie@divsol.com> Delivered-To: andy-andy@petdance.com Received: (qmail 24576 invoked from network); 24 Nov 2003 05:39:10 -0000 Received: from mail01.powweb.com (63.251.216.34) by chimpy.petdance.com with SMTP; 24 Nov 2003 05:39:10 -0000 Received: from divsol.com (65-100-181-124.dnvr.qwest.net [65.100.181.124]) by mail01.powweb.com (Postfix) with ESMTP id 85A6E367A7 for <andy@petdance.com>; Sun, 23 Nov 2003 21:39:19 -0800 (PST) Message-ID: <3FC19986.5020000@divsol.com> Date: Sun, 23 Nov 2003 22:39:18 -0700 From: Jim Cromie <jcromie@divsol.com> User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 X-Accept-Language: en-us, en MIME-Version: 1.0 To: andy@petdance.com Subject: WWW::Mechanize test-patch and Qs Content-Type: multipart/mixed; boundary="------------090304060709040906040406" X-Spam-Checker-Version: SpamAssassin 2.60-rc3 (1.202-2003-08-29-exp) on chimpy.petdance.com X-Spam-Level: X-Spam-Status: No, hits=0.7 required=5.0 tests=HTML_MESSAGE,HTML_TAG_BALANCE_BODY, HTML_TAG_BALANCE_HTML autolearn=no version=2.60-rc3 This is a multi-part message in MIME format. --------------090304060709040906040406 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit hi Andy, nice module. Ive added a few tests - 1st bunch tests for find_link *_regex usage errors. theyre in a skip-block in-case no Test::Warn, Unfortunately, this doesnt quite work with a perl-version that doesnt have Test-Warn installed. I dont have sufficient Test-More fu to know why. Can't locate Test/Warn.pm in @INC (@INC contains: lib /usr/local/lib/perl5/5.6.2/i686-linux /usr/local/lib/perl5/5.6.2 /usr/local/lib/perl5/site_perl/5.6.2/i686-linux /usr/local/lib/perl5/site_perl/5.6.2 /usr/local/lib/perl5/site_perl/5.6.1/i686-linux /usr/local/lib/perl5/site_perl/5.6.1 /usr/local/lib/perl5/site_perl .) at t/find_link.t line 134. # Looks like you planned 59 tests but only ran 55. # Looks like your test died just after 55. Also - I note that the converse is not a usage error. I added a corresponding (inner) TODO block. Q1: A tag_regex => "foo" could reasonably build a regex from the string, though that isnt as efficient as forcing user to do the right thing. Otherwize, tag => qr/a|frame/, should error the same way. Q2. You could possibly have it both ways, ex: use WWW::Mechanize qw ( :laxfind ); though Im not sure an import-time option can be easily optimized, but I suppose moving all of the function into an eval "block" would cover it. Q3. I added tests for padded text & text_regex, but they fail cuz $mech->{links}[1] is trimmed. Ive patched pod to reflect that in "Content-handling methods $a->find_link()". I think the whole find_link() pod belongs up near "Link-following methods", but thats too editorial for me to be doing to your writing. Q4. I made a sub-optimal addition to META.yml, theres no build_recommends. So the attached test-patch is incomplete; I wanted to run it by you to see what you prefer. Ill work up a patch per your feedback. this patch was against 0.66, but I just pulled cvs, and it looks like it would apply wo rejects. thx, jimc --------------090304060709040906040406 Content-Type: text/plain; name="WM-patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="WM-patch" diff -ru WWW-Mechanize-0.66/META.yml WWW-Mechanize-0.66-mod/META.yml --- WWW-Mechanize-0.66/META.yml Thu Nov 13 13:58:32 2003 +++ WWW-Mechanize-0.66-mod/META.yml Sun Nov 23 15:50:03 2003 @@ -21,5 +21,11 @@ URI::file: 0 URI::URL: 0 +# one of these 2 is better +recommends: + Test::Warn 0 +build_requires: + Test::Warn 0 + distribution_type: module generated_by: ExtUtils::MakeMaker version 6.21 Only in WWW-Mechanize-0.66-mod/: META.yml~ Only in WWW-Mechanize-0.66-mod/: Makefile Only in WWW-Mechanize-0.66-mod/: blib Only in WWW-Mechanize-0.66-mod/: blibdirs diff -ru WWW-Mechanize-0.66/lib/WWW/Mechanize.pm WWW-Mechanize-0.66-mod/lib/WWW/Mechanize.pm --- WWW-Mechanize-0.66/lib/WWW/Mechanize.pm Thu Nov 13 13:44:59 2003 +++ WWW-Mechanize-0.66-mod/lib/WWW/Mechanize.pm Sun Nov 23 22:11:57 2003 @@ -878,6 +878,12 @@ $a->find_link( text_regex => qr/download/i ); +Note that the text extracted from the page's links are trimmed, for +example C<< <a> foo </a> >> is stored as 'foo', therefore searching +for leading and trailing spaces will fail. In other words, C<< +find_link(text => ' foo ') >> will not work. + + =item * C<< url => string >> and C<< url_regex => regex >> Matches the URL of the link against I<string> or I<regex>, as appropriate. Only in WWW-Mechanize-0.66-mod/lib/WWW: Mechanize.pm~ Only in WWW-Mechanize-0.66-mod/: pm_to_blib Only in WWW-Mechanize-0.66-mod/t: SKIP-MECH-DUMP diff -ru WWW-Mechanize-0.66/t/find_link.html WWW-Mechanize-0.66-mod/t/find_link.html --- WWW-Mechanize-0.66/t/find_link.html Fri Oct 24 16:09:06 2003 +++ WWW-Mechanize-0.66-mod/t/find_link.html Sun Nov 23 19:40:38 2003 @@ -26,6 +26,10 @@ <AREA HREF="http://www.cpan.org/area" COORDS="10,11,12,13" /> </MAP> <IMG SRC="SOME_IMAGE" USEMAP="#SOME_MAP"> + + <!-- new stuff --> + <A HREF="http://nowhere.org/" Name="Here">NoWhere</A> + <A HREF="http://nowhere.org/padded" Name=" Here "> NoWhere </A> </body> </html> Only in WWW-Mechanize-0.66-mod/t: find_link.html~ diff -ru WWW-Mechanize-0.66/t/find_link.t WWW-Mechanize-0.66-mod/t/find_link.t --- WWW-Mechanize-0.66/t/find_link.t Fri Oct 24 16:09:07 2003 +++ WWW-Mechanize-0.66-mod/t/find_link.t Sun Nov 23 21:48:46 2003 @@ -2,7 +2,7 @@ use warnings; use strict; -use Test::More tests => 55; +use Test::More tests => 59; use URI::file; BEGIN { @@ -129,3 +129,109 @@ $x = $t->find_link( text => "Rebuild Index" ); isa_ok( $x, 'WWW::Mechanize::Link' ); is_deeply( $x, [ "/cgi-bin/MT/mt.cgi", "Rebuild Index", undef, "a" ], 'Got the JavaScript link' ); + + +# NEW TESTS + +SKIP: { + # tests for usage errors + # OTOH - *_regex could be construed as instruction to build a regex. + + skip "no Test::Warn" unless require Test::Warn; + Test::Warn->import qw(warning_like); + + diag "test for CAPITALIZED (not allowed) selectors"; + + for my $tname (qw( TEXT NAME URL TAG )) { + warning_like ( sub { $t->find_link( $tname => "expect error" ) }, + qr(Unknown link-finding parameter), + "detected usage error: $tname => 'string'" ); + } + + diag "test for *_regex => string errors"; + + for my $tn (qw( text name url tag )) { + my $tname = $tn.'_regex'; + warning_like ( sub { $t->find_link( $tname => "expect error" ) }, + qr/passed as $tname is not a regex/, + "detected usage error: $tname => 'string'" ); + } +}; + +TODO: { + local $TODO = "doesnt work, but doesnt warn either"; + diag $TODO; + + $DB::single = 1; + + $x = $t->find_link( text => qr/NoWhere/ ); + isa_ok( $x, 'WWW::Mechanize::Link' ) + ? is( $x->url, "http://nowhere.org/", "found un-padded text with regex" ) + : fail("text => qr// fails, without warning"); + + $x = $t->find_link( url => qr/nowhere.org/ ); + isa_ok( $x, 'WWW::Mechanize::Link' ) + ? is( $x->url, "http://nowhere.org/", "found url with regex" ) + : fail("url => qr// fails, without warning"); + + $x = $t->find_link( name => qr/Here/ ); + isa_ok( $x, 'WWW::Mechanize::Link' ) + ? is( $x->url, "http://nowhere.org/", "found name with regex" ) + : fail("name => qr// fails, without warning"); + + $x = $t->find_link( name => qr/ Here / ); + isa_ok( $x, 'WWW::Mechanize::Link' ); + is( $x->url, "http://nowhere.org/", "found name with padded regex" ); + + $x = $t->find_link( tag => qr/NoWhere/ ); + isa_ok( $x, 'WWW::Mechanize::Link' ); + +} + +if (0) { + + diag "doesnt work, but doesnt warn either"; + $x = $t->find_link( text => qr/NoWhere/ ); + + local $TODO = "doesnt work, but doesnt warn either"; + isa_ok( $x, 'WWW::Mechanize::Link' ); + is( $x->url, "http://nowhere.org/", "found un-padded text" ); + + local $TODO = 'test for *_regex => string errors'; + + warning_like ( sub { $t->find_link( text => qr/NoWhere/ ) }, + qr(passed as text_regex is not a regex), + "detected text => regex usage error" ); + + warning_like ( sub { $t->find_link( url => qr/NoWhere/ ) }, + qr(passed as url_regex is not a regex), + "detected url => regex usage error" ); + + warning_like ( sub { $t->find_link( name => qr/NoWhere/ ) }, + qr(passed as name_regex is not a regex), + "detected name => regex usage error" ); + + warning_like ( sub { $t->find_link( tag => qr/NoWhere/ ) }, + qr(passed as tag_regex is not a regex), + "detected tag => regex usage error" ); +} + + +__END__ + +$x = $t->find_link( text => /NoWhere/ ); +isa_ok( $x, 'WWW::Mechanize::Link' ); +is( $x->url, "http://nowhere.org/", "found un-padded text" ); + + +# NEW TESTS of padded text + + +TODO: { + local $TODO = "handle padded text (maybe)"; + + $x = $t->find_link( text => " NoWhere " ); + isa_ok( $x, 'WWW::Mechanize::Link' ); + #is( $x->url, "http://nowhere.org/padded", "found padded text" ); +} + Only in WWW-Mechanize-0.66-mod/t: find_link.t~ --------------090304060709040906040406--