On 10/31/19 10:02 AM, Andrew Dunstan wrote:

This small patch authored by my colleague Craig Ringer enhances
Testlib's command_fails_like by allowing the passing of extra keyword
type arguments. The keyword initially recognized is 'extra_ipcrun_opts'.
The value for this keyword needs to be an array, and is passed through
to the call to IPC::Run.

Hi Andrew, a few code review comments:

The POD documentation for this function should be updated to include a description of the %kwargs argument list.

Since command_fails_like is patterned on command_like, perhaps you should make this change to both of them, even if you only originally intend to use the new functionality in command_fails_like. I'm not sure of this, though, since I haven't seen any example usage yet.

I'm vaguely bothered by having %kwargs gobble up the remaining function arguments, not because it isn't a perl-ish thing to do, but because none of the other functions in this module do anything similar. The function check_mode_recursive takes an optional $ignore_list array reference as its last argument. Perhaps command_fails_like could follow that pattern by taking an optional $kwargs hash reference.

--
Mark Dilger


Reply via email to