On Wed, Feb 12, 2025 at 10:36 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi hackers,
>
> Recently, while writing some new TAP tests my colleague inadvertently
> called the command_fails() subroutine instead of command_fails_like()
> subroutine. Their parameters are almost the same but
> command_fails_like() also takes a pattern for checking in the logs.
> Notice if too many parameters are passed to command_fails they get
> silently ignored.
>
> Because of the mistake, the tests were all bogus because they were no
> longer testing error messages as was the intention. OTOH, because
> command failure was expected those tests still would record a "pass"
> despite the wrong subroutine being used, so there is no evidence that
> anything is wrong.
>
> I wondered if the command_fails() subroutine could have done more to
> protect users from accidentally shooting themselves. My attached patch
> does this by ensuring that no "extra" (unexpected) parameters are
> being passed to command_fails(). It seems more foolproof.
>
We will need to fix many perl functions this way but I think
command_fails and command_fails_like or any other pair of similarly
named functions needs better protection. Looking at
https://stackoverflow.com/questions/19234209/perl-subroutine-arguments,
I feel a construct like below is more readable and crisp.
die "Too many arguments for subroutine" unless @_ <= 1;
Another question is whether command_fails and command_fails_like is
the only pair or there are more which need stricter checks?
This won't eliminate cases where command_like is used instead of
command_like_safe, and vice-versa, where logic is slightly different.
So the question is how far do we go detecting such misuses?
--
Best Wishes,
Ashutosh Bapat
