FAQ
Hi Marc, & CPAN Workers,

I’ve been looking into the final two CPAN Testers fails, and have finally got to the bottom of them.
The failing test is search50.t, and the problem is where it does the following:

  - call survey() to get hash of name => path
  - foreach name, then call find() and check it returns the same path

There are two CPAN Testers fails:

  http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee <http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee>
  http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a <http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a>

Basically the problem is that

  - the pod directory has both perlpodstyle and perlpodstyle.pod in it (how come?!)
  - the survey() method doesn’t even find perlpodstyle, it just finds perlpodstyle.pod
  - the find() method finds both and it finds perlpodstyle first
  - so the test failed

With this understanding I reproduced the failing test.

The problem comes down to two lines of code. In _make_search_callback(), which is invoked by survey(), we have:

  unless( m/^[-_a-zA-Z0-9]+\.(?:pod|pm|plx?)\z/is ) {

So it’s only considering files that end in .pod, .pm, .pl, or .plx

But in find() we have:

  foreach my $ext ('', '.pod', '.pm', '.pl') { # possible extensions

As you can see, it first checks for no extension. Also note that it’s not checking for the ‘.plx’ extension, which survey handles. I’ve never come across anyone using the .plx extensions, but I guess for a while maybe people did:

  http://www.perlmonks.org/?node_id=336713 <http://www.perlmonks.org/?node_id=336713>

I think that the best fix is to make the survey() pattern match files with no extensions, and also files with the .plx extension.
This means that survey will scan a number of extra files, but it also scans them looking for pod, so it shouldn’t return any false positive. This would mean that survey would now find scripts that have pod in them, which it currently misses.

Do you agree with the proposed fix, and are there any gotchas I might be missing?

Cheers,
Neil

Search Discussions

  • James E Keenan at Mar 6, 2016 at 2:19 am

    On 03/05/2016 03:56 PM, Neil Bowers wrote:
    Hi Marc, & CPAN Workers,

    I’ve been looking into the final two CPAN Testers fails, and have finally got to the bottom of them.
    The failing test is search50.t, and the problem is where it does the following:

    - call survey() to get hash of name => path
    - foreach name, then call find() and check it returns the same path

    There are two CPAN Testers fails:

    http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee <http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee>
    http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a <http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a>
    Do you have any thoughts on why these occurred on these particular
    OS/Perl version combinations?

    Also, could I ask what repository/branch you're working from?
    Basically the problem is that

    - the pod directory has both perlpodstyle and perlpodstyle.pod in it (how come?!)
    - the survey() method doesn’t even find perlpodstyle, it just finds perlpodstyle.pod
    - the find() method finds both and it finds perlpodstyle first
    - so the test failed

    With this understanding I reproduced the failing test.

    The problem comes down to two lines of code. In _make_search_callback(), which is invoked by survey(), we have:

    unless( m/^[-_a-zA-Z0-9]+\.(?:pod|pm|plx?)\z/is ) {

    So it’s only considering files that end in .pod, .pm, .pl, or .plx

    But in find() we have:

    foreach my $ext ('', '.pod', '.pm', '.pl') { # possible extensions

    As you can see, it first checks for no extension. Also note that it’s not checking for the ‘.plx’ extension, which survey handles. I’ve never come across anyone using the .plx extensions, but I guess for a while maybe people did:

    http://www.perlmonks.org/?node_id=336713 <http://www.perlmonks.org/?node_id=336713>

    I think that the best fix is to make the survey() pattern match files with no extensions, and also files with the .plx extension.
    If I read your analysis correctly, survey() -- or, more precisely
    _make_search_callback() -- already handles '.plx'. It's find() that
    fails to handle '.pl'.

    If that's correct, then why not make both functions handle exactly the
    same set of file extensions? Whether that should be specified
    explicitly or by regex -- I have no preference. But we could then
    abstract out the formula for scanning extensions into a function in a
    single location.

    Note: untested.

    Thank you very much.
    Jim Keenan
  • Neil Bowers at Mar 6, 2016 at 10:06 am

    There are two CPAN Testers fails:
    http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee
    http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a
    Do you have any thoughts on why these occurred on these particular OS/Perl version combinations?
    Basically the problem is that
    - the pod directory has both perlpodstyle and perlpodstyle.pod in it (how come?!)
    Is it possible that the testers have files left over from previous test runs?
    Not the foggiest. That was one of the reasons I included cpan-workers on this, because I’m curious how this came about.

    I’ve emailed the two people who produced these two fails, asking them to look in the relevant directory and check my theory (just because I can reproduce the error, doesn’t mean that I’ve reproduced how they produced the error. Though it seems likely I have).

    Somehow when installing perl, they ended up with “perlpodstyle” as well as “perlpodstyle.pod” in their pod directory. And given what the test does, it looks like it was only for that one pod file. One on MacOSX and one on Windows, and different versions of perl.

    Odd.
    Also, could I ask what repository/branch you're working from?
    I forked Marc’s repo for Pod-Simple, since `corelist --upstream Pod::Simple` says it’s upstream cpan.
    If I read your analysis correctly, survey() -- or, more precisely _make_search_callback() -- already handles '.plx'. It's find() that fails to handle '.pl’.
    Yup, I miswrote in my summary.
    If that's correct, then why not make both functions handle exactly the same set of file extensions? Whether that should be specified explicitly or by regex -- I have no preference. But we could then abstract out the formula for scanning extensions into a function in a single location.
    I’ve been resisting the urge to refactor, and just make minimal changes, but you’re right, in this case I should do a teeny bit more.

    Cheers,
    Neil
  • James E Keenan at Mar 6, 2016 at 2:30 am

    On 03/05/2016 03:56 PM, Neil Bowers wrote:
    Hi Marc, & CPAN Workers,

    I’ve been looking into the final two CPAN Testers fails, and have finally got to the bottom of them.
    The failing test is search50.t, and the problem is where it does the following:

    - call survey() to get hash of name => path
    - foreach name, then call find() and check it returns the same path

    There are two CPAN Testers fails:

    http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee <http://www.cpantesters.org/cpan/report/4ddcddb1-6c58-1014-bec3-a1032b7077ee>
    http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a <http://www.cpantesters.org/cpan/report/39970866-dd9c-11e5-a3ee-89603848fe5a>

    Basically the problem is that

    - the pod directory has both perlpodstyle and perlpodstyle.pod in it (how come?!)
    Is it possible that the testers have files left over from previous test
    runs?

    jimk
  • Sawyer X at Mar 6, 2016 at 9:50 am

    On Sat, Mar 5, 2016 at 9:56 PM, Neil Bowers wrote:
    [...]
    As you can see, it first checks for no extension. Also note that it’s not
    checking for the ‘.plx’ extension, which survey handles. I’ve never come
    across anyone using the .plx extensions, but I guess for a while maybe
    people did:

    http://www.perlmonks.org/?node_id=336713
    This comment seems to clarify the .plx (from the thread):

         Actually .plx is an extension to use ActivePerl and IIS with ISAPI.
  • David E. Wheeler at Mar 7, 2016 at 6:12 pm

    There is also this one:

    http://www.cpantesters.org/cpan/report/fa4758fa-db9d-11e5-88e3-a488a082370d

    # Failed test ' find('perldoc') should match survey's name2where{perldoc}'
    # at t/search50.t line 76.
    # got: '/home/njh/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/perldoc.pod'
    # expected: '/home/njh/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/Pod/perldoc.pod'

    Note that the files are in different directories!

    I have fought the issue of divergence between survey() and find() for a couple years now, and while the number of failures has come down over that time (mainly by making them search the same directories in the same order, prefer the same file extension precedence, and doing case-insensitive comparisons on case-insensitive file systems), it’s been a rather drawn-out game of whack-a-mole.
    I think that the best fix is to make the survey() pattern match files with no extensions, and also files with the .plx extension.
    Yes, these should be made consistent, which ought to address the failures you saw, but not the one where it finds the files in different directories. I think that’s an issue specific to perldoc, actually. At one time I considered putting a special case pass for that case in the test file, though I never couldn’t bring myself to do it. (Note that there is already a special-case handling of strict.)
    This means that survey will scan a number of extra files, but it also scans them looking for pod, so it shouldn’t return any false positive. This would mean that survey would now find scripts that have pod in them, which it currently misses.
    Makes sense.

    D

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupcpan-workers @
categoriesperl
postedMar 5, '16 at 8:57p
activeMar 7, '16 at 6:12p
posts6
users4
websitecpan.org

People

Translate

site design / logo © 2018 Grokbase