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

t/real-export-setup.t - fix broken test in SETUPALT #17

Merged
merged 1 commit into from
Jul 21, 2023
Merged

Conversation

demerphq
Copy link
Contributor

@demerphq demerphq commented Jul 20, 2023

This test calls Test::SubExport::SETUPALT->import(":all") but there is not such method defined, and only works because Perl special cases the handling of a missing import() method to not throw an exception, thus making such calls a no-op. This no-op behavior simulates what would happen if there was a UNIVERSAL::import() method that does nothing.

In 5.39.0 Perl added a UNIVERSAL::import() method which throws an error if it is passed an argument, as the argument will be ignored, and this pattern is usually indicitive of a usage error, especially on case insensitive file systems. This then breaks this test.

This patch could also simply remove the call to import() but that would make the code look a bit odd. Instead we use the import function provided by Exporter, and define an "all" tag with no symbols in it.

See: Perl/perl5#21269

Happy Birthday.

This test calls Test::SubExport::SETUPALT->import(":all") but there is
not such method defined, and only works because Perl special cases the
handling of a missing import() method to not throw an exception, thus
making such calls a no-op. This no-op behavior simulates what would
happen if there was a UNIVERSAL::import() method that does nothing.

In 5.39.0 Perl added a UNIVERSAL::import() method which throws an error
if it is passed an argument, as the argument will be ignored, and this
pattern is usually indicitive of a usage error, especially on case
insensitive file systems. This then breaks this test.

This patch could also simply remove the call to import() but that would
make the code look a bit odd. Instead we use the import function
provided by Exporter, and define an "all" tag with no symbols in it.
@rjbs
Copy link
Owner

rjbs commented Jul 21, 2023

I have finally made the time to look into this. I think I'll apply this patch, along with a big comment. Thanks!

@rjbs rjbs merged commit 15fc335 into rjbs:main Jul 21, 2023
@rjbs
Copy link
Owner

rjbs commented Jul 21, 2023

@demerphq Oh and by the way, thanks for a nice clear commit message!

@rjbs rjbs mentioned this pull request Jul 21, 2023
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

Successfully merging this pull request may close these issues.

2 participants