module-build-general — General discussion about Module::Build project

[Fwd: Re: Different types of tests (was Re: [Module::Build] [RFC] author tests)]

[David G. <da...@hy...>]

(Bringing my reply to Rob back to the list as well.)

-------- Original Message --------
Subject: Re: Different types of tests (was Re: [Module::Build] [RFC] 
author tests)
Date: Fri, 03 Feb 2006 13:39:26 -0500
From: David Golden <da...@hy...>
To: Rob Kinyon <rob...@gm...>
References: <703...@ma...> 
  <43E...@hy...> 
<703...@ma...>

It doesn't look like you copied this to the list.

A few comments in reply:

Rob Kinyon wrote:
> I don't think they're non-functional tests. They're useful tests and

I was using "functional" along the lines of "functional" and
"non-functional" requirements.  I.e. "does what it says it does".  In
that sense, making sure the Pod is correct is non-functional.  It's
confusing jargon, I'll admit.

> You're missing the point. What if I, as the maintainer of my distro,
> have a series of tests that require special setup? What about
> benchmarking tests? What about tests for a reason that only Damian can
> think of?

We may be missing each others' points.  I'm suggesting solving a
narrower problem, intentionally.

> Plus, if you decide that a given test file belongs in a different
> testing scheme, you change the Build.PL, not the test file. This means
> that the test has more integrity  because it's been touched less.

Are you really suggesting that here that a benchmarking test is going to
become a production test?

> This is a straw man and there's a very easy solution - require the
> version of MB that provides this feature. This is no different than
> the Test::Simple upgrade from 0.46 to 0.47 which had the major API
> change. Ever wonder why every Build.PL in existence has:
> 
>     build_requires => {
>         'Test::More' => '0.47',
>     },

Which as a side note is broken for many version of CPANPLUS because
CPANPLUS doesn't (didn't?) honor build_requires.  That's partly my point
about the interdependencies between the tool chain.  As another example,
how does "prove" know which tests are in which of your groups?  The
environment variable approach leaves it to tests files to know if they
should fire rather than the tools running the test files.  (There's an
interesting functional vs OO paradigm question in this, I think.)

> I do have another option - I write and maintain Module::Build::Rob
> that provides this feature. Except, I don't want to do that because:
>     a) I have enough on my plate
>     b) The name sucks and I can't think of a better one
>     c) I don't want to.

I guess the proof will be in the code.  I think it's easier to write and
maintain the environment variable approach.  I think what you're talking
about sounds great, too, but I'm not sold that the need for it justifies
the effort to create it and test it and maintain it.  If you do --
great.  If not, that also suggests a simpler approach might be better.

Regards,
David

Fwd: Different types of tests (was Re: [Module::Build] [RFC] author tests)

[Rob K. <rob...@gm...>]

I forgot to cc the list. :-|

Rob

---------- Forwarded message ----------
From: Rob Kinyon <rob...@gm...>
Date: Feb 3, 2006 12:57 PM
Subject: Re: Different types of tests (was Re: [Module::Build] [RFC]
author tests)
To: David Golden <da...@hy...>


On 2/3/06, David Golden <da...@hy...> wrote:
> I think this will mean massive overbuilding of a framework without a
> clear need for that type of functionality.

I don't think so - I'll explain further down.

> If we remember where this all started -- it's the desire not to inflict
> "pod tests" in *.t files on end users.  Thanks to CPANTS and Kwalitee --
> and not helped by Module::Starter::PBP -- there's a ton of cargo-cult
> non-functional tests winding up in distributions.

I don't think they're non-functional tests. They're useful tests and
I, as a user, like seeing that POD::Coverage passes. They're better
described as documentation tests. Whether the maintainer feels that
they should be user acceptance tests is up to the maintainer.

> What's missing in all that cargo-culting is a widely accepted mechanism
> for opting in/out of the tests.  Building something into the toolchain
> -- Module::Build and ExtUtils::MakeMaker -- (and then getting that into
> the Test::Pod docs, maybe CPAN/CPANPLUS, etc.) is just a way of driving
> a standard.
>
> I'd prefer to KISS -- look for the simplest solution that could possibly
> work.  So just because M::B, etc. is a conveient place to push the
> standard doesn't mean that the standard should be (a) overworked or (b)
> too tightly coupled to M::B specifically.

It's not. This isn't a standard - it's a way for users of MB to
differentiate between the various kinds of tests in a way that's
maintainer-specific without any extra coding. If other packaging
systems wish to add this feature, they're more than welcome to do so.

> I prefer the environment variable approach because, while it's global --
> it's one of easiest methods of interprocess communication between a
> build framework, the test harness and test files.

Environment variables aren't supported on all systems in the same way,
plus a number of users have issues with them. Personally, I feel the
usage of environment variables needs to die a very quick death,
replaced with parameters passed upon invocation.

> What I'd *really* like to see is the community agree on an environment
> variable that signals that "author" testing (for lack of a better term)
> is desired.  And then I'd like to see Test::Pod, Test::Pod::Coverage,
> etc. look for that environment variable and skip running their tests
> unless it exists.  In other words -- I'd like the skip logic be a
> convention followed by those who write tests rather than something
> hacked up for all the different ways people might write/structure/run tes=
ts.

You're missing the point. What if I, as the maintainer of my distro,
have a series of tests that require special setup? What about
benchmarking tests? What about tests for a reason that only Damian can
think of?

In other words, the various ways of slicing up the testing strategies
are too numerous for a single agreement to cover them all. You'll
start with one environment variable and end up with 22, each meaning
slight variations of the same thing.

Why not just specify the various taggings (because that's what this is
- a tagging scheme) in one place?

Plus, if you decide that a given test file belongs in a different
testing scheme, you change the Build.PL, not the test file. This means
that the test has more integrity  because it's been touched less.

> Among other things, the environment variable approach means that the
> selection of tests to run doesn't depend on how up to date a user's M::B
> is. It's a big problem if all the tests are *.t and only the latest M::B
> has the logic for which ones to run.  At least the *.ta approach
> defaults to not running.  On the downside, I think getting people to
> shift to *.ta (and getting Module::Starter, ExtUtils::ModuleMaker, etc.
> to use it) will be a bigger hurdle than changing the example synopsis in
> Test::Pod and the default templates, etc.  (Assuming, of course, that
> those who write such tools, like Andy Lester, agree that opt-in is good
> practice.)

This is a straw man and there's a very easy solution - require the
version of MB that provides this feature. This is no different than
the Test::Simple upgrade from 0.46 to 0.47 which had the major API
change. Ever wonder why every Build.PL in existence has:

    build_requires =3D> {
        'Test::More' =3D> '0.47',
    },

This would be a similar API change and would have the exact same
solution. And, if you don't want to use the feature, then you don't
have to require a specific MB version.

I do have another option - I write and maintain Module::Build::Rob
that provides this feature. Except, I don't want to do that because:
    a) I have enough on my plate
    b) The name sucks and I can't think of a better one
    c) I don't want to.

So, maybe this becomes a feature of a MB subclass that is bundled with
the next release of MB. I choose to use it and require people who
install my distros to upgrade. You don't use it and your users don't
have to upgrade to install your distros.

Is that fair?

Rob

Re: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 10:52 PM, Chris Dolan wrote:

> On Feb 2, 2006, at 10:26 PM, Ken Williams wrote:
>
>> On Feb 2, 2006, at 10:18 PM, Chris Dolan wrote:
>>> Well, if it throws an exception, it's probably not a version 
>>> instance, right?
>>
>> Why not?  It could be a version instance that overloads isa() for 
>> whatever satanic purpose it wants to, and throws an exception, 
>> because it's evil and it can.
>
> I don't think version.pm is actually in league with the devil. :-)

I was referring to some possibly strange & unknown class that 
subclasses version.pm.

> Presumably, version would only override isa() if it really, really 
> needed to be overridden.  Why should we ignore that decision that was 
> probably made with great care?

Because IMO even if it was made with great care it's wrong.  You can't 
change perl's notion of what constitutes a subclass even if you do 
override isa().

As I published in a review of UNIVERSAL::isa() last night, this is very 
different from overriding can().  For can(), the author does have the 
ability to override perl's dispatch of method calls via AUTOLOAD(), so 
overriding can() is a good idea.  For isa(), we can't override perl's 
OO inheritance notions, so overriding isa() is a bad idea and will 
leave the caller in a state of confusion over who's right, perl or 
isa().


> No, that's not right.  What you want to know is if the $version is not 
> a plain scalar and therefore whether it needs to be numify()ed.  
> isa('version') is a close proxy for that decision.

Just because it's not a plain scalar, though, doesn't mean it needs to 
be numified.  Theoretically someone has written their *own* class for 
version objects, independent of version.pm, which they think is going 
to handle things their own special way without use of a numify() 
method.

In this case we really *do* want to know whether the $version inherits 
from version.pm, because then it will have a published numify() method 
with known semantics.  If it's some other class, we know nothing.

  -Ken

Re: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 3, 2006, at 6:17 AM, John Peacock wrote:

> Ken Williams wrote:
>>> While we're at it, we should change the
>>>   UNIVERSAL::isa( $result, 'version' )
>>> to
>>>   eval { $result->isa('version') }
>>
>> That's probably better, but it has its own problems.  What if 
>> someone's
>> overloaded isa() object is trying to throw an exception?  This will
>> throw it away.
>
> But we aren't talking about some random class.  This *only* applies to
> version.pm and I can personally guarantee that it will never include an
> overloaded isa() method (I know the author).
>
> Just use the original patch I suggested with ->numify() and 
> UNIVERSAL::isa() and
> worry about the theoretical best practices someplace else... ;-)

Yup, that's what I've done.

  -Ken

Re: Different types of tests (was Re: [Module::Build] [RFC] author tests)

[David G. <da...@hy...>]

Rob Kinyon wrote:
> Here's the rationale behind my proposal - currently, there's no
> mechanism for specifying different types of tests in either MB or
> EUMM. Essentially, that's what this proposal is all about. I see a lot
> of the confusion arising because different people want different kinds
> of tests and don't conceive of them as "author tests", but as "dist
> tests", "unit tests", "packaging tests", "acceptance tests", "system
> tests", etc ad nauseum.

I think this will mean massive overbuilding of a framework without a 
clear need for that type of functionality.

If we remember where this all started -- it's the desire not to inflict 
"pod tests" in *.t files on end users.  Thanks to CPANTS and Kwalitee -- 
and not helped by Module::Starter::PBP -- there's a ton of cargo-cult 
non-functional tests winding up in distributions.

What's missing in all that cargo-culting is a widely accepted mechanism 
for opting in/out of the tests.  Building something into the toolchain 
-- Module::Build and ExtUtils::MakeMaker -- (and then getting that into 
the Test::Pod docs, maybe CPAN/CPANPLUS, etc.) is just a way of driving 
a standard.

I'd prefer to KISS -- look for the simplest solution that could possibly 
work.  So just because M::B, etc. is a conveient place to push the 
standard doesn't mean that the standard should be (a) overworked or (b) 
too tightly coupled to M::B specifically.

I prefer the environment variable approach because, while it's global -- 
it's one of easiest methods of interprocess communication between a 
build framework, the test harness and test files.

What I'd *really* like to see is the community agree on an environment 
variable that signals that "author" testing (for lack of a better term) 
is desired.  And then I'd like to see Test::Pod, Test::Pod::Coverage, 
etc. look for that environment variable and skip running their tests 
unless it exists.  In other words -- I'd like the skip logic be a 
convention followed by those who write tests rather than something 
hacked up for all the different ways people might write/structure/run tests.

Among other things, the environment variable approach means that the 
selection of tests to run doesn't depend on how up to date a user's M::B 
is. It's a big problem if all the tests are *.t and only the latest M::B 
has the logic for which ones to run.  At least the *.ta approach 
defaults to not running.  On the downside, I think getting people to 
shift to *.ta (and getting Module::Starter, ExtUtils::ModuleMaker, etc. 
to use it) will be a bigger hurdle than changing the example synopsis in 
Test::Pod and the default templates, etc.  (Assuming, of course, that 
those who write such tools, like Andy Lester, agree that opt-in is good 
practice.)

Regards,
David

Different types of tests (was Re: [Module::Build] [RFC] author tests)

[Rob K. <rob...@gm...>]

I'm reposting the below snippet because I don't think anyone has
seriously considered this option (or, if they have, they haven't
replied back, except for Chris Dolan whose concerns I've addressed and
I have that email below as well.)

Here's the rationale behind my proposal - currently, there's no
mechanism for specifying different types of tests in either MB or
EUMM. Essentially, that's what this proposal is all about. I see a lot
of the confusion arising because different people want different kinds
of tests and don't conceive of them as "author tests", but as "dist
tests", "unit tests", "packaging tests", "acceptance tests", "system
tests", etc ad nauseum.

So, I'm going to expand my proposal below and generalize it to any
kind of test type:

1) The current "./Build test" is really "./Build test
type=3Dacceptance". It's expected that these tests will run on the
user's machine and is the gatekeeper as to whether CPAN(PLUS) will
install or not.
2) The maintainer needs to be able to specify a hierarchy of test
types along with which test files belong under which type.
3) A test file may belong under more than one type.
4) The type "acceptance" will be the default type if no type is
specified. This preserves backward compatibility with the current MB
semantics as well as the expectations of the Perl community at large.
4) The type "full" is a synonym for the type "all".
5) If "build_makefile =3D> 'traditional'" is specified, then the TESTS
parameter for WriteMakefile() will be given an explicity list of all
the testfiles that fall under the "acceptance" type. This preserves
the current EU::MM semantics.
6) "./Build test_foo" will run all the tests under the type "foo".
This is shorthand for "./Build test type=3Dfoo".

If anyone wants, I'll gladly whip up the first iteration of this
functionality this weekend.

Thanks,
Rob

On 2/2/06, Rob Kinyon <rob...@gm...> wrote:
> What about doing the following:
> 1) Create a testfull action (vs. the normal test action - see
> testcover as precedence). This will run all the tests in test_files
> and fulltest_files
> 2) test_files is kept as t/*.t
> 3) fulltest_files is added as an option
>
> If you want to differentiate between regular tests and full tests,
> then you, as the author, are expected to use fulltest_files (and
> possibly test_files) to differentiate. Anything listed in
> fulltest_files (globs permitted) will be EXCLUDED from test_files.
>
> This is, obviously, not quite compatible with EU::MM, but I don't
> think that's a problem.

On 2/2/06, Chris Dolan <ch...@cl...> wrote:
> On Feb 2, 2006, at 9:23 AM, Rob Kinyon wrote:
> > This is, obviously, not quite compatible with EU::MM, but I don't
> > think that's a problem.
>
> That last paragraph is the show-stopper.  If it doesn't work with
> EU::MM then it's DOA.  This is why I initially proposed the *.ta
> suffix, but now prefer the envvar trigger.  The envvar trigger is the
> most backward- and cross-compatible suggestion to date, at the
> expense of making the *.t files a tiny bit more complicated.

It's not INcompatible with EU::MM ... just not completely compatible.
If you use a MB-generated Makefile.PL (as I do), then MB would specify
the test files by hand using the TESTS parameter to WriteMakefile(). I
don't think there's an easy way to add to a new command to EU::MM, but
if there was, then we could add the testfull command there, too.

If you hand-generate your Makefile.PL, then it's your responsability
to make sure that it's equivalent to the Build.PL, as it always has
been.

Re: [Module::Build] version.pm and version number comparison problem

[John P. <jpe...@ro...>]

Ken Williams wrote:
>> While we're at it, we should change the
>>   UNIVERSAL::isa( $result, 'version' )
>> to
>>   eval { $result->isa('version') }
> 
> That's probably better, but it has its own problems.  What if someone's
> overloaded isa() object is trying to throw an exception?  This will
> throw it away.

But we aren't talking about some random class.  This *only* applies to
version.pm and I can personally guarantee that it will never include an
overloaded isa() method (I know the author).

Just use the original patch I suggested with ->numify() and UNIVERSAL::isa() and
worry about the theoretical best practices someplace else... ;-)

John

-- 
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4720 Boston Way
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747

Re: [Module::Build] [RFC] author tests

[David G. <da...@hy...>]

Ron Savage wrote:
> The killer for an env var is that I for one install modules via a cgi script on
> machines where I do /not/ have access to httpd.conf, so I can't ever use
> Perl(Set)Env to pass personal env vars thru to my scripts.

But do you need to run all the author tests, then?

David

Re: [Module::Build] [RFC] author tests

[Ron S. <ro...@sa...>]

On Thu, 2 Feb 2006 21:47:21 -0600, Ken Williams wrote:

Hi Ken

> In general I like the idea.  In specific I don't like the idea of
> using an environment variable.  Could we use some parameter or
> command-line argument or something like that instead?  Environment
> variables aren't completely portable and they're about as global as
> one can get, so usually I've tried to avoid using them in M::B.

The killer for an env var is that I for one install modules via a cgi script on
machines where I do /not/ have access to httpd.conf, so I can't ever use
Perl(Set)Env to pass personal env vars thru to my scripts.

This is the danger of admin-centric proposals.
--
Ron Savage
ro...@sa...
http://savage.net.au/index.html

Re: [Module::Build] [RFC] author tests

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 9:19 PM, Yitzchak Scott-Thoennes wrote:

> Chris, how are you currently set up to run these tests only when
> preparing a release?

I make no such distinction.  Instead, I see these tests as part of my  
day-to-day development and run all of them with every "./Build  
test".  Presently, I simply include them all in my MANIFEST.SKIP so  
they are not included in my uploaded package.

That's why I call these "author tests" and not "release tests" or  
"exhaustive tests".

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: [Module::Build] version.pm and version number comparison problem

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 10:26 PM, Ken Williams wrote:

> On Feb 2, 2006, at 10:18 PM, Chris Dolan wrote:
>> Well, if it throws an exception, it's probably not a version  
>> instance, right?
>
> Why not?  It could be a version instance that overloads isa() for  
> whatever satanic purpose it wants to, and throws an exception,  
> because it's evil and it can.

I don't think version.pm is actually in league with the devil. :-)   
Presumably, version would only override isa() if it really, really  
needed to be overridden.  Why should we ignore that decision that was  
probably made with great care?

> What I really want to know, and what UNIVERSAL::isa() tells me, is  
> whether there's an @ISA-chain up to 'version'.

No, that's not right.  What you want to know is if the $version is  
not a plain scalar and therefore whether it needs to be numify()ed.   
isa('version') is a close proxy for that decision.

Sorry to drag this out -- I know you already abstained, Ken -- but it  
took *me* a long time to grasp this through a series of emails with  
nothingmuch, so I felt that I should share.

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: [Module::Build] version.pm and version number comparison problem

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 10:17 PM, Ken Williams wrote:

> On Feb 2, 2006, at 10:22 AM, Chris Dolan wrote:
>
>> While we're at it, we should change the
>>   UNIVERSAL::isa( $result, 'version' )
>> to
>>   eval { $result->isa('version') }
>
> BTW, I actually tried this, and it broke the t/xs.t tests.  I'll  
> leave the reason why as an interesting exercise for the reader. =)
>
>  -Ken


[SPOILER BELOW!]


  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .
  .

It's a false failure.  t/xs.t is calling dispatch() and testing $@ to  
see if they fail.  The die() triggered by isa() inside the eval sets  
$@, and the test fails because $@ is set.  However, this is because t/ 
xs.t mistakenly omits wrapping some of the calls to dispatch() in  
their own eval{}.  The following test (#2 of 22) passes in xs.t:

    eval {$mb->dispatch('build')};
    is $@, '';

but this test (#20 of 22) erroneously fails:

    $mb->dispatch('build');
    is $@, '';

So the test is checking $@ from the wrong eval.


The moral of the story is that using best practices help you find  
bugs in code.  ;-)

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: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 10:18 PM, Chris Dolan wrote:

> On Feb 2, 2006, at 10:10 PM, Ken Williams wrote:
>
>> That's probably better, but it has its own problems.  What if 
>> someone's overloaded isa() object is trying to throw an exception?  
>> This will throw it away.
>
> Well, if it throws an exception, it's probably not a version instance, 
> right?

Why not?  It could be a version instance that overloads isa() for 
whatever satanic purpose it wants to, and throws an exception, because 
it's evil and it can.

What I really want to know, and what UNIVERSAL::isa() tells me, is 
whether there's an @ISA-chain up to 'version'.

  -Ken

Re: [Module::Build] version.pm and version number comparison problem

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 10:10 PM, Ken Williams wrote:

> On Feb 2, 2006, at 10:22 AM, Chris Dolan wrote:
>
>> On Feb 2, 2006, at 8:06 AM, John Peacock wrote:
>>
>>>
>>>    # Unbless it if it's a version.pm object
>>> -  $result = "$result" if UNIVERSAL::isa( $result, 'version' );
>>> +  $result = $result->numify if UNIVERSAL::isa( $result,  
>>> 'version' );
>>
>> While we're at it, we should change the
>>   UNIVERSAL::isa( $result, 'version' )
>> to
>>   eval { $result->isa('version') }
>
> That's probably better, but it has its own problems.  What if  
> someone's overloaded isa() object is trying to throw an exception?   
> This will throw it away.

Well, if it throws an exception, it's probably not a version  
instance, right?

> Personally I think inheritance is a language feature, and we ought  
> to have a language feature (like blessed() or tied()), not an  
> inheritable method, to inspect it.  But other people are more  
> passionate about this than me, so I won't argue.

As I've said in another forum, the isa() and can() stuff is just  
broken and we can only hope that Perl6 gets it right.  I'm grateful  
to John Peacock for trying to get version right at least.

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: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 10:22 AM, Chris Dolan wrote:

> On Feb 2, 2006, at 8:06 AM, John Peacock wrote:
>
>>
>>    # Unbless it if it's a version.pm object
>> -  $result = "$result" if UNIVERSAL::isa( $result, 'version' );
>> +  $result = $result->numify if UNIVERSAL::isa( $result, 'version' );
>
> While we're at it, we should change the
>   UNIVERSAL::isa( $result, 'version' )
> to
>   eval { $result->isa('version') }

BTW, I actually tried this, and it broke the t/xs.t tests.  I'll leave 
the reason why as an interesting exercise for the reader. =)

  -Ken

Re: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 8:06 AM, John Peacock wrote:

> However, happily, I have a solution (diff vs. Module-Build-0.27_07):
>
> --- lib/Module/Build/ModuleInfo.pm.orig 2006-02-02 08:46:21.000000000 
> -0500
> +++ lib/Module/Build/ModuleInfo.pm      2006-02-02 08:59:23.000000000 
> -0500
> @@ -299,7 +299,7 @@ sub _evaluate_version_line {
>    warn "Error evaling version line '$eval' in $self->{filename}: 
> $@\n" if $@;
>
>    # Unbless it if it's a version.pm object
> -  $result = "$result" if UNIVERSAL::isa( $result, 'version' );
> +  $result = $result->numify if UNIVERSAL::isa( $result, 'version' );
>
>    return $result;
>  }

That was awfully easy.  I wonder why we didn't think of this before. =)

I'll apply.

  -Ken

Re: [Module::Build] version.pm and version number comparison problem

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 10:22 AM, Chris Dolan wrote:

> On Feb 2, 2006, at 8:06 AM, John Peacock wrote:
>
>>
>>    # Unbless it if it's a version.pm object
>> -  $result = "$result" if UNIVERSAL::isa( $result, 'version' );
>> +  $result = $result->numify if UNIVERSAL::isa( $result, 'version' );
>
> While we're at it, we should change the
>   UNIVERSAL::isa( $result, 'version' )
> to
>   eval { $result->isa('version') }

That's probably better, but it has its own problems.  What if someone's 
overloaded isa() object is trying to throw an exception?  This will 
throw it away.

Personally I think inheritance is a language feature, and we ought to 
have a language feature (like blessed() or tied()), not an inheritable 
method, to inspect it.  But other people are more passionate about this 
than me, so I won't argue.

  -Ken

Re: [Module::Build] [RFC] author tests

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 12:24 PM, Eric Wilhelm wrote:

> # from Tyler MacDonald
> # on Thursday 02 February 2006 10:01 am:
>
>> Whatever convention you're using, if these tests are only going to
>> work on your system, then they definately shouldn't be in "t".
>
> There is a good case for that, particularly in reverse-compatibility.

Nah.  Just don't name them *.t, as Chris originally suggested, and they 
won't cause any trouble in t/.  I like the *.ta or *.at idea.

  -Ken

Re: [Module::Build] [RFC] author tests

[Ken W. <ke...@ma...>]

On Feb 2, 2006, at 10:07 AM, Chris Dolan wrote:

> On Feb 2, 2006, at 6:49 AM, David Golden wrote:
>
>> Eric Wilhelm wrote:
>>> I guess I'm undecided on this.  On the one hand, the *.ta convention 
>>> is going to break the test juggler with which I've recently been 
>>> experimenting, but on the other it means there is less 'whoami' code 
>>> in the tests.  An example of my current whoami-based approach is 
>>> below.
>>
>> My thought was that if someone wants to use *.ta files or a ta/*.t 
>> directory, they can still do so by subclassing in Build.PL and having 
>> a custom authortest action that includes those files.
>
> Oh, sure, everything I've suggested in this thread *could* be done as 
> Build.PL customization.  But the reason I brought this up was to try 
> to achieve some consensus and find a Best Practice before the Cargo 
> Cult started by Test::Pod takes a deeper hold.

There's already a property called 'recursive_test_files' that would 
presumably cover this need.

In general I like the idea.  In specific I don't like the idea of using 
an environment variable.  Could we use some parameter or command-line 
argument or something like that instead?  Environment variables aren't 
completely portable and they're about as global as one can get, so 
usually I've tried to avoid using them in M::B.


> I like the thought of having this as a feature in M::B because then it 
> flattens the learning curve for a third party to mess with my author 
> tests.  And as a corollary, I don't have to scratch my head as much 
> when looking at someone else's author tests.

Right, and it becomes a paradigm that people just know about, rather 
than having to scrutinize some author's custom build system.

  -Ken

Re: [Module::Build] [RFC] author tests

[Yitzchak Scott-T. <sth...@ef...>]

On Thu, Feb 02, 2006 at 10:01:48AM -0800, Tyler MacDonald wrote:

I strongly feel that authors should keep everything necessary
for their distribution public; either in the CPAN distribution
itself, or via a "permanent" publicly available version control
system.

Who's to say you won't lose interest in maintaining the module or in
perl altogether at some point?  Or move to Antarctica and be unable to
maintain it?  Or have your home/business or wherever your files are
kept destroyed in a hurricane or other natural disaster?

And, of course, we all will die someday.  Some suddenly.

I do agree that many (all?) of these tests are irrelevant to someone
packaging your distribution.

> Chris Dolan <ch...@cl...> wrote:
> >  * copyright.t - Ensures that there is a "Copyright ".([localtime]-> 
> > [5]+1900) somewhere in every .pm file.  Will break 11 months from now.
> >  * distribution.t - Relies on Test::Distribution, which is not in my  
> > prereq list
> >  * perlcritic.t - Runs Test::Perl::Critic on all .pm files.  Will  
> > fail without my specific $HOME/.perlcriticrc

"Test::Perl::Critic allows you to configure Perl::Critic by passing the
path to a F<perlcriticrc> file in the C<use> pragma.  For example:

  use Test::Perl::Critic (-profile => 't/perlcriticrc');
  all_critic_ok();"

Probably you'd like this to keep in sync with any changes to your
.perlcriticrc, which may require some changes to your Makefile.PL/
Build.PL.

> > and will fail with future, more exhaustive versions of P::C

It would be nice if there was some way to indicate which version of
P::C was expected to pass, and TODO any newly looked for problems.

> >  * spelling.t - Runs Test::Spelling.  Will fail without my custom  
> > dictionary

There's Test::Spelling::add_stopwords and =for stopwords.  These
should be used as much as possible instead of adding to your custom
dictionary.  But at least your custom dictionary is recreatable
(because all the needed words are included in your pod :) should
someone else pick up maintenance of your distribution.

> >  * versionsync.t - Checks that the $VERSION is the same in all bin/*  
> > and *.pm files.  This test is pointless after release, since it's  
> > already been tested before release
> >  * pod.t - Checks POD validity.  This test is pointless after  
> > release, since it's already been tested before release
> >  * pod-coverage.t - Checks POD completeness.  This test is pointless  
> > after release, since it's already been tested before release
> > 
> > and one I have not yet employed:
> >  * coverage.t - Ensures that Devel::Cover totals are higher than  
> > some threshold
> 
> Wow, you really *are* exhaustive. How do you find the time to write any
> code? ;-)
> 
> Now that I understand exactly what you mean by "author" tests, here's what I
> think: Whatever convention you're using, if these tests are only going to
> work on your system, then they definately shouldn't be in "t". And since
> there's absolutely no value in these types of tests for anybody else except
> the module author, there's no real point in having a convention, just stick
> 'em anywhere that the make/buildfiles will ignore them.

I disagree; presumably anyone running disttest would want these tests run,
so they belong in t and named .t, with an appropriate skip_all at the top.

Does anyone have a problem with disttest setting PERL_TEST_EXHAUSTIVE?
Or a suggestion for a better name?

Chris, how are you currently set up to run these tests only when
preparing a release?

Re: [Module::Build] version.pm and version number comparison problem

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 3:10 PM, Julian Mehnle wrote:

> All that doesn't help much as long as the "UNIVERSAL" POD is part  
> of the
> official Perl distribution, while the "UNIVERSAL::isa" POD is not.   
> This
> makes the "UNIVERSAL" POD look authoritative, not the other POD.
>
> Besides, the "UNIVERSAL::isa" POD doesn't explain the issue very  
> well, see
> above.  And why is it titled "Hack around people calling  
> UNIVERSAL::can()
> as a function" if it is supposed to actually _improve_ code, which  
> is not
> normally the case with _hacks_?

Look again at Perl 5.8.8 or 5.9.3.  The UNIVERSAL.pm POD was updated  
(by chromatic, I think, a month or two ago) to reflect the current  
accepted best practice.  To quote:

        "UNIVERSAL" provides the following methods:

        "$obj->isa( TYPE )"
        "CLASS->isa( TYPE )"
        "eval { VAL->isa( TYPE ) }"

	[... snip ...]

        You may request the import of all three functions ("isa",  
"can", and
        "VERSION"), however it is usually harmful to do so.  Please  
don't do
        this in new code.


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: [Module::Build] [RFC] author tests

[Christopher H. L. <cl...@ch...>]

Chris Dolan wrote:
> On Feb 2, 2006, at 3:06 PM, Christopher H. Laco wrote:
>=20
>> Chris Dolan wrote:
>>> Test::Pod is different from general regression tests.  POD should eit=
her
>>> pass on all systems or fail on all.  In what scenario would POD fail =
on
>>> a non-developer system.
>>
>> Manifying problems...ala troff/pod2html/pod2man issues....
>=20
> This is news to me.  So what do you do with CPAN packages that don't
> include pod.t and have broken POD?  Just fail to install them?
>=20
> It seems to me that to solve manifying problems you need *authors* to
> run Test::Pod, not end users since the latter have no power to fix the
> distro.  If you are a packager, then you probably have to run Test::Pod=

> by hand anyway since so many packages lack pod.t.
>=20
> 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
>=20
> Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
> (http://www.media-landscape.com/) and partners in the revolutionary
> Croquet project (http://www.opencroquet.org/)
>=20
>=20
>=20
Yes, and No. The author can run pod tests and ensure correct pod.

What the packager or end user may not know, and the author can not know,
is if the various pod parsing modules on the users system are kosher, or
if the end users versions will have issues converting the _correct_ pod
into other formats.  Pod syntax test can be the first sign of thing
being wrong they may leave the user without readable docs assuming an
install succeeds...

While true, it's not the module authors job to ensure the users system
can properly spit out man/html pages from the good pod, it's always nice
to know that some OSes version of perl/pod2*/Pod:: is going to cause
your dist to fail the make install "manifying pod" portion...and a
failing dist comes right back into the authors lap..or the packagers...

Sure, it's rare, but it does happen...

-=3DChris

Re: [Module::Build] [RFC] author tests

[Chris D. <ch...@cl...>]

On Feb 2, 2006, at 3:06 PM, Christopher H. Laco wrote:

> Chris Dolan wrote:
>> Test::Pod is different from general regression tests.  POD should  
>> either
>> pass on all systems or fail on all.  In what scenario would POD  
>> fail on
>> a non-developer system.
>
> Manifying problems...ala troff/pod2html/pod2man issues....

This is news to me.  So what do you do with CPAN packages that don't  
include pod.t and have broken POD?  Just fail to install them?

It seems to me that to solve manifying problems you need *authors* to  
run Test::Pod, not end users since the latter have no power to fix  
the distro.  If you are a packager, then you probably have to run  
Test::Pod by hand anyway since so many packages lack pod.t.

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: [Module::Build] version.pm and version number comparison problem

[Julian M. <ju...@me...>]

John Peacock wrote:
> Julian Mehnle wrote:
> > "This is wrong and you should not do it." -- What kind of
> > justification is that??
>
> The POD has a little more than that (from UNIVERSAL::can):
>=20
> | Some authors call methods in the UNIVERSAL class on potential
> | invocants as functions, bypassing any possible overriding. This is
> | wrong and you should not do it. Unfortunately, not everyone heeds
> | this warning and their bad code can break your good code.

That's not exactly explaining the issue well.

Rob Kinyon wrote:
> > Why is it wrong to do UNIVERSAL::isa($foo, $type)?  This usage is
> > documented in `perldoc UNIVERSAL`, and it works fine, after all.
>
> What if I overload can() in order to correctly report what I support
> through AUTOLOAD because I will handle dynamic attributes?
> UNIVERSAL::can() fails there.
>
> What if I overload isa() in order to report back a facade object is
> really what it's decorating? UNIVERSAL::isa() fails there, too.

Thanks for the plausible explanation!

John Peacock wrote:
> > Why is it wrong to do UNIVERSAL::isa($foo, $type)?  This usage is
> > documented in `perldoc UNIVERSAL`, and it works fine, after all.
>
> [The "UNIVERSAL" POD] was written a long time ago and should not be
> considered to be the only way to think about it.  As it turns out,
> UNIVERSAL::isa (the core code, not the CPAN replacement) was written in a
> poorly designed way that does not DTRT with some objects that are now in
> usage.=20

All that doesn't help much as long as the "UNIVERSAL" POD is part of the=20
official Perl distribution, while the "UNIVERSAL::isa" POD is not.  This=20
makes the "UNIVERSAL" POD look authoritative, not the other POD.

Besides, the "UNIVERSAL::isa" POD doesn't explain the issue very well, see=
=20
above.  And why is it titled "Hack around people calling UNIVERSAL::can()=20
as a function" if it is supposed to actually _improve_ code, which is not=20
normally the case with _hacks_?

Re: [Module::Build] [RFC] author tests

[Christopher H. L. <cl...@ch...>]

Chris Dolan wrote:
> On Feb 2, 2006, at 12:11 PM, Tyler MacDonald wrote:
>=20
>>     If they're not a developer, what are they doing running unit tests=

>> in the first place??? End users typically install applications, which
>> either
>> prebundle a whole bunch of perl packages or rely on some package
>> management
>> system (ppm, deb, etc) to separate real dependancies from build
>> dependancies
>> for them and only install what they need.
>=20
> The end users may be in a different environment from me.  I usually onl=
y
> tests Perl 5.8.6 on OS X and Linux before release, so I rely on other
> installers to provide bug reports for me.
>=20
>>     I think in this case Test::Pod should be a build_requires... Your
>> other examples (copyright.t, etc) obviously will only work on your
>> system so
>> it makes sense for those to have some sort of special treatment.
>=20
> Test::Pod is different from general regression tests.  POD should eithe=
r
> pass on all systems or fail on all.  In what scenario would POD fail on=

> a non-developer system. =20


Manifying problems...ala troff/pod2html/pod2man issues....

-=3DChris