On Wed, 1 Oct 2003, Geoffrey Young wrote:
or, I could just commit it now and Randy can decide which route to go. I think I'll just do that...
Here's a revised set of tests, using Geoff's implementation of Apache::CRLF. This also addresses a couple of earlier comments of Stas - the files used for comparison are now assumed to be found as t/htdocs/perlio/http.pod and t/htdocs/perlio/http_cycle.png, and also a constant data file name is used (and then cleaned up after the tests are done).
Cool. The following are just style comments.
+ # test reading and writing text and binary files
+ {
+ local $/;
Randy, can you please put local $/; just before it's needed? Otherwise it's too far away from its usage and it may affect other things if the test is expanded in the future. The best practice IMHO is:
{
local $/;
... <$fh>
}in every place you need it. this makes the code easier to read.
+ my ($rfh, $wfh, $pfh);
why these are defined outside and not where they are opened?
open my $rfh, "<:APR", $in, $r->pool
etc.
+ for my $file ('http.pod', 'http_cycle.png') {
+ my $in = catfile $dir, $file;
+ my $out = catfile $dir, "$file.out";
+ open $rfh, "<:APR", $in, $r->pool
+ or die "Cannot open $in for reading: $!";
+ binmode($rfh); # not necessary
if it's not necessary, why do we need it?
+ my $apr_content = <$rfh>; + close $rfh; + open $pfh, "<", $in + or die "Cannot open $in for reading: $!"; + binmode($pfh); + my $perl_content = <$pfh>; + close $pfh; + ok t_cmp(length $perl_content, + length $apr_content, + "testing data size of $file"); + + open $wfh, ">:APR", $out, $r->pool + or die "Cannot open $out for writing: $!"; + print $wfh $apr_content; + close $wfh; + ok t_cmp(-s $in, + -s $out, + "testing file size of $file"); + unlink $out; + }
I'd enclose the following code of its own block showing that it's an independent sub-test. you won't need to predeclare lexicals vars above. This practice makes easier to debug subtests where you can comment out blocks without affecting the rest of the test. see apr/pool.pm for such an example.
+ my $scratch = catfile $dir, 'scratch.dat'; + my $text; + my $count = 2000; + open $wfh, ">:crlf", $scratch + or die "Cannot open $scratch for writing: $!"; + print $wfh 'a' . ((('a' x 14) . "\n") x $count); + close $wfh; + open $rfh, "<:APR", $scratch, $r->pool + or die "Cannot open $scratch for reading: $!"; + $text = <$rfh>; + close $rfh; + ok t_cmp($count, + count_chars($text, Apache::CRLF), + 'testing for presence of \015\012'); + ok t_cmp($count, + count_chars($text, "\n"), + 'testing for presence of \n'); + + open $wfh, ">:APR", $scratch, $r->pool + or die "Cannot open $scratch for writing: $!"; + binmode($wfh); # not necessary + print $wfh 'a' . ((('a' x 14) . Apache::CRLF) x $count); + close $wfh; + open $rfh, "<:APR", $scratch, $r->pool + or die "Cannot open $scratch for reading: $!"; + $text = <$rfh>; + close $rfh; + ok t_cmp($count, + count_chars($text, Apache::CRLF), + 'testing for presence of \015\012'); + ok t_cmp($count, + count_chars($text, "\n"), + 'testing for presence of \n'); + open $rfh, "<:crlf", $scratch + or die "Cannot open $scratch for reading: $!"; + $text = <$rfh>; + close $rfh; + ok t_cmp(0, + count_chars($text, Apache::CRLF), + 'testing for presence of \015\012'); + ok t_cmp($count, + count_chars($text, "\n"), + 'testing for presence of \n'); + + my $utf8 = "\x{042F} \x{0432}\x{0430}\x{0441} \x{043B}\x{044E}"; + open $wfh, ">:APR", $scratch, $r->pool + or die "Cannot open $scratch for writing: $!"; + binmode($wfh, ':utf8'); + print $wfh $utf8; + close $wfh; + open $rfh, "<:APR", $scratch, $r->pool + or die "Cannot open $scratch for reading: $!"; + binmode($rfh, ':utf8'); + $text = <$rfh>; + close $rfh; + ok t_cmp($utf8, + $text, + 'utf8 binmode test'); + unlink $scratch; + }
# XXX: need tests # - for stdin/out/err as they are handled specially @@ -232,6 +323,13 @@ # cleanup: t_mkdir will remove the whole tree including the file
Apache::OK; +} + +sub count_chars { + my($text, $chars) = @_; + my $seen = 0; + $seen++ while $text =~ /$chars/g; + return $seen; }
1;
===============================================================
--
__________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
