Re: Testing for test labels

[Christopher H. Laco]

Chris Dolan wrote:
> On Sep 27, 2006, at 5:53 PM, Christopher H. Laco wrote:
> 
>> Ha ha. I just realized that I totally screwed myself out of using this
>> policy. I'm using a subclass of Test::More, not Test:More itself.
>>
>> See "Don't 'rm -fr /' when testing" for the topic of that.
>>
>> Would it be possible, or even desirable to flip some sort of config to
>> make this test all t files, or tell this policy that my test class eq
>> 'Test::More' in this instance?
> 
> Good point.  I just added this feature to Perl::Critic svn.  Do like
> this in your perlcriticrc file:
> 
> [TestingAndDebugging::RequireTestLabels]
> modules = My::Test::SubClass  Some::Other::Module


Ok, today I'm bumping Andy. Now you're my new hero. :-)



signature.asc
Description: OpenPGP digital signature

Re: Testing for test labels

[Chris Dolan]

On Sep 27, 2006, at 5:53 PM, Christopher H. Laco wrote:


Ha ha. I just realized that I totally screwed myself out of using this
policy. I'm using a subclass of Test::More, not Test:More itself.

See "Don't 'rm -fr /' when testing" for the topic of that.

Would it be possible, or even desirable to flip some sort of config to
make this test all t files, or tell this policy that my test class eq
'Test::More' in this instance?


Good point.  I just added this feature to Perl::Critic svn.  Do like  
this in your perlcriticrc file:


[TestingAndDebugging::RequireTestLabels]
modules = My::Test::SubClass  Some::Other::Module

Test::More is always implicitly included.  Are there any common  
Test::More subclasses that I should add to the default list?

Chris

--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software  
(http://www.media-landscape.com/) and partners in the revolutionary  
Croquet project (http://www.opencroquet.org/)

Re: Testing for test labels

[A. Pagaltzis]

* Adrian Howard <[EMAIL PROTECTED]> [2006-09-29 14:21]:
> If I had a big pile of tests without descriptions I wouldn't
> want them dying either.

PERL5OPT=-MTest::EnsureDescriptions=:nonfatal

?

Regards,
-- 
Aristotle Pagaltzis // 

Re: Testing for test labels

[Adrian Howard]

On 28 Sep 2006, at 22:37, Jonathan Rockway wrote:
Override Test::Builder::ok to croak if it doesn't get a test name  
argument. Package that up in a module and shim it in with PERL5OPT  
or HARNESS_PERL_SWITCHES.


I kind of dislike this approach.  If my tests are failing, I want them
to fail because my program is broken, not because my tests are broken.


Me to - it's just that I interpret tests throwing exceptions as a  
failure of the test script, rather than a test failure. I try very  
hard no to have test scripts die.



I also don't want missing comments to stop my test from running...
sometimes you need to quickly fix something, run the tests, and *then*
make everything pretty.


Fair enough. Throw a warning instead, or log it.

If I had a big pile of tests without descriptions I wouldn't want  
them dying either.


However, if I had a test suite that had very few tests without  
descriptions then I would want things to die on a regular basis -  
since it should be trivial to clean up, and having it die in a nasty  
manner makes it more likely that I'd keep things clean.


YMMV of course :-)

So I would prefer that "t/style.t" fails due to "No description of  
test

on line 31 of frob.t", rather than t/frob.t failing (or croaking!) for
the same reason.


I'm not following the distinction you're drawing here... probably me  
being dim at this late hour :)



Unfortunately, this is sort of a only-perl-can-parse-Perl issue.  You
don't know that you didn't have a description until you've run the  
test.


The best thing I can think of is to have the harness check for  
comments,

and report an error at the end if some were missing.  Maybe it could
make up a test, so the output looks like:

[snip]
This has the added benefit of restricting this test to the  
developer --
it's unlikely that end-users of your module care about the TAP  
comments.

 I think this would be pretty easy to implement, too.


Yup. Log the errors by overriding Test::Builder::ok and have  
tap_comments check the log file.


Adrian

Re: Testing for test labels

[Jonathan Rockway]

