Skip to content
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

PHP 7.4 + PHPUnit 8.2 support #32844

Closed
23 tasks done
nicolas-grekas opened this issue Jul 31, 2019 · 3 comments
Closed
23 tasks done

PHP 7.4 + PHPUnit 8.2 support #32844

nicolas-grekas opened this issue Jul 31, 2019 · 3 comments
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them.
Projects

Comments

@nicolas-grekas
Copy link
Member

nicolas-grekas commented Jul 31, 2019

In #32842, I implemented a new ForwardCompatTestTrait in the phpunit-bridge to allow tests to run on the new php74-snapshot job, see #32289 This trait adds the void return type that phpunit 8 requires, while preserving compatibility with branch 3.4, which supports PHP 5.5. As a reminder, we target to make 3.4 run on PHP 7.4.

As of #32882, the phpunit-bridge is able to run phpunit 4.8 to 8 and PHP 5.5 to 7.4. It does so by polyfilling some missing methods and by patching the return type of setUp*/tearDown* methods when needed.

Now, there are failures we need to overcome on PHP 7.4 + phpunit 8, as reported here:
https://travis-ci.org/symfony/symfony/jobs/567908746

We need to implement polyfill methods in ForwardCompatTestTrait to allow fixing the deprecation warnings that phpunit 8 displays.

With #32887 in place, the process would be:

  1. open a PR against branch 4.4 to make the phpunit-bridge polyfill a missing feature when needed
  2. open a PR against branch 3.4 with the fix applied, and with the SYMFONY_PHPUNIT_BRIDGE_PR set to the number of the previous PR in .travis.yaml
  3. we'll merge 1. once things are validated, and you'll have to revert the change on .travis.yaml before we merge 2.

Here is the list of things that phpunit 8 reports and should be fixed:

And fix PHP 7.4 deprecations:

Then:

I'm opening this issue to ask for help to the community, this is a lot of work that can be split efficiently I believe.

@nicolas-grekas nicolas-grekas added this to Nicolas' in Help Wanted Aug 1, 2019
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
…usse)

This PR was merged into the 3.4 branch.

Discussion
----------

[PhpUnitBridge] Fix deprecation assertInternalType

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR fixes PhpUnit deprecation :
> assertInternalType() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertIsArray(), assertIsBool(), assertIsFloat(), assertIsInt(), assertIsNumeric(), assertIsObject(), assertIsResource(), assertIsString(), assertIsScalar(), assertIsCallable(), or assertIsIterable() instead

- update all tests to use `assertIsX` instead of `assertInternalType('x'`
- adds methods `assertIsX` in `ForwardCompatTestTraitForV5`

Commits
-------

4c84424 Fix assertInternalType deprecation in phpunit 9
@jderusse
Copy link
Member

jderusse commented Aug 1, 2019

just posting a comment here in order to avoid that there are two of us working on the same thing: I'm taking the deprecation @expectedException*

@luispabon
Copy link
Contributor

I'm doing the void return declarations on setUp* and tearDown*

nicolas-grekas added a commit that referenced this issue Aug 1, 2019
…rDownAfterClass methods in tests are compatible with phpunit 8.2 (luispabon)

