Add processing of {INVOKE_WP_CLI_WITH_PHP_ARGS-} vars to FeatureContext. #4597
Conversation
| @@ -332,11 +332,45 @@ public function getHookDefinitionResources() { | |||
| * Replace {VARIABLE_NAME}. Note that variable names can only contain uppercase letters and underscores (no numbers). | |||
There was a problem hiding this comment.
The comment about variable name requirements is not technically correct anymore.
| public function replace_variables( $str ) { | ||
| $ret = preg_replace_callback( '/\{([A-Z_]+)\}/', array( $this, '_replace_var' ), $str ); | ||
| if ( false !== strpos( $str, '{INVOKE_WP_CLI_WITH_PHP_ARGS-' ) ) { | ||
| $str = $this->_replace_invoke_wp_cli_with_php_args( $str ); |
There was a problem hiding this comment.
I'm not a fan of leading underscores in method names when we can just mark them as private.
Instead of matching the other functions, maybe consider just changing them to get rid of that underscore in all of them.
There was a problem hiding this comment.
Yes, looks funny too - will remove underscore from all...
|
Was thinking about allowing numbers in standard variable names (except at start), as it's a bit of a gotcha? |
|
Yes, adding digits (except as the first character) makes sense, to adhere to POSIX: Environment variable names used by the utilities in the Shell and Utilities volume of IEEE Std 1003.1-2001 consist solely of uppercase letters, digits, and the '_' (underscore) from the characters defined in Portable Character Set and do not begin with a digit. Other characters may be permitted by an implementation; applications shall tolerate the presence of such names. I wonder though whether digits were left out because of a technical reason further down the pipe...? |
Even better!
It's good it's restrictive as it makes special processing such as here possible, but I can't think of any issue that could arise by allowing digits... |
|
Yes, let's go ahead with adding digits then. 👍 |
|
Made that digits change. Also added Re latter there's some changes to steps I needed to make to get behat running on Windows where it wasn't obvious that the behaviour wasn't comprised or slightly different so it'll good to have this to "test the tests", plus could act as documentation/examples. |
2 participants
Related #4245
Adds processing of special variable
{INVOKE_WP_CLI_WITH_PHP_ARGS-}toFeatureContext::replace_variables()so that PHP args can be given to either the wp-cli phar or the shell file, in order to test stuff under varying PHP conditions.Uses it in
aliases.feature,help.featureandruncommand.featureto test running withproc_open()/proc_close()unavailable, and removes corresponding phpunit tests.Before (and still with the remaining phpunit tests) was only testing git not phar invocation. Will be useful in other cases also (eg wp-cli/package-command#27).