Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test::Class::Load::_load might lead to "redefined" errors in case of inheritance and subdis #39

Open
szabgab opened this issue Feb 23, 2021 · 0 comments

Comments

@szabgab
Copy link
Collaborator

szabgab commented Feb 23, 2021

Please take the following description as the doc for my own implementation of Test::Class::Load::_load, it should be good enough to describe the problem anyway:

{@link #import} recursively searches for files within the provided directories and forwards each
found and accepted test file with the base directory provided to {@link #import} used to search
for here to {@code require} that file. The base implementation expects the given directory to be
the base of the package and strips paths of the given file accordingly, so that a package name
relative to the given dir is created and {@code require}d afterwards. Because of this assumption,
the given dir is added to {@code @inc} as well, so that the package is guaranteed to be found.

That doesn't work if inheritance is used with the given file and the tests are part of a larger
package structure, for which the given dir to {@link #import} only is a subdirectory. In such a
case the required file contains {@code use base '...'} statements with package names which can't
be resolved using the given dir, but are resolved using the other entries of {@code @inc}. The
problem is that all referenced base packages are added to {@code %INC} with their complete package
name, relative to some base dir which doesn't follow the approach our base class implements. So if
the base class is in the searched for test dirs as well and should be required, {@code require} is
called with a different package name than was the case during parsing former test files, so that
{@code require} doesn't know that the file has already been required before. Instead, it's required
again, leading to various {@code redefined} errors.

Our current workaround is to always assume the found files relative to the current working dir
instead of the given one. So we don't reimplement the base, but instead simply forward the given
file with some other dir and everything else works as expected.

Some example:

[...]/SomeApp/Tests/SomePkg/SomeTest.pm -> package SomeApp::Tests::SomePkg::SomeTest; use base '[...]::SomePkg';
[...]/SomeApp/Tests/SomePkg.pm -> package SomeApp::Tests::SomePkg; use base '[...]';

{@link #import} is called with {@code SomeApp/Tests}, so that dir is iterated and the above files
are found and both accepted as test files. {@code SomeApp/Tests} is added to {@code @inc} and
{@code SomePkg} required, which leads to another entry like {@code SomePkg.pm => '...'} in {@code %INC},
because {@code SomeApp/Tests} is always stripped fomr the file to require. Same for the other
package and during parsing of {@code SomeTest.pm} the statement {@code use base '...'} with the
full package name is encountered. That full package name is converted to what is used as key in
{@code %INC}, NOT found there and the file required. But because it's the same package already
required before, only using a different path, the compiler issues {@code redefined} warnings.
{@code %INC} contains {@code SomePkg.pm} instead of {@code SomeApp/Tests/SomePkg.pm}, which would
be needed during parsing of {@code SomeTest.pm}.

This problem can not even be fixed by forcing a depth-first order for requireing packages, because
after {@code SomeTest.pm} {@code %INC} would contain {@code SomeApp/Tests/SomePkg.pm} because of
{@code use base '...'} instead of {@code SomePkg.pm}, which is required afterwards. {@code require}
thinks as well that the package hasn't been processed before, requires it and we have the {@code redefined}
warnings again.

Original: https://rt.cpan.org/Ticket/Display.html?id=123151

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant