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

Improve PHPUnit fixture #54

Merged

Conversation

peter279k
Copy link
Contributor

Changed log

  • According to the PHPUnit doc, it should be protected function setUp() : void and protected function tearDown() : void.

@cxj
Copy link
Contributor

cxj commented Mar 22, 2021

This needs to have a custom config setting for php.ini added to the Travis set up in .travis.yml to enable code coverage checks to work correctly. It nees to set 'xdebug.mode' to 'coverage'. More information on how to do that is here: https://docs.travis-ci.com/user/languages/php/#custom-php-configuration

@peter279k peter279k force-pushed the improve_phpunit_fixtures branch from 850df4b to 39f5f7f Compare March 25, 2021 15:48
@peter279k
Copy link
Contributor Author

This needs to have a custom config setting for php.ini added to the Travis set up in .travis.yml to enable code coverage checks to work correctly. It nees to set 'xdebug.mode' to 'coverage'. More information on how to do that is here: https://docs.travis-ci.com/user/languages/php/#custom-php-configuration

Thanks. It's fixed now :).

@cxj
Copy link
Contributor

cxj commented Mar 25, 2021

This now fails checks by Psalm, due to a Symfony polyfill's usage of the null coalescing assignment operator ??= which was introduced in PHP 7.4. Here is the error message for convenience:

ERROR: ParseError - vendor/symfony/polyfill-mbstring/bootstrap80.php:125:88 - Syntax error, unexpected '=' on line 125 (see https://psalm.dev/173)

    function mb_scrub(?string $string, ?string $encoding = null): string { $encoding ??= mb_internal_encoding(); return mb_convert_encoding((string) $string, $encoding, $encoding); }

@cxj
Copy link
Contributor

cxj commented Mar 25, 2021

I think probably composer.json needs to be updated to use a version of Psalm that is newer than version 4.5.2, where the Psalm project fixed some bugs which prevented it from working with PHP 7.1 and 7.2. I have not tested this idea.

https://github.com/vimeo/psalm/releases/tag/4.5.2

@kevinsmith
Copy link
Contributor

Hey @peter279k, thanks for the contribution! Sorry it took so long to respond. I had to make some other small updates to get the CI builds to start passing again on the 2.x branch first.

The void return types technically aren't required until PHPUnit 8, but it doesn't hurt anything to add them now and get it ready for future compatibility. I'll merge it in.

@kevinsmith kevinsmith merged commit e2573fb into relayphp:2.x May 22, 2021
@peter279k peter279k deleted the improve_phpunit_fixtures branch May 22, 2021 17:13
@peter279k
Copy link
Contributor Author

Hi @kevinsmith, thanks for your reply. I glad you come back to concern this PR.

Thanks for merging this :).

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

Successfully merging this pull request may close these issues.

3 participants