> Override Test::Builder::ok to croak if it doesn't get a test name argument. 
> Package that up in a module and shim it in with PERL5OPT or 
> HARNESS_PERL_SWITCHES.

I kind of dislike this approach.  If my tests are failing, I want them
to fail because my program is broken, not because my tests are broken.
I also don't want missing comments to stop my test from running...
sometimes you need to quickly fix something, run the tests, and *then*
make everything pretty.

So I would prefer that "t/style.t" fails due to "No description of test
on line 31 of frob.t", rather than t/frob.t failing (or croaking!) for
the same reason.

Unfortunately, this is sort of a only-perl-can-parse-Perl issue.  You
don't know that you didn't have a description until you've run the test.

The best thing I can think of is to have the harness check for comments,
and report an error at the end if some were missing.  Maybe it could
make up a test, so the output looks like:

-
t/real.t.ok
t/foo.t..ok
t/tap_comments...23
# FAIL: t/foo.t test #42 needs a comment
...NOT OK

Failed 1/96 tests, 98.96% okay
Failed Test Stat Wstat Total Fail  List of Failed
---
tap_comments  481  23
Failed 1/1 test scripts. 1/96 subtests failed.
Files=2, Tests=96,  4 wallclock secs ( 0.11 cusr +  0.06 csys =  0.17 CPU)
Failed 1/3 test programs. 1/96 subtests failed.
-

This has the added benefit of restricting this test to the developer --
it's unlikely that end-users of your module care about the TAP comments.
 I think this would be pretty easy to implement, too.

Regards,
Jonathan Rockway

-- 
package JAPH;use Catalyst qw/-Debug/;($;=JAPH)->config(name => do {
$,.=reverse qw[Jonathan tsu rehton lre rekca Rockway][$_].[split //,
";$;"]->[$_].q; ;for 1..4;$,=~s;^.;;;$,});$;->setup;

Re: Testing for test labels

[Adrian Howard]

On 27 Sep 2006, at 23:53, Christopher H. Laco wrote:
[snip]

Would it be possible, or even desirable to flip some sort of config to
make this test all t files, or tell this policy that my test class eq
'Test::More' in this instance?

[snip]

Not that this wouldn't be a nice idea, but as another approach that  
I've used in the past...


Override Test::Builder::ok to croak if it doesn't get a test name  
argument. Package that up in a module and shim it in with PERL5OPT or  
HARNESS_PERL_SWITCHES.


Just a thought..

Adrian

Re: Testing for test labels

[Christopher H. Laco]

Chris Dolan wrote:
> On Sep 24, 2006, at 12:41 PM, Christopher H. Laco wrote:
> 
>> I've got a crap loads of tests in Handel at the moment; 1+
>> checkpoints, and enough files that I hit the dreaded "command line too
>> long" under win32 [now fixed]. That's not saying all the tests are
>> glorious and not repetitive. :-)
>>
>> I've still got a ways to go before I'm happy without the dist enough to
>> declare the 1.0 version done. Among things on my list are cleaning up
>> the current test code. A big offender is that not all of my tests have
>> labels.
>>
>> I could've swore that sometime in the last month I saw some chatter
>> about a Test::TestDescriptions module, or a ::TestsHaveDescriptions type
>> thing for Perl::Critic. Assuming I'm not just making that up, has anyone
>> seen mention of such a thing?
>>
>> If no such a beast exists thus far, what would be the easiest way to get
>> such functionality? I was thinking that I could just run a subclass of
>> Test::More in my tests that dies whenever no label argument is supplied.
>> That's not a terrible thing since I already have a Handel::Test module
>> that deals with SQLite setup and teardown in most tests.
> 
> Done.  I created Perl::Critic::Policy::Testing::RequireTestLabels and
> added it to the Perl::Critic SVN at
>   http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic
> (username: "guest", password: "")
> 
> It was pretty simple to write.  Below are the important bits of the
> code.  I encourage others to write Test::More-specific policies for
> Perl::Critic!
> 
> Chris

Ha ha. I just realized that I totally screwed myself out of using this
policy. I'm using a subclass of Test::More, not Test:More itself.

See "Don't 'rm -fr /' when testing" for the topic of that.

Would it be possible, or even desirable to flip some sort of config to
make this test all t files, or tell this policy that my test class eq
'Test::More' in this instance?

-=Chris



signature.asc
Description: OpenPGP digital signature

Re: Testing for test labels

[Chris Dolan]

On Sep 25, 2006, at 8:16 PM, Christopher H. Laco wrote:


Now, for part 2. :-)
What's the sanest way to test t/*.t files without also testing
everything in t/lib ?

If I'm already using all_critic_ok(), I can't just call critic_ok() on
all the t/*.t files because a plan has already been set.

Would it be possible to have all_critic_ok also take globs:

all_critic_ok('blib', 't/*.t')

-=Chris


Here's what I would do:


use Test::More;
use Test::Perl::Critic qw(critic_ok);
use File::Slurp qw(read_dir);
use Perl::Critic::Utils qw(all_perl_files);

my @files = all_perl_files('blib'), grep {m/\.t\z/} read_dir('t');
plan tests => scalar @files;
for my $file (@files) {
   critic_ok( $file, $file );
}


Of course, you could easily replace that use of File::Slurp with  
opendir,readdir,closedir.


Chris

--
Chris Dolan, Software Developer, http://www.chrisdolan.net/
Public key: http://www.chrisdolan.net/public.key
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Re: Testing for test labels

[Christopher H. Laco]

Chris Dolan wrote:
> On Sep 24, 2006, at 5:42 PM, Christopher H. Laco wrote:
> 
>>
>> Ok, I'll play your game. :-)
>>
>> http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/lib/Perl/Critic/Policy/Testing/
>>
>>
>> [Assuming I'm not silly] Empty! Rev. 667
>>
>> -=Chris
> 
> D'oh!  SVN commits work better if you "svn add" first...  Fixed, should
> work now. :-)
> 
> Chris
> 
> -- 
> Chris Dolan, Software Developer, http://www.chrisdolan.net/
> Public key: http://www.chrisdolan.net/public.key
> vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Now, for part 2. :-)
What's the sanest way to test t/*.t files without also testing
everything in t/lib ?

If I'm already using all_critic_ok(), I can't just call critic_ok() on
all the t/*.t files because a plan has already been set.

Would it be possible to have all_critic_ok also take globs:

all_critic_ok('blib', 't/*.t')

-=Chris



signature.asc
Description: OpenPGP digital signature

Re: Testing for test labels

[Christopher H. Laco]

Chris Dolan wrote:
> On Sep 24, 2006, at 5:42 PM, Christopher H. Laco wrote:
> 
>>
>> Ok, I'll play your game. :-)
>>
>> http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/lib/Perl/Critic/Policy/Testing/
>>
>>
>> [Assuming I'm not silly] Empty! Rev. 667
>>
>> -=Chris
> 
> D'oh!  SVN commits work better if you "svn add" first...  Fixed, should
> work now. :-)
> 
> Chris
> 
> -- 
> Chris Dolan, Software Developer, http://www.chrisdolan.net/
> Public key: http://www.chrisdolan.net/public.key
> vCard: http://www.chrisdolan.net/ChrisDolan.vcf
> 


Been there. Done That.

I wonder if there's a config option anywhere to have svn commit bark
loudly if there are non-added files.



signature.asc
Description: OpenPGP digital signature

Re: Testing for test labels

[Chris Dolan]

On Sep 24, 2006, at 5:42 PM, Christopher H. Laco wrote:



Ok, I'll play your game. :-)

http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/lib/ 
Perl/Critic/Policy/Testing/


[Assuming I'm not silly] Empty! Rev. 667

-=Chris


D'oh!  SVN commits work better if you "svn add" first...  Fixed,  
should work now. :-)


Chris

--
Chris Dolan, Software Developer, http://www.chrisdolan.net/
Public key: http://www.chrisdolan.net/public.key
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Re: Testing for test labels

[Christopher H. Laco]

Chris Dolan wrote:
[snip]
> Done.  I created Perl::Critic::Policy::Testing::RequireTestLabels and
> added it to the Perl::Critic SVN at
>   http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic
> (username: "guest", password: "")
> 
> It was pretty simple to write.  Below are the important bits of the
> code.  I encourage others to write Test::More-specific policies for
> Perl::Critic!
> 
> Chris
[snip]

Ok, I'll play your game. :-)

http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic/lib/Perl/Critic/Policy/Testing/

[Assuming I'm not silly] Empty! Rev. 667

-=Chris



signature.asc
Description: OpenPGP digital signature

Re: Testing for test labels

[Chris Dolan]

On Sep 24, 2006, at 12:41 PM, Christopher H. Laco wrote:


I've got a crap loads of tests in Handel at the moment; 1+
checkpoints, and enough files that I hit the dreaded "command line too
long" under win32 [now fixed]. That's not saying all the tests are
glorious and not repetitive. :-)

I've still got a ways to go before I'm happy without the dist  
enough to

declare the 1.0 version done. Among things on my list are cleaning up
the current test code. A big offender is that not all of my tests have
labels.

I could've swore that sometime in the last month I saw some chatter
about a Test::TestDescriptions module, or a ::TestsHaveDescriptions  
type
thing for Perl::Critic. Assuming I'm not just making that up, has  
anyone

seen mention of such a thing?

If no such a beast exists thus far, what would be the easiest way  
to get

such functionality? I was thinking that I could just run a subclass of
Test::More in my tests that dies whenever no label argument is  
supplied.

That's not a terrible thing since I already have a Handel::Test module
that deals with SQLite setup and teardown in most tests.


Done.  I created Perl::Critic::Policy::Testing::RequireTestLabels and  
added it to the Perl::Critic SVN at

  http://perlcritic.tigris.org/svn/perlcritic/trunk/Perl-Critic
(username: "guest", password: "")

It was pretty simple to write.  Below are the important bits of the  
code.  I encourage others to write Test::More-specific policies for  
Perl::Critic!


Chris

==

my %label_arg_pos = (
   ok => 1,
   is => 2,
   isnt => 2,
   like => 2,
   unlike => 2,
   cmp_ok => 3,
   is_deeply => 2,
   pass => 0,
   fail => 0,
);

sub default_severity { return $SEVERITY_MEDIUM }
sub applies_to { return 'PPI::Token::Word' }

sub violates {
my ($self, $elem, $doc) = @_;

my $arg_index = $label_arg_pos{$elem};
return if !defined $arg_index; # this is the fastest  
conditional, so do it first

return if !is_function_call($elem);
return if !_has_test_more($doc);

# Does the function call have enough arguments?
my @args = parse_arg_list($elem);
return if ( @args > $arg_index );

return $self->violation( $desc, $expl, $elem );
}

sub _has_test_more {
my ( $doc ) = @_;

my $includes = $doc->find('PPI::Statement::Include');
return if !$includes;
return any { $_->module() eq 'Test::More' } @{ $includes };
}

==

--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf

Clotho Advanced Media, Inc. - Creators of MediaLandscape Software  
(http://www.media-landscape.com/) and partners in the revolutionary  
Croquet project (http://www.opencroquet.org/)

Testing for test labels

[Christopher H. Laco]

I've got a crap loads of tests in Handel at the moment; 1+
checkpoints, and enough files that I hit the dreaded "command line too
long" under win32 [now fixed]. That's not saying all the tests are
glorious and not repetitive. :-)

I've still got a ways to go before I'm happy without the dist enough to
declare the 1.0 version done. Among things on my list are cleaning up
the current test code. A big offender is that not all of my tests have
labels.

I could've swore that sometime in the last month I saw some chatter
about a Test::TestDescriptions module, or a ::TestsHaveDescriptions type
thing for Perl::Critic. Assuming I'm not just making that up, has anyone
seen mention of such a thing?

If no such a beast exists thus far, what would be the easiest way to get
such functionality? I was thinking that I could just run a subclass of
Test::More in my tests that dies whenever no label argument is supplied.
That's not a terrible thing since I already have a Handel::Test module
that deals with SQLite setup and teardown in most tests.

Thoughts?

-=Chris



signature.asc
Description: OpenPGP digital signature

Re: Test labels

[Mark Stosberg]

> On Mon, Dec 06, 2004 at 10:28:45PM -0600, Andy Lester wrote:
> I think even better than 
> 
>   ok( $expr, "name" );
> 
> or
> 
>   ok( $expr, "comment" );
> 
> is
> 
>   ok( $expr, "label" );
> 
> RJBS points out that "comment" implies "not really worth doing", and I
> still don't like "name" because it implies (to me) a unique identifier.
> We also talked about "description", but "description" is just s
> overloaded.

I prefer "name" or "label" to "comment". 

Name does not imply 'unique' for me, just like 'John Smith' 
is not expected to a unique name of a person. 

Mark

-- 
http://mark.stosberg.com/ 

Re: Test labels

[Stevan Little]

I don't really agree that "comment" implies "not really worth doing". I 
think
those inclined to not put a name/comment/label on their test will not 
do it
whatever we end up calling it. IMO the only way to get people to do it 
would
be to make it required, and I don't think that is a good idea.

I like "label", but to me that also implies a (somewhat-less-than-name) 
unique identifier.

I also like H. Merjin's suggestions of "tag", however it might be 
confusing to the more
web-inclinded. (Tag as in shirt-tag, not as in HTML-tag)

Personally I use them like comments, so that makes the most sense to me.
Steve
On Dec 6, 2004, at 11:28 PM, Andy Lester wrote:
I think even better than
  ok( $expr, "name" );
or
  ok( $expr, "comment" );
is
  ok( $expr, "label" );
RJBS points out that "comment" implies "not really worth doing",
and I still don't like "name" because it implies (to me) a unique 
identifier.
We also talked about "description", but "description" is just s
overloaded.

Thoughts?
xoxo,
Andy
--
Andy Lester => [EMAIL PROTECTED] => www.petdance.com => AIM:petdance

Re: Test labels

[Steve Peters]

On Mon, Dec 06, 2004 at 10:28:45PM -0600, Andy Lester wrote:
> I think even better than 
> 
>   ok( $expr, "name" );
> 
> or
> 
>   ok( $expr, "comment" );
> 
> is
> 
>   ok( $expr, "label" );
> 
> RJBS points out that "comment" implies "not really worth doing", and I
> still don't like "name" because it implies (to me) a unique identifier.
> We also talked about "description", but "description" is just s
> overloaded.
> 
> Thoughts?

Label sounds fine to me

Steve Peters
[EMAIL PROTECTED]

Re: Test labels

[H.Merijn Brand]

On Tue 07 Dec 2004 05:28, Andy Lester <[EMAIL PROTECTED]> wrote:
> I think even better than 
> 
>   ok( $expr, "name" );
> 
> or
> 
>   ok( $expr, "comment" );
> 
> is
> 
>   ok( $expr, "label" );

or ok ($expr, "indicator");
or ok ($expr, "tag");

for me the name/comment/label/tag/indicator/... is just a tag (which does not
have to be unique) to be able to find back my test in the test script. As long
as all tests run fine, you don't read them, so "tag" sounds best to me

> RJBS points out that "comment" implies "not really worth doing", and I
> still don't like "name" because it implies (to me) a unique identifier.
> We also talked about "description", but "description" is just s
> overloaded.

-- 
H.Merijn BrandAmsterdam Perl Mongers (http://amsterdam.pm.org/)
using perl-5.6.1, 5.8.5, & 5.9.x, and 809 on  HP-UX 10.20 & 11.00, 11i,
   AIX 4.3, AIX 5.2, SuSE 9.1, and Win2k.  http://www.cmve.net/~merijn/
http:[EMAIL PROTECTED]/   [EMAIL PROTECTED]
send smoke reports to: [EMAIL PROTECTED], QA: http://qa.perl.org

Test labels

[Andy Lester]

I think even better than 

  ok( $expr, "name" );

or

  ok( $expr, "comment" );

is

  ok( $expr, "label" );

RJBS points out that "comment" implies "not really worth doing", and I
still don't like "name" because it implies (to me) a unique identifier.
We also talked about "description", but "description" is just s
overloaded.

Thoughts?

xoxo,
Andy

-- 
Andy Lester => [EMAIL PROTECTED] => www.petdance.com => AIM:petdance