-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix tests to run in same process #565
Conversation
@@ -73,14 +72,14 @@ public function admin_enqueue_scripts() { | |||
WPA0_VERSION | |||
); | |||
|
|||
$profile = get_auth0userinfo( $user_id ); | |||
$profile = get_auth0userinfo( $GLOBALS['user_id'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different way to access globals in PHP, works better for testing.
|
||
if ( ! current_user_can( 'edit_users', $user_id ) ) { | ||
if ( ! isset( $GLOBALS['user_id'] ) || ! current_user_can( 'edit_users', $GLOBALS['user_id'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different way to access globals in PHP, works better for testing.
global $user_id; | ||
if ( ! current_user_can( 'edit_users', $user_id ) ) { | ||
|
||
if ( ! isset( $GLOBALS['user_id'] ) || ! current_user_can( 'edit_users', $GLOBALS['user_id'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Different way to access globals in PHP, works better for testing.
@@ -1,12 +1,11 @@ | |||
<?xml version="1.0"?> | |||
<phpunit | |||
bootstrap="tests/bootstrap.php" | |||
backupGlobals="false" | |||
backupGlobals="true" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep the global vars the same between tests
@@ -39,6 +39,8 @@ class TestConstantSettings extends TestCase { | |||
|
|||
/** | |||
* Test that setting a constant will store the constant key. | |||
* | |||
* @runInSeparateProcess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Separate process required here because constants are set (cannot be unset)
@joshcanhelp Why is it better for testing? Shouldn't both methods to access a Global give the same results? |
@cocojoe - They do in practice but while testing, it was causing issues, possibly with how PHPUnit backs them up between tests. |
I wondered if it had to do with that, the one off use in a method is probably clearer than using |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Changes
Changed the test runner to run in the same process, speeding tests up by a lot.
References
PHPUnit annotations
Testing
Changes to test suite only.
Checklist