On Wed, Feb 12, 2025 at 10:36 AM Peter Smith <smithpb2...@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
