Enable displaying PHP errors to stderr if running behat.#4515
Enable displaying PHP errors to stderr if running behat.#4515
Conversation
|
This would now be a use case for an external test suite package, that could be separately versioned. This way, we could add breaking changes by bumping the major version. Different commands could use different versions of the tests. (... and probably break our automated full test suite in the process...? ) |
Is this actually the case though? These files are copied into the bundled commands. |
php/WP_CLI/Process.php
Outdated
| throw new \RuntimeException( $r ); | ||
| } | ||
| // But do it right if testing. | ||
| if ( ! empty( $this->env['BEHAT_RUN'] ) && ! empty( $r->stderr ) ) { |
There was a problem hiding this comment.
Can we perform this check where run_check() is called, instead of modifying the Process class itself?
There was a problem hiding this comment.
It's called in a lot of places - do mean just in invoke_proc() https://github.com/wp-cli/wp-cli/blob/v1.4.1/features/steps/when.php#L7?
There was a problem hiding this comment.
Yes. Given this change is specific to the test suite, it makes more sense to include it in the test suite.
There was a problem hiding this comment.
Hmm, that would make the code a bit yuck, which usually isn't a good sign. Is the BEHAT_RUN check not exclusive enough?
There was a problem hiding this comment.
Hmm, that would make the code a bit yuck, which usually isn't a good sign.
Why?
Is the
BEHAT_RUNcheck not exclusive enough?
It's hacky. It's not the Process class' responsibility to adapt behavior based on some environmental flag. It's Behat that wants to change the behavior of STDERR.
There was a problem hiding this comment.
I'll add it as a new function run_check_stderr() - that should keep us both happy...
Those tests that do an |
But they're only fail after I run |
Sorry, yes (I think!). |
danielbachhuber
left a comment
There was a problem hiding this comment.
👌 Looks good on my end. Feel free to merge if you're happy with it.
|
Cool. How do you want to deal with fixing the test breakages in the bundled commands? If you want to split them out and assign them I'm good for several hours now... |
If you want to go through each package individually and run I don't think you need to create issues though. Feel free to simply start creating PRs with the updated test suite and the fixes. |
|
Okay I'll start going through them alphabetically... |
As of wp-cli#4515, `WP_CLI\Runner\enable_error_reporting()` is called when running the Behat suite, in order to expose errors. That's desirable in most situations, but there are some Behat scenarios which need to test WP_CLI's error handling directly, and `enable_error_reporting()` creates unwanted side-effects in those situations. `WP_CLI_FORCE_BEHAT_ERROR_REPORTING` provides a way for scenarios to disable those side-effects when needed.
As of wp-cli#4515, `WP_CLI\Runner\enable_error_reporting()` is called when running the Behat suite, in order to expose errors. That's desirable in most situations, but there are some Behat scenarios which need to test WP_CLI's error handling directly, and `enable_error_reporting()` creates unwanted side-effects in those situations. `WP_CLI_FORCE_BEHAT_ERROR_REPORTING` provides a way for scenarios to disable those side-effects when needed.
As of wp-cli#4515, `WP_CLI\Runner\enable_error_reporting()` is called when running the Behat suite, in order to expose errors. That's desirable in most situations, but there are some Behat scenarios which need to test WP_CLI's error handling directly, and `enable_error_reporting()` creates unwanted side-effects in those situations. `WP_CLI_FORCE_BEHAT_ERROR_REPORTING` provides a way for scenarios to disable those side-effects when needed.
As of wp-cli#4515, `WP_CLI\Runner\enable_error_reporting()` is called when running the Behat suite, in order to expose errors. That's desirable in most situations, but there are some Behat scenarios which need to test WP_CLI's error handling directly, and `enable_error_reporting()` creates unwanted side-effects in those situations. `WP_CLI_FORCE_BEHAT_ERROR_REPORTING` provides a way for scenarios to disable those side-effects when needed.
As of wp-cli#4515, `WP_CLI\Runner\enable_error_reporting()` is called when running the Behat suite, in order to expose errors. That's desirable in most situations, but there are some Behat scenarios which need to test WP_CLI's error handling directly, and `enable_error_reporting()` creates unwanted side-effects in those situations. `WP_CLI_FORCE_BEHAT_ERROR_REPORTING` provides a way for scenarios to disable those side-effects when needed.
Fixes #4504
Enables displaying PHP errors (except
E_DEPRECATED) to stderr if running behat, first on starting WP-CLI and then after WP loads.Changes
WP_CLI\Processto throw an error ifstderris non-empty if running behat.Changes
FeatureContextto redirect composer messages (which are output to stderr) to stdout, and to redirect warning messages on caching wp core download, and toskip-email, so as not to pollute stderr.Suppresses "Warning: Wildcard DNS may not be configured correctly" message on
Given a WP multisite install, so as not to pollute stderr. Also addsnotoption to theThen the return code shouldstep for situations where there could be OS-dependent return values.Adjusts various tests to allow for these changes, and removes individual stderr handling. Also in
help.featuresimplifies some copy-pasta introduced (by me) in #4285.As noted in #4504, this PR will cause tests for bundled commands to fail until they're adjusted. It will also cause breakage in any external code that uses the behat test suite.