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

[PhpUnitBridge] PHPUnit 8 incompatibility #30071

Closed
evertharmeling opened this issue Feb 4, 2019 · 22 comments
Closed

[PhpUnitBridge] PHPUnit 8 incompatibility #30071

evertharmeling opened this issue Feb 4, 2019 · 22 comments

Comments

@evertharmeling
Copy link
Contributor

Symfony version(s) affected: 4.2.x (but probably all PHP<7.0 compatible versions too)

Description
Using the symfony/testpack, symfony/phpunit-bridge and setting the phpunit version on 8.0. When creating a test which extends Symfony\Bundle\FrameworkBundle\Test\KernelTestCase it will throw the following error:

PHP Fatal error:  Declaration of Symfony\Bundle\FrameworkBundle\Test\KernelTestCase::tearDown() must be compatible with PHPUnit\Framework\TestCase::tearDown(): void in /projects/phpunit-8/vendor/symfony/framework-bundle/Test/KernelTestCase.php on line 24

How to reproduce

https://github.com/evertharmeling/symfony-phpunit-8

Using the 'reproducer' project:

  • Run composer install in project root.
  • Run vendor/bin/simple-phpunit in project root.

Possible Solution

  • Restricting the phpunit/phpunit version to <8.0

Additional context

Somewhat related #14736. Because this could solve for support on phpunit >=8.0

@alexander-schranz
Copy link
Contributor

Another solution could be using class_alias: see #30084

nicolas-grekas added a commit that referenced this issue Feb 8, 2019
…schranz)

This PR was merged into the 3.4 branch.

Discussion
----------

Fix KernelTestCase compatibility for PhpUnit 8

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

As the PhpUnit 8 Testcase has different return types as PhpUnit 7 there need to be 2 different classes to support both PhpUnit 8 and PhpUnit 7. With a class alias we can then change which version is used based on the PhpUnit Version constant. The fix is a little bit hacky but to support different major versions I see no other way.

Not sure as we can't upgrade symfony/symfony to PhpUnit 8 how we can create a TestCase for this change.

Commits
-------

83a56a0 Fix phpunit 8 compatibility
@evertharmeling
Copy link
Contributor Author

#30124

@xabbuh
Copy link
Member

xabbuh commented Feb 18, 2019

I think we can close here now that the related PRs were merged.

@xabbuh xabbuh closed this as completed Feb 18, 2019
@mente
Copy link

mente commented Feb 19, 2019

Wasn't #30124 merged only into 3.4 branch only? This bug report is about 4.2

@xabbuh
Copy link
Member

xabbuh commented Feb 19, 2019

We regularly merge lower branches up into branches for all still maintained Symfony versions. So the bugfix will also be part of the next 4.2 patch release.

@garak
Copy link
Contributor

garak commented Mar 6, 2019

What about other Symfony tests?
For example, https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Test/FormIntegrationTestCase.php is still missing the void return type, so extending it to test a form is not working.
Doing a quick search (just a grep -r 'function setUp()' vendor/symfony/ | grep Test), I can find 208 occurrences of setUp method, all without void return type.

@nicolas-grekas
Copy link
Member

@garak There should be two distinct approaches. One for Test namespaces, another for the internal Tests ones. Would you mind sending a PR for the first case, following what's been done for KernelTestCase?

@garak
Copy link
Contributor

garak commented Mar 7, 2019

@nicolas-grekas I'd be glad to propose a PR, but I'm afraid I'm a bit lost... I tried to search in related issues and PRs and only found something related to tearDown, nothing about setUp.
Can you point me in the right direction? Thanks

@nicolas-grekas
Copy link
Member

I think the same strategy as https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Test/KernelShutdownOnTearDownTrait.php could work for setUp too. I didn't have a better look so can't be more specific sorry.

@garak
Copy link
Contributor

garak commented Mar 7, 2019

But in this case the content of setUp is not the same, duplicating it with eval looks a bit cumbersome to me. What if a target a PR to 4.2 branch instead?

@nicolas-grekas
Copy link
Member

Adding void on 4.2 would immediately break all CI that don't use phpunit8 yet. No go.

garak added a commit to garak/symfony that referenced this issue Mar 7, 2019
garak added a commit to garak/symfony that referenced this issue Mar 7, 2019
fabpot added a commit that referenced this issue Mar 9, 2019
This PR was merged into the 3.4 branch.

Discussion
----------

compatibility with phpunit8

This basically adds the same phpunit8 compatibility layer added in #30084 (but for other test classes)
See also discussion in #30071

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

Commits
-------

5ef254f compatibility with phpunit8
@StevenCarre
Copy link

StevenCarre commented May 3, 2019

How can I restrict php-unit version to <8.0?

@evertharmeling
Copy link
Contributor Author

@StevenCarre when using PhpUnitBridge with https://github.com/evertharmeling/symfony-phpunit-8/blob/master/phpunit.xml.dist#L14

@StevenCarre
Copy link

thank you. Can you tell me where this file is located please?

@evertharmeling
Copy link
Contributor Author

You can place the xml configuration file where you want, but you have to point to the configuration file when running (simple-)phpunit, like bin/simple-phpunit -c /path/to/phpunit.xml.dist.

See https://phpunit.readthedocs.io/en/8.1/textui.html and https://phpunit.readthedocs.io/en/8.1/configuration.html

@mrtnzagustin
Copy link

mrtnzagustin commented Mar 5, 2020

Hi there, i have a problem when executing ./vendor/bin/simple-phpunit in a docker testing stage container. Each time i make a build, test are slow cause simple-phpunit dowload phpunit. How can i download it with other command to mantain it in cache?

@nicolas-grekas
Copy link
Member

you should run ./vendor/bin/simple-phpunit install just after composer install, when building the image.

@mrtnzagustin
Copy link

mrtnzagustin commented Mar 6, 2020

@nicolas-grekas , thanks. I've installed phpunit with that command. The problem is when i run RUN ./vendor/bin/simple-phpunit after the installation, load of fixtures, etc. The output is:

image

Looks like simple-phpunit is not detecting the installation itself, try to install and fail cause the folder already exists.

@mrtnzagustin
Copy link

In addition, trying setting version in phpunit.xml.dist, and the ./vendor/bin/simple-phpunit install dont use that version. When the test are executed with ./vendor/bin/simple-phpunit, the script install the version in the file.

@mrtnzagustin
Copy link

The problem vanish when i use a single line to copy the entire app and just one composer install:

RUN composer install
COPY [".", "/var/www/"]
RUN ./vendor/bin/simple-phpunit install

# Crea la base de datos y la carga con fixtures
RUN php bin/console doctrine:database:create --env=test --no-debug
RUN php bin/console doctrine:schema:update --env=test --force --no-debug
RUN php bin/console hautelook:fixtures:load --env=test

# Ejecuta testing
RUN ./vendor/bin/simple-phpunit

but error in test execution when i do this:

# Copia solo dependencias y las instala
COPY composer.json composer.json
COPY composer.lock composer.lock
COPY symfony.lock symfony.lock
RUN composer install --prefer-dist --no-scripts --no-autoloader && rm -rf /root/.composer
RUN ./vendor/bin/simple-phpunit install 

# Copia el codigo fuente a donde corresponde dentro del contenedor
COPY [".", "/var/www/"]

# Finish composer
RUN composer dump-autoload --no-scripts --optimize

# Crea la base de datos y la carga con fixtures
RUN php bin/console doctrine:database:create --env=test --no-debug
RUN php bin/console doctrine:schema:update --env=test --force --no-debug
RUN php bin/console hautelook:fixtures:load --env=test

# Ejecuta testing
RUN ./vendor/bin/simple-phpunit

@enriquerene
Copy link

enriquerene commented Oct 28, 2020

I replaced function setUp () by function setUp (): void and worked for me (PHPUnit 8, Synfony 5.1).

@StevenCarre
Copy link

@enriquerene one year later thanks, it worked for me too. Cheers

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants