Subject: TAP test command_fails versus command_fails_like

From: Peter Smith <[email-masked]>
Date: Wed, 12 Feb 2025 16:05:29 +1100

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.

Thoughts?

(make check-world passes).

======
Kind Regards,
Peter Smith.
Fujitsu Australia

---

From: Ashutosh Bapat <[email-masked]>
Date: Wed, 12 Feb 2025 16:26:50 +0530

> 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