-
Notifications
You must be signed in to change notification settings - Fork 154
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
[TASK] Use PHPUnit 9.x with PHP 7.3+ #930
Conversation
1b8269b
to
1eb5f29
Compare
Managed to add Where is the documentation for PHIVE? |
It's built-in: #930 (comment) |
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.
Please let's use the latest PHIVE version and explicitly allow Sebastian's GPG key.
This does not appear to be possible. The Some proper documentation for PHIVE would be really helpful. |
Okay, then it seems we need to keep this workaround for trusting the GPG key. Please let's still use the latest PHIVE version. |
I've added phar-io/phive#296. |
Done. |
I think #906 has introduced some changes that are not compatible with PHPUnit 9. |
PHPUnit's public function __construct()
{
$this->exporter = new Exporter();
} (And create an issue that we should rework this once we're PHPUnit-9-only.) |
This is required to be able to test against PHP 8. However, PHPUnit 9.x requires PHP 7.3, so a different version of PHPUnit is required for different testing environments. The main change here is a step in the GitHub Action to conditionally update PHPUnit via PHIVE for PHP >=7.3. PHIVE does not yet have the ability for conditional installs (see phar-io/phive#295 (comment)) so the script must check the PHP version before running the update. PHIVE has also been added to the tools (self-referencing) as it is not available by default to GitHub Actions. Note: There are warnings from PHPUnit 9.x about use of deprecated `assert` methods (which will be removed in PHPUnit 10.x). However, these don't cause the tests to fail, and the replacement methods are not available in PHPUnit 7.x which is still required to test against PHP 7.1 and 7.2. Part of #925/#926.
4630a27
to
999c536
Compare
I've added a constructor which will conditionally call the parent constructor only if it exists. This should cover all bases, including if some future version of PHPUnit reintroduces a constructor. So I don't think any future rework would be required. I can't help thinking that PHP should provide an implicit default constructor, as C++ does, to avoid this type of issue. |
Only since PHP 7.2 can the access level in a child class be made more restrictive. See [PHP bug #61970](https://bugs.php.net/bug.php?id=61970).
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.
<3
There are some suggestions for how to handle PHIVE in relation to this in the responses to phar-io/phive#296. I don't know if this is something we want to address in the short term; it will become a non-issue when we drop PHP 7.2 support. |
This is required to be able to test against PHP 8. However, PHPUnit 9.x
requires PHP 7.3, so a different version of PHPUnit is required for different
testing environments.
The main change here is a step in the GitHub Action to conditionally update
PHPUnit via PHIVE for PHP >=7.3. PHIVE does not yet have the ability for
conditional installs (see
phar-io/phive#295 (comment)) so the
script must check the PHP version before running the update.
PHIVE has also been added to the tools (self-referencing) as it is not available
by default to GitHub Actions.
Note: There are warnings from PHPUnit 9.x about use of deprecated
assert
methods (which will be removed in PHPUnit 10.x). However, these don't cause the
tests to fail, and the replacement methods are not available in PHPUnit 7.x
which is still required to test against PHP 7.1 and 7.2.
Part of #925/#926.