Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Revert use_ok (#287 and #288) #292

Merged
merged 2 commits into from

2 participants

@schwern
Owner

This removes the lexical effect code from use_ok() while keeping the bug fixes. #287

It also rejiggers the docs to de-emphasize use_ok() and emphasize require_ok(). #288

schwern added some commits
@schwern schwern Undo the lexical use_ok() feature. Sorry FC.
It was causing backwards compat problems, the implementation was complex and
use_ok() is generally considered useless if not actively harmful.

For #287
7e32d0e
@schwern schwern Discourage use_ok() and emphasize require_ok().
Rather than write a whole bunch of docs about why use_ok() is evil and
clutter things up for new developers, require_ok() has been emphesized.

* The main docs have been moved to require_ok()
* require_ok() comes first
* use_ok() is gone from the SYNOPSIS
* Various misuses of use_ok() are mentioned
* The "Module Tests" blurb doesn't give bad advice

For #288
ab98acb
@schwern
Owner

@pdl I'd like your opinion on the doc change in ab98acb. I took some of your changes from #289 but felt adding a lot of extra text to explain why use_ok() shouldn't be used clutters the text, especially for new readers who don't even know what the problem is. What do you think?

@pdl

Much clearer! Putting require_ok first makes a lot more sense, and also solves the issue of use_ok being incorrectly more prominent and better explained despite being less useful.

@schwern schwern merged commit f9f85f7 into master
@schwern
Owner

Thanks for the input, and for taking a stab at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2012
  1. @schwern

    Undo the lexical use_ok() feature. Sorry FC.

    schwern authored
    It was causing backwards compat problems, the implementation was complex and
    use_ok() is generally considered useless if not actively harmful.
    
    For #287
  2. @schwern

    Discourage use_ok() and emphasize require_ok().

    schwern authored
    Rather than write a whole bunch of docs about why use_ok() is evil and
    clutter things up for new developers, require_ok() has been emphesized.
    
    * The main docs have been moved to require_ok()
    * require_ok() comes first
    * use_ok() is gone from the SYNOPSIS
    * Various misuses of use_ok() are mentioned
    * The "Module Tests" blurb doesn't give bad advice
    
    For #288
This page is out of date. Refresh to see the latest.
View
9 Changes
@@ -8,6 +8,15 @@
* use_ok() was calling class->import without quoting which could
cause problems if "class" is also a function.
+ Doc Fixes
+ * use_ok() has been discouraged and de-emphasized as a general
+ replacement for `use` in tests. [github #288]
+
+ Incompatible Changes With Previous Alphas
+ * use_ok() will no longer apply lexical pragams. The incompatibilities
+ and extra complexity is not worth the marginal use.
+ [github #287]
+
0.98_02 Thu Nov 24 01:13:53 PST 2011
Bug Fixes
View
1  MANIFEST
@@ -144,7 +144,6 @@ t/threads.t
t/todo.t
t/undef.t
t/use_ok.t
-t/use_ok_leaking.t
t/useing.t
t/utf8.t
t/versions.t
View
192 lib/Test/More.pm
@@ -49,7 +49,6 @@ Test::More - yet another framework for writing test scripts
# or
use Test::More; # see done_testing()
- BEGIN { use_ok( 'Some::Module' ); }
require_ok( 'Some::Module' );
# Various ways to say "ok"
@@ -782,21 +781,101 @@ sub fail (;$) {
=head2 Module tests
-You usually want to test if the module you're testing loads ok, rather
-than just vomiting if its load fails. For such purposes we have
-C<use_ok> and C<require_ok>.
+Sometimes you want to test if a module, or a list of modules, can
+successfully load. For example, you'll often want a first test which
+simply loads all the modules in the distribution to make sure they
+work before going on to do more complicated testing.
+
+For such purposes we have C<use_ok> and C<require_ok>.
=over 4
+=item B<require_ok>
+
+ require_ok($module);
+ require_ok($file);
+
+Tries to C<require> the given $module or $file. If it loads
+successfully, the test will pass. Otherwise it fails and displays the
+load error.
+
+C<require_ok> will guess whether the input is a module name or a
+filename.
+
+No exception will be thrown if the load fails.
+
+ # require Some::Module
+ require_ok "Some::Module";
+
+ # require "Some/File.pl";
+ require_ok "Some/File.pl";
+
+ # stop testing if any of your modules will not load
+ for my $module (@module) {
+ require_ok $module or BAIL_OUT "Can't load $module";
+ }
+
+=cut
+
+sub require_ok ($) {
+ my($module) = shift;
+ my $tb = Test::More->builder;
+
+ my $pack = caller;
+
+ # Try to determine if we've been given a module name or file.
+ # Module names must be barewords, files not.
+ $module = qq['$module'] unless _is_module_name($module);
+
+ my $code = <<REQUIRE;
+package $pack;
+require $module;
+1;
+REQUIRE
+
+ my( $eval_result, $eval_error ) = _eval($code);
+ my $ok = $tb->ok( $eval_result, "require $module;" );
+
+ unless($ok) {
+ chomp $eval_error;
+ $tb->diag(<<DIAGNOSTIC);
+ Tried to require '$module'.
+ Error: $eval_error
+DIAGNOSTIC
+
+ }
+
+ return $ok;
+}
+
+sub _is_module_name {
+ my $module = shift;
+
+ # Module names start with a letter.
+ # End with an alphanumeric.
+ # The rest is an alphanumeric or ::
+ $module =~ s/\b::\b//g;
+
+ return $module =~ /^[a-zA-Z]\w*$/ ? 1 : 0;
+}
+
+
=item B<use_ok>
BEGIN { use_ok($module); }
BEGIN { use_ok($module, @imports); }
-These simply use the given $module and test to make sure the load
-happened ok. It's recommended that you run use_ok() inside a BEGIN
-block so its functions are exported at compile-time and prototypes are
-properly honored.
+Like C<require_ok>, but it will C<use> the $module in question and
+only loads modules, not files.
+
+If you just want to test a module can be loaded, use C<require_ok>.
+
+If you just want to load a module in a test, we recommend simply using
+C<use> directly. It will cause the test to stop.
+
+It's recommended that you run use_ok() inside a BEGIN block so its
+functions are exported at compile-time and prototypes are properly
+honored.
If @imports are given, they are passed through to the use. So this:
@@ -830,10 +909,6 @@ import anything, use C<require_ok>.
BEGIN { require_ok "Foo" }
-Lexical effects will occur as usual. For example, this will turn on strictures.
-
- BEGIN { use_ok "strict"; }
-
=cut
sub use_ok ($;@) {
@@ -842,40 +917,33 @@ sub use_ok ($;@) {
my $tb = Test::More->builder;
my( $pack, $filename, $line ) = caller;
+ $filename =~ y/\n\r/_/; # so it doesn't run off the "#line $line $f" line
- my $f = $filename;
- $f =~ y/\n\r/_/; # so it doesn't run off the "#line $line $f" line
-
- my $version;
+ my $code;
if( @imports == 1 and $imports[0] =~ /^\d+(?:\.\d+)?$/ ) {
- # probably a version check
- $version = shift @imports;
- }
+ # probably a version check. Perl needs to see the bare number
+ # for it to work with non-Exporter based modules.
+ $code = <<USE;
+package $pack;
- my $version_check = defined $version ? qq{$module->VERSION($version)} : "";
- my $code = <<"USE";
+#line $line $filename
+use $module $imports[0];
+1;
+USE
+ }
+ else {
+ $code = <<USE;
package $pack;
-# Work around [perl #70151]
-\$^H = \${\$args[1]};
-%^H = %{\$args[2]};
-#line $line $f
-require $module; $version_check; '$module'->import(\@{\$args[0]});
-\${\$args[1]} = \$^H;
-%{\$args[2]} = %^H;
+
+#line $line $filename
+use $module \@{\$args[0]};
1;
USE
+ }
- my $hints = $^H;
- my %hints = %^H;
- my( $eval_result, $eval_error )
- = _eval( $code, \(@imports, $hints, %hints) );
+ my( $eval_result, $eval_error ) = _eval( $code, \@imports );
my $ok = $tb->ok( $eval_result, "use $module;" );
- if( $ok ) {
- $^H = $hints;
- %^H = %hints;
- }
-
unless($ok) {
chomp $eval_error;
$@ =~ s{^BEGIN failed--compilation aborted at .*$}
@@ -908,56 +976,6 @@ sub _eval {
return( $eval_result, $eval_error );
}
-=item B<require_ok>
-
- require_ok($module);
- require_ok($file);
-
-Like use_ok(), except it requires the $module or $file.
-
-=cut
-
-sub require_ok ($) {
- my($module) = shift;
- my $tb = Test::More->builder;
-
- my $pack = caller;
-
- # Try to determine if we've been given a module name or file.
- # Module names must be barewords, files not.
- $module = qq['$module'] unless _is_module_name($module);
-
- my $code = <<REQUIRE;
-package $pack;
-require $module;
-1;
-REQUIRE
-
- my( $eval_result, $eval_error ) = _eval($code);
- my $ok = $tb->ok( $eval_result, "require $module;" );
-
- unless($ok) {
- chomp $eval_error;
- $tb->diag(<<DIAGNOSTIC);
- Tried to require '$module'.
- Error: $eval_error
-DIAGNOSTIC
-
- }
-
- return $ok;
-}
-
-sub _is_module_name {
- my $module = shift;
-
- # Module names start with a letter.
- # End with an alphanumeric.
- # The rest is an alphanumeric or ::
- $module =~ s/\b::\b//g;
-
- return $module =~ /^[a-zA-Z]\w*$/ ? 1 : 0;
-}
=back
View
20 t/use_ok.t
@@ -67,26 +67,6 @@ note "Signals are preserved"; {
}
-note "strict works"; {
- no strict;
-
- {
- BEGIN { use_ok 'strict' }
- ok !eval { ()=@{"!#%^"}; 1 }, 'use_ok with pragma';
- }
-
- ok eval { ()=@{"!#%^"}; 1 }, 'pragmata enabled by use_ok are lexical';
-}
-
-
-note "strict works with a version check"; {
- no strict;
-
- BEGIN { use_ok 'strict', 1 }
- ok !eval { ()=@{"!#%^"}; 1 }, 'use_ok with pragma and version';
-}
-
-
note "Line numbers preserved"; {
my $package = "that_cares_about_line_numbers";
View
14 t/use_ok_leaking.t
@@ -1,14 +0,0 @@
-#!/usr/bin/env perl -w
-
-# use_ok was leaking Test::More's loaded pragmas [rt.cpan.org 67538]
-
-no strict; # This is deliberate
-
-use Test::More;
-
-BEGIN { use_ok "Symbol"; }
-
-$str = "hello"; # deliberately don't declare the variable
-is $str, "hello";
-
-done_testing;
Something went wrong with that request. Please try again.