This PR was squashed before being merged into the 4.3 branch (closes #32854).

Discussion
----------

Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | n/a

Please bear with me as this is my first pr to symfony. Branched off 4.3 as requested here: #32844

Tests currently do not pass due to:

```
PHP Fatal error:  Declaration of Symfony\Bridge\Monolog\Logger::reset() must be compatible with Monolog\Logger::reset(): void in /home/luis/Projects/symfony/src/Symfony/Bridge/Monolog/Logger.php on line 23
```

Commits
-------

97bcb5d Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2
symfony-splitter pushed a commit to symfony/http-foundation that referenced this issue Aug 1, 2019
…rDownAfterClass methods in tests are compatible with phpunit 8.2 (luispabon)

This PR was squashed before being merged into the 4.3 branch (closes #32854).

Discussion
----------

Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | symfony/symfony#32844
| License       | MIT
| Doc PR        | n/a

Please bear with me as this is my first pr to symfony. Branched off 4.3 as requested here: symfony/symfony#32844

Tests currently do not pass due to:

```
PHP Fatal error:  Declaration of Symfony\Bridge\Monolog\Logger::reset() must be compatible with Monolog\Logger::reset(): void in /home/luis/Projects/symfony/src/Symfony/Bridge/Monolog/Logger.php on line 23
```

Commits
-------

97bcb5da50 Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2
symfony-splitter pushed a commit to symfony/framework-bundle that referenced this issue Aug 1, 2019
…rDownAfterClass methods in tests are compatible with phpunit 8.2 (luispabon)

This PR was squashed before being merged into the 4.3 branch (closes #32854).

Discussion
----------

Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | symfony/symfony#32844
| License       | MIT
| Doc PR        | n/a

Please bear with me as this is my first pr to symfony. Branched off 4.3 as requested here: symfony/symfony#32844

Tests currently do not pass due to:

```
PHP Fatal error:  Declaration of Symfony\Bridge\Monolog\Logger::reset() must be compatible with Monolog\Logger::reset(): void in /home/luis/Projects/symfony/src/Symfony/Bridge/Monolog/Logger.php on line 23
```

Commits
-------

97bcb5da50 Ensure signatures for setUp|tearDown|setUpAfterClass|tearDownAfterClass methods in tests are compatible with phpunit 8.2
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
… (jderusse)

This PR was merged into the 4.3 branch.

Discussion
----------

[PhpUnitBridge] Fix deprecation assertInternalType - 4.3

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR fixes PhpUnit deprecation :
> assertInternalType() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertIsArray(), assertIsBool(), assertIsFloat(), assertIsInt(), assertIsNumeric(), assertIsObject(), assertIsResource(), assertIsString(), assertIsScalar(), assertIsCallable(), or assertIsIterable() instead

follow #32846 for 4.3 branch

Commits
-------

aa58789 Fix assertInternalType deprecation in phpunit 9
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
… (jderusse)

This PR was merged into the 4.4 branch.

Discussion
----------

[PhpUnitBridge] Fix deprecation assertInternalType - 4.4

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR fixes PhpUnit deprecation :
> assertInternalType() is deprecated and will be removed in PHPUnit 9. Refactor your test to use assertIsArray(), assertIsBool(), assertIsFloat(), assertIsInt(), assertIsNumeric(), assertIsObject(), assertIsResource(), assertIsString(), assertIsScalar(), assertIsCallable(), or assertIsIterable() instead

follow #32846 for 4.4 branch

Commits
-------

51ba665 Fix assertInternalType deprecation in phpunit 9
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
…g parent classes (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

Skip tests that fatal-error on PHP 7.4 because of missing parent classes

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | -

See #32395 and  https://bugs.php.net/78351 for more background.

This skips the affected tests with a warning, which means tests won't pass so we won't forget about them.

Commits
-------

c2c7ba8 Skip tests that fatal-error on PHP 7.4 because of missing parent classes
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Add polyfill for TestCase::createMock()

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | -

Commits
-------

abcd45a Add polyfill for TestCase::createMock()
@nicolas-grekas nicolas-grekas added the Help wanted Issues and PRs which are looking for volunteers to complete them. label Aug 1, 2019
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
…s (jderusse)

This PR was merged into the 5.0-dev branch.

Discussion
----------

[PhpUnitBridge] Add polyfill for expectException* methods

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

this PR provides a polyfill for methods `setException`, `setExceptionMessage`, `setExceptionMessageRegExp` and `setExceptionCode`

Commits
-------

c7a8ce5 [Bridge/PhpUnit] Add polyfill for expectException*
nicolas-grekas added a commit that referenced this issue Aug 1, 2019
…usse)

This PR was merged into the 3.4 branch.

Discussion
----------

Replace calls to setExpectedException by Pollyfill

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | need #32869 to be merged
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

PhpUnit method `setExceptionException` has been deprecated seens 5.7. This PR replace theme by `exceptException` provide by the pollyfill.

Commits
-------

41c02d7 Replace calls to setExpectedException by Pollyfill
nicolas-grekas added a commit that referenced this issue Aug 2, 2019
…erusse)

This PR was squashed before being merged into the 3.4 branch (closes #32875).

Discussion
----------

[PhpUnitBridge] Remove @ExpectedException annotation

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

this PR replaces the deprecated annotation `@expectedException` by calls to the method `expectException`.

To automate the process, I used rector/rector:
```
./vendor/bin/rector process src/Symfony --set phpunit60 -a .phpunit/phpunit-6.5/vendor/autoload.php
```
Which also replace `PHPUnit_Framework_Error_X` by `\PHPUnit\Framework\Error\X`

Commits
-------

a22a9c4 Fix tests
3a626e8 Fix deprecated phpunit annotation
fabpot added a commit to twigphp/Twig that referenced this issue Aug 2, 2019
…n PHP 7.4 (nicolas-grekas)

This PR was merged into the 1.x branch.

Discussion
----------

Fix using array_key_exists() on objects, it is deprecated in PHP 7.4

Found as part of symfony/symfony#32844

Commits
-------

d0550b7 Fix using array_key_exists() on objects, it is deprecated in PHP 7.4
nicolas-grekas added a commit that referenced this issue Aug 6, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Use assertStringContainsString when needed

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR replaces deprecated assertContains to comparre string by the dedicated method `assertStringContainsString`

Commits
-------

058ef39 Use assertStringContainsString when needed
nicolas-grekas added a commit that referenced this issue Aug 6, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Use assertContainsEquals when needed

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR replaces deprecated paramèter `$checkForObjectIdentity` of methods `assertContains` by the dedicated method `assertContainsEquals`

Commits
-------

f842e59 Use assert assertContainsEquals when needed
nicolas-grekas added a commit that referenced this issue Aug 6, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Use assertEqualsWithDelta when needed

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR replaces deprecated paramèter `$delta` of methods `assertEquals` by the dedicated method `assertEqualsWithDelta`

Commits
-------

3a0a901 Use assertEqualsWithDelta when required
nicolas-grekas added a commit that referenced this issue Aug 7, 2019
This PR was squashed before being merged into the 4.3 branch (closes #32977).

Discussion
----------

Remove deprecated assertContains

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | symfony

This PR replaces `assertContains()` with `assertStringContainsString()`.

Commits
-------

43acda6 Remove deprecated assertContains
nicolas-grekas added a commit that referenced this issue Aug 7, 2019
…erusse)

This PR was squashed before being merged into the 3.4 branch (closes #32992).

Discussion
----------

[ProxyManagerBridge] Polyfill for unmaintained version

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

The current implementation of proxy-manager triggers a PHP 7.4 deprecation `ReflectionType::__toString`, and the patch won't be applied to a version prior to 2.5 (see Ocramius/ProxyManager#484) will older version of proxy-manager (2.1 to 2.4 are also compatible with php 7.4).

This PR fixes the implementation of `ProxiedMethodReturnExpression` for version prior to 2.5

Commits
-------

33f722d [ProxyManagerBridge] Polyfill for unmaintained version
nicolas-grekas added a commit that referenced this issue Aug 7, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Fix tests/code for php 7.4

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

Fix remaining tests and deprecation

Commits
-------

05ec8a0 Fix remaining tests
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

Make HttpClientTestCase compatible with PHPUnit8

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

the abstract class `HttpClientTestCase` may be extends by end user and execute by both PHPUnit 8 and bellow. Adding a return typehint on it will force all users extending it to add it too and would be a BC Break.

Note. I don't know how to trigger a deprecation here and help user to add it.

Commits
-------

55daf15 Fix compatibility with PHPUnit 8
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

Disable typehint patch on PHPUnit

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

This PR removes the `SYMFONY_PHPUNIT_REMOVE_RETURN_TYPEHINT` patch and adds a `: void` typehint on `setup` and `tearDown` methods in order to be compatible with PHPUnit 8

Commits
-------

a5af6c4 Disable phpunit typehint patch on 4.3 branch
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

Fix deprecations on 4.3

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

Fix deprecations in branch 4.3
note: remaining deprecation `assertStringContainsString` will be fixed in #32977

* [ ] fix tests in branch 3.4 in #32981

Commits
-------

8fd16a6 Fix deprecation on 4.3
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 4.4 branch.

Discussion
----------

Fix tests deprecation in 4.4 branch

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

Fix deprecation in 4.4 branch

Commits
-------

0564045 Fix deprecation in 4.4 branche
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

Replace warning by isolated test

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

Failing test introduced in PHP 7.4 (fatal error) were skiped with a warning exception.
This PR un tests is isolated process in order to correctly flag the test without stoping the test suite.

I kept a comment to the original bug in order to easily remove theme

Commits
-------

9c45a8e Replace warning by isolated test
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
…hout return typehint (jderusse)

This PR was merged into the 4.3 branch.

Discussion
----------

[VarDumper] Fix test patern to handle callstack with/without return typehint

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

The TestCase::tearDownAfterClass methods does not always have the same signature which change the output of the reflection. This use another methods for testing

Commits
-------

feaadd1 Fix tst patern to handle callstack with/without return typehint
nicolas-grekas added a commit that referenced this issue Aug 8, 2019
This PR was merged into the 5.0-dev branch.

Discussion
----------

Fix deprecation test on master

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | NA

Fix PHPUnit 8 deprecations in master branch

nb: `master` should have been identical to `4.4` but a part of code have been migrated in #31899

Commits
-------

d2f7c6c Fix deprecation test on master
@nicolas-grekas
Copy link
Member Author

Only #32995 remaining, we can close here. Thank you everyone involved.

nicolas-grekas added a commit that referenced this issue Aug 22, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

[VarExporter] fix support for PHP 7.4

| Q             | A
| ------------- | ---
| Branch?       | 4.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #32844
| License       | MIT
| Doc PR        | -

Commits
-------

1a036dc [VarExporter] fix support for PHP 7.4
hultberg pushed a commit to hultberg/symfony that referenced this issue Sep 17, 2021
…(jderusse)

This PR was merged into the 3.4 branch.

Discussion
----------

[PhpUnitBridge] Remove use of ForwardCompatTrait

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | symfony#32844
| License       | MIT
| Doc PR        | NA

With symfony#32882 the ForwardCompatibilityTrait is injected in TestCase which now act as a true polyfill

Commits
-------

ac6242f Remove use of ForwardCompatTrait
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Ideal for your first contribution! (some Symfony experience may be required) Help wanted Issues and PRs which are looking for volunteers to complete them.
Projects
Help Wanted
Nicolas'
Development

No branches or pull requests

3 participants