Skip to content

Conversation

@tonycoz
Copy link
Contributor

@tonycoz tonycoz commented Sep 11, 2025

I got sick of reverse engineering run_multiple_progs() tests and runperl() arguments each time I needed to use them.

There's some documentation in comments inline but it's pretty variable and less accessible than pod.

Since (in theory anyway) we want test.pl to exercise as little of perl as possible, the POD doesn't belong in test.pl itself.

So I've put this in t/test_pl.pod since it's not really end user documentation that would belong in pod/.

Suggestions welcome for a better location.

  • This set of changes does not require a perldelta entry.

@jkeenan
Copy link
Contributor

jkeenan commented Sep 11, 2025

This looks very good but I'll have to defer a thorough look until tomorrow.

Copy link
Contributor

@jkeenan jkeenan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a lot to review here. The sections up through Test::More differences look good to me. I suggest getting different people to review the various remaining sections:

204:=head1 Extended test functions
438:=head1 Utility functions
661:=head1 Runperl Arguments
736:=head1 run_multiple_progs() tests

t/test_pl.pod Outdated

=head1 Extended test functions

These test some condition and produce TAP for a test.
Copy link
Contributor

@jkeenan jkeenan Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point in my life, I know what TAP is when I see it ... but I had to stop and think what the acronym stands for. Perhaps a link to someplace that defines it would help.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mostly going for "produces a counted test" but I'm not sure that's better

t/test_pl.pod Outdated

=back

There's also some variables which detect useful configuration:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grammar: should be "There are also ..."

@khwilliamson
Copy link
Contributor

I think we should just merge this. It lgtm, but there is a lot of stuff here that I didn't know about. If something comes up where this isn't quite right, what harm will have been done. We would just fix it. End-users would not be affected.

I got sick of reverse engineering run_multiple_progs() tests and
runperl() arguments each time I needed to use them.

There's some documentation in comments inline but it's pretty variable
and less accessible than pod.

Since (in theory anyway) we want test.pl to exercise as little of perl
as possible, the POD doesn't belong in test.pl itself.

So I've put this in t/test_pl.pod since it's not really end user
documentation that would belong in pod/.
podcheck normally excludes t/ but I do want t/test_pl.pod checked.

Updating is_pod_file() to allow only this file seems more complex than
just adding t/test_pl.pod afterwards, so just add it, just as
t/porting/podcheck.t is added.
@tonycoz
Copy link
Contributor Author

tonycoz commented Sep 15, 2025

added it to podcheck and fixed some issues (C</dev/null> to F</dev/null> and some verbatim overflows.

@khwilliamson
Copy link
Contributor

still lgtm

@tonycoz tonycoz merged commit e96b722 into Perl:blead Sep 17, 2025
33 checks passed
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.

3 participants