Bug #127383 for Perl-PrereqScanner-NotQuiteLite: Some recent change breaks handling of .pm files under t/ dirs

Tue Oct 16 22:35:17 2018 BAREFOOT [...] cpan.org - Ticket created

Subject:

Some recent change breaks handling of .pm files under t/ dirs

I think I can demonstrate this minimally: ``` [caemlyn:~] mkdir t [caemlyn:~] echo 'use Test::Warnings;' > t/T.pm [caemlyn:~] scan-perl-prereqs-nqlite --version scan-perl-prereqs-nqlite v0.93 [caemlyn:~] scan-perl-prereqs-nqlite t/T.pm Test::Warnings ``` vs ``` vagrant@ce-dev-sandbox:~$ mkdir t vagrant@ce-dev-sandbox:~$ echo 'use Test::Warnings;' > t/T.pm vagrant@ce-dev-sandbox:~$ scan-perl-prereqs-nqlite --version scan-perl-prereqs-nqlite v0.96 vagrant@ce-dev-sandbox:~$ scan-perl-prereqs-nqlite t/T.pm ``` I'm not entirely sure, but at a guess this is caused by the addition of the codeblock that starts with the comment "add test requirements by .pm files used in .t files" (in Perl::PrereqScanner::NotQuiteLite::App). I could take a crack at finding the fix, but I was betting you'd have a better idea where the problem lies, given the code is less than a month old. :-) Let me know if you need any further info. And thx so much for this module! Outside of this minor issue, this is hands-down the best dependency scanner on CPAN.

Wed Oct 17 00:27:51 2018 ishigaki [...] cpan.org - Correspondence added

On Wed Oct 17 11:35:17 2018, BAREFOOT wrote: Show quoted text

> I think I can demonstrate this minimally: > ``` > [caemlyn:~] mkdir t > [caemlyn:~] echo 'use Test::Warnings;' > t/T.pm > [caemlyn:~] scan-perl-prereqs-nqlite --version > scan-perl-prereqs-nqlite v0.93 > [caemlyn:~] scan-perl-prereqs-nqlite t/T.pm > Test::Warnings > ``` > vs > ``` > vagrant@ce-dev-sandbox:~$ mkdir t > vagrant@ce-dev-sandbox:~$ echo 'use Test::Warnings;' > t/T.pm > vagrant@ce-dev-sandbox:~$ scan-perl-prereqs-nqlite --version > scan-perl-prereqs-nqlite v0.96 > vagrant@ce-dev-sandbox:~$ scan-perl-prereqs-nqlite t/T.pm > ``` > > I'm not entirely sure, but at a guess this is caused by the addition > of the codeblock that starts with the comment "add test requirements > by .pm files used in .t files" (in > Perl::PrereqScanner::NotQuiteLite::App). I could take a crack at > finding the fix, but I was betting you'd have a better idea where the > problem lies, given the code is less than a month old. :-) > > Let me know if you need any further info. And thx so much for this > module! Outside of this minor issue, this is hands-down the best > dependency scanner on CPAN.

Thanks for the report. It is an intentional change to exclude modules in .pm files that are only included as a corpus, namely to be read from a test to find something etc. In the case above, as t/T.pm is not used in any .t files, make test (or prove -l t) should pass without Test::Warnings (if there is a test). However, this behavior may not be optimal if you use something like Test::Class that loads implicitly modules under t. It may be useful to add some option to find dependencies more eagerly. What do you think?

Wed Oct 17 00:27:52 2018 The RT System itself - Status changed from 'new' to 'open'

Wed Oct 17 00:32:06 2018 BAREFOOT [...] cpan.org - Correspondence added

Okay, I've done a local patch on my side, so I know how to "fix" it now. But I see that this is a deliberate feature that you've added on your side, so perhaps I'd best explain why it's not working for me. :-) We use Test::Class::Most almost exclusively for our testing, so there are actually very few .t files in our t/ dirs. What few there are tend to build lists of .pm files to process by scanning the filesystem, so that's why your algorithm isn't picking them up. This seems to be introduced in a couple of commits around v0.96. What I _thought_ was the offending code was the big block in commit 5d46a03d29, but it turns out that it's really the `elsif ($relpath =~ /\.pm$/)` condition from commit ac83f28837. So I just changed that to a `$self->_add` in my patch and it all works for me. (Side note: I originally thought I'd just revert back to a pre-0.96 version, but 0.96 also adds the code that generates a decent `build` phase list, so I want to keep that functionality.) For your side, perhaps you could have a command-line switch to allow people to control this behavior? If you think that's a decent idea, let me know what you think such a switch should be called and I can easily submit you a patch that will keep everything working for everyone. :-)

Wed Oct 17 00:34:01 2018 BAREFOOT [...] cpan.org - Correspondence added

Show quoted text

> However, this behavior may not be optimal if you use something like > Test::Class that loads implicitly modules under t. It may be useful to > add some option to find dependencies more eagerly. > > What do you think?

Hah! Our replies got crossed in the Internet streams, methinks. :-D Yep, as I said in my last message, I think that's the best solution. Throw out a name and I'll work up a pull request.

Wed Oct 17 00:57:16 2018 ishigaki [...] cpan.org - Correspondence added

On Wed Oct 17 13:34:01 2018, BAREFOOT wrote: Show quoted text

> > However, this behavior may not be optimal if you use something like > > Test::Class that loads implicitly modules under t. It may be useful > > to > > add some option to find dependencies more eagerly. > > > > What do you think?

> > Hah! Our replies got crossed in the Internet streams, methinks. :-D > > Yep, as I said in my last message, I think that's the best solution. > Throw out a name and I'll work up a pull request.

Thanks. As I'm not a native English speaker, could you name it for me? Also, maybe it's useful to turn the switch on implicitly if any of Test::Class family (Test::Class::Most, Test::Class::Moose, Test::Class::Sugar, Test::Classy...) is found as a test prereq.

Thu Oct 18 17:06:06 2018 BAREFOOT [...] cpan.org - Correspondence added

Show quoted text

> Thanks. As I'm not a native English speaker, could you name it for me?

Well, it's a tough one ... I suppose that, if you favor longer names (which admittedly I do), then I would probably go with something like `--allow-test-pms`. If you're looking for something more concise, then I supopse you want to imply that the checking is less strict, for which the common term is "lax." I don't particularly like that in this situation though; maybe something more like "inclusive," or ... actually that's all I can think of. :-D But, if you're telling me I can pick whatever I like, I'll just go with `--allow-test-pms`, which is wordy, but I think very clear. Show quoted text

> Also, maybe it's useful to turn the switch on implicitly if any of > Test::Class family (Test::Class::Most, Test::Class::Moose, > Test::Class::Sugar, Test::Classy...) is found as a test prereq.

Oh, as a potential _user_, I think that's brilliant and I'd love to have it. :-) As a potential _contributor_, though, I'm not quite sure how to effect that. ;-> How about I get it working with a command-line switch first, and then we can look at figuring out a heuristic for flipping the switch automatically?

Thu Oct 18 22:47:51 2018 ishigaki [...] cpan.org - Correspondence added

On Fri Oct 19 06:06:06 2018, BAREFOOT wrote: Show quoted text

> > Thanks. As I'm not a native English speaker, could you name it for > > me?

> > Well, it's a tough one ... I suppose that, if you favor longer names > (which admittedly I do), then I would probably go with something like > `--allow-test-pms`. If you're looking for something more concise, > then I supopse you want to imply that the checking is less strict, for > which the common term is "lax." I don't particularly like that in > this situation though; maybe something more like "inclusive," or ... > actually that's all I can think of. :-D But, if you're telling me I > can pick whatever I like, I'll just go with `--allow-test-pms`, which > is wordy, but I think very clear.

Then let's go with --allow-test-pms :) Show quoted text

> > Also, maybe it's useful to turn the switch on implicitly if any of > > Test::Class family (Test::Class::Most, Test::Class::Moose, > > Test::Class::Sugar, Test::Classy...) is found as a test prereq.

> > Oh, as a potential _user_, I think that's brilliant and I'd love to > have it. :-) As a potential _contributor_, though, I'm not quite sure > how to effect that. ;-> How about I get it working with a command- > line switch first, and then we can look at figuring out a heuristic > for flipping the switch automatically?

OK. I'll take this additional part as I have CPANTS to see if it really works. Thanks for your advice. I look forward to your PR about command line switch.

Mon Oct 29 13:58:01 2018 PCRONIN [...] cpan.org - Correspondence added

It seems I also have this problem. Glad I checked open issues before reporting it myself. I'm trying to use Perl-PrereqScanner-NotQuiteLite to scan a Perl project for CPAN dependencies as part of a Travis build config. It seems to miss dependencies from my testing library. The project (https://github.com/PatrickCronin/foegcl-membership-webapp) uses Test::Class::Moose to instantiate and run several test classes. Thanks to finding this ticket, I tried version 0.95 and it works as expected, but 0.96 and 0.97 do not. For a particular example, from the root of the project linked above, I'm trying to list the dependencies in t/lib/TestHelper/TestLoader.pm. gwana:foegcl-membership-database-old pcronin$ cpanm Perl::PrereqScanner::NotQuiteLite@0.95 --> Working on Perl::PrereqScanner::NotQuiteLite Fetching http://backpan.perl.org/authors/id/I/IS/ISHIGAKI/Perl-PrereqScanner-NotQuiteLite-0.95.tar.gz ... OK Configuring Perl-PrereqScanner-NotQuiteLite-0.95 ... OK Building and testing Perl-PrereqScanner-NotQuiteLite-0.95 ... OK Successfully installed Perl-PrereqScanner-NotQuiteLite-0.95 (downgraded from 0.97) 1 distribution installed gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite --version scan-perl-prereqs-nqlite v0.95 gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite t/lib/TestHelper/TestLoader.pm FOEGCL::Membership::Moose FOEGCL::Membership::Types Module::Runtime Path::Iterator::Rule Path::Tiny gwana:foegcl-membership-database-old pcronin$ cpanm Perl::PrereqScanner::NotQuiteLite@0.96 --> Working on Perl::PrereqScanner::NotQuiteLite Fetching http://backpan.perl.org/authors/id/I/IS/ISHIGAKI/Perl-PrereqScanner-NotQuiteLite-0.96.tar.gz ... OK Configuring Perl-PrereqScanner-NotQuiteLite-0.96 ... OK Building and testing Perl-PrereqScanner-NotQuiteLite-0.96 ... OK Successfully installed Perl-PrereqScanner-NotQuiteLite-0.96 (upgraded from 0.95) 1 distribution installed gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite --version scan-perl-prereqs-nqlite v0.96 gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite t/lib/TestHelper/TestLoader.pm [None listed] gwana:foegcl-membership-database-old pcronin$ cpanm Perl::PrereqScanner::NotQuiteLite@0.97 --> Working on Perl::PrereqScanner::NotQuiteLite Fetching http://www.cpan.org/authors/id/I/IS/ISHIGAKI/Perl-PrereqScanner-NotQuiteLite-0.97.tar.gz ... OK Configuring Perl-PrereqScanner-NotQuiteLite-0.97 ... OK Building and testing Perl-PrereqScanner-NotQuiteLite-0.97 ... OK Successfully installed Perl-PrereqScanner-NotQuiteLite-0.97 (upgraded from 0.96) 1 distribution installed gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite --version scan-perl-prereqs-nqlite v0.97 gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite t/lib/TestHelper/TestLoader.pm [None listed]

Tue Oct 30 14:27:05 2018 ishigaki [...] cpan.org - Correspondence added

On Tue Oct 30 02:58:01 2018, PCRONIN wrote: Show quoted text

> It seems I also have this problem. Glad I checked open issues before > reporting it myself. I'm trying to use Perl-PrereqScanner-NotQuiteLite > to scan a Perl project for CPAN dependencies as part of a Travis build > config. It seems to miss dependencies from my testing library. > > The project (https://github.com/PatrickCronin/foegcl-membership- > webapp) uses Test::Class::Moose to instantiate and run several test > classes. > > Thanks to finding this ticket, I tried version 0.95 and it works as > expected, but 0.96 and 0.97 do not. For a particular example, from the > root of the project linked above, I'm trying to list the dependencies > in t/lib/TestHelper/TestLoader.pm. > > gwana:foegcl-membership-database-old pcronin$ cpanm > Perl::PrereqScanner::NotQuiteLite@0.95 > --> Working on Perl::PrereqScanner::NotQuiteLite > Fetching http://backpan.perl.org/authors/id/I/IS/ISHIGAKI/Perl- > PrereqScanner-NotQuiteLite-0.95.tar.gz ... OK > Configuring Perl-PrereqScanner-NotQuiteLite-0.95 ... OK > Building and testing Perl-PrereqScanner-NotQuiteLite-0.95 ... OK > Successfully installed Perl-PrereqScanner-NotQuiteLite-0.95 > (downgraded from 0.97) > 1 distribution installed > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > --version > scan-perl-prereqs-nqlite v0.95 > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > t/lib/TestHelper/TestLoader.pm > FOEGCL::Membership::Moose > FOEGCL::Membership::Types > Module::Runtime > Path::Iterator::Rule > Path::Tiny > gwana:foegcl-membership-database-old pcronin$ cpanm > Perl::PrereqScanner::NotQuiteLite@0.96 > --> Working on Perl::PrereqScanner::NotQuiteLite > Fetching http://backpan.perl.org/authors/id/I/IS/ISHIGAKI/Perl- > PrereqScanner-NotQuiteLite-0.96.tar.gz ... OK > Configuring Perl-PrereqScanner-NotQuiteLite-0.96 ... OK > Building and testing Perl-PrereqScanner-NotQuiteLite-0.96 ... OK > Successfully installed Perl-PrereqScanner-NotQuiteLite-0.96 (upgraded > from 0.95) > 1 distribution installed > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > --version > scan-perl-prereqs-nqlite v0.96 > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > t/lib/TestHelper/TestLoader.pm > [None listed] > gwana:foegcl-membership-database-old pcronin$ cpanm > Perl::PrereqScanner::NotQuiteLite@0.97 > --> Working on Perl::PrereqScanner::NotQuiteLite > Fetching http://www.cpan.org/authors/id/I/IS/ISHIGAKI/Perl- > PrereqScanner-NotQuiteLite-0.97.tar.gz ... OK > Configuring Perl-PrereqScanner-NotQuiteLite-0.97 ... OK > Building and testing Perl-PrereqScanner-NotQuiteLite-0.97 ... OK > Successfully installed Perl-PrereqScanner-NotQuiteLite-0.97 (upgraded > from 0.96) > 1 distribution installed > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > --version > scan-perl-prereqs-nqlite v0.97 > gwana:foegcl-membership-database-old pcronin$ scan-perl-prereqs-nqlite > t/lib/TestHelper/TestLoader.pm > [None listed]

Shipped 0.98 with allow-test-pm option. Thanks!

Tue Oct 30 14:27:06 2018 ishigaki [...] cpan.org - Status changed from 'open' to 'resolved'