-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
tests: Upgrade unit tests for PHP 7.4 compat #1710
Conversation
499132d
to
5c9486c
Compare
@@ -16,7 +16,7 @@ | |||
"guzzlehttp/psr7": "^1.2" | |||
}, | |||
"require-dev": { | |||
"phpunit/phpunit": "~4.8.36", | |||
"phpunit/phpunit": "^4.8|^5.0", |
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.
"phpunit/phpunit": "^4.8|^5.0", | |
"phpunit/phpunit": "^4.8.36|^5.6", |
We need a PHPUnit version that already has the namespaced TestCase
class alias.
@google-admin Bump! |
@@ -24,7 +24,7 @@ php: | |||
- 7.1 | |||
- 7.2 | |||
- 7.3 | |||
- 7.4snapshot | |||
- 7.4 |
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.
👍
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.
This is amazing!
} | ||
public function testRevokeAccessGuzzle5() | ||
{ | ||
$this->onlyGuzzle5(); |
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.
Why did you fix the spaces here but not in the other tests? Is it because you realized that arduous task would crush your soul?
FWIW I think this is great, and if we want we can have a follow-up PR for just converting 2 spaces to 4 spaces
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.
yes, that's a fairly good description of what happened.
In older versions of PHPUnit, the mocking framework has fairly major compatibility issues with PHP 7.4, and due to (in my opinion) questionable decisions taken by the maintainers to not backport fixes to anyone using less than PHP 7.2, will not be taking action to address this in a way we can make use of.
This change replaces the PHPUnit mocks with prophecy mocks.
Once this PR is tagged by prophecy, we will be able to remove the allow failures tag from PHP 7.4.done.