PhpUnitExpectationFixer - update for Phpunit 8.4 #4908

ktomk · 2020-04-10T15:26:32Z

With Phpunit 8.4 the method expectExceptionMessageRegExp has been
deprecated and it's replacement is the new expectExceptionMessageMatches.

When running the migration to target a more current Phpunit version, the
php_unit_expectation fixer previously did choose the deprecated (old)
method.

Improvement here is to map to the non-deprecated method for Phpunit 8.4+.

NOTE: The old method is removed in Phpunit 9 (not 10!)

Additionally the PhpUnitExpectationFixer fixer is extended replacing the deprecated method expectExceptionMessageRegExp with expectExceptionMessageMatches.

Ref: sebastianbergmann/phpunit@d1199cb

@ExpectedException

a couple of phpunit assertions are deprecated in phpunit 8 and the test-suite was very mixed for exceptions (annotations, set... and expect...). heavy lifting of with PHP-CS-Fixer: // assertInternalType(<type> >>> assertIs<type>( 'php_unit_dedicate_assert_internal_type' => true, // @ExpectedException... >>> ->setExpectedException(... 'php_unit_no_expectation_annotation' => ['use_class_const' => false], // ->setExpectedException...( >>> ->expectException...( 'php_unit_expectation' => true, via scripts/csfix.sh: usage examples: $ PHP_CS_FIXER_BIN=../PHP-CS-Fixer/php-cs-fixer unbuffer lib/scripts/csfix.sh \ --dry-run --diff -- tests/unit tests/integration | less -R $ PHP_CS_FIXER_BIN=../PHP-CS-Fixer/php-cs-fixer lib/scripts/csfix.sh \ -- tests/unit tests/integration some places aren't caught thought and some aren't "fixed", so here are some details: - YamlTester does not extend a test-case, this is why PHP-CS-Fixer can not detect it (?). - assertContains for string to assertStringContainsString as PHP-CS-Fixer has no fixer for it (yet as phpunit 8, little use here) - move away from setExpectedException to expectException, using a patched PHP-CS-Fixer (PHP-CS-Fixer/PHP-CS-Fixer#4908)

@ExpectedException

a couple of phpunit assertions are deprecated in phpunit 8 and the test-suite was very mixed for exceptions (annotations, set... and expect...). heavy lifting of with PHP-CS-Fixer: // assertInternalType(<type> >>> assertIs<type>( 'php_unit_dedicate_assert_internal_type' => true, // @ExpectedException... >>> ->setExpectedException(... 'php_unit_no_expectation_annotation' => ['use_class_const' => false], // ->setExpectedException...( >>> ->expectException...( 'php_unit_expectation' => true, via scripts/csfix.sh: usage examples: $ PHP_CS_FIXER_BIN=../PHP-CS-Fixer/php-cs-fixer unbuffer lib/scripts/csfix.sh \ --dry-run --diff -- tests/unit tests/integration | less -R $ PHP_CS_FIXER_BIN=../PHP-CS-Fixer/php-cs-fixer lib/scripts/csfix.sh \ -- tests/unit tests/integration some places aren't caught thought and some aren't "fixed", so here are some details: - YamlTester does not extend a test-case, this is why PHP-CS-Fixer can not detect it (?). - assertContains for string to assertStringContainsString as PHP-CS-Fixer has no fixer for it (yet as phpunit 8, little use here) - move away from setExpectedException to expectException, using a patched PHP-CS-Fixer (PHP-CS-Fixer/PHP-CS-Fixer#4908)

kubawerlos

Can you add @PHPUnit84Migration:risky in RuleSet?

julienfalque · 2020-04-19T08:10:56Z

This is a new feature, can you rebase onto master please?

julienfalque · 2020-04-19T08:17:02Z

NOTE: As the old method is removed in Phpunit 9 (not 10!), an additional
fixer is needed to rename an already fixed method.

Can't we also map expectExceptionMessageRegExp to expectExceptionMessageMatches?

@kubawerlos

ruleset was missing. //cc @kubawerlos Ref: PHP-CS-Fixer#4908

ktomk · 2020-04-19T23:32:11Z

@julienfalque

This is a new feature, can you rebase onto master please?

and:

NOTE: As the old method is removed in Phpunit 9 (not 10!), an additional
fixer is needed to rename an already fixed method.

Does adding a new rule-set in this case count as new feature? Just wondering, as for this PR this is merely a fix to use the fixer for phpunit migration towards a PHP 7.4 target (coming from PHP 7.3 with backwards compat PHP 5.3 baseline in the project).

For the change (not part of this PR and I could also benefit of help with using standard methods in renaming method calls) to upgrade the previous deprecated method to phpunit 8.4 new ones, this would be a different fixer (and I would be happy to base it onto master, no problem with that).

Were you concerned about that part? Never edited RuleSet, so also please bear with me having such kind of questions.

julienfalque · 2020-04-20T06:34:05Z

Does adding a new rule-set in this case count as new feature?

Yes (it changes the public API of the tool), and so does adding a new value to an existing option of a rule.

For the change [...] to upgrade the previous deprecated method to phpunit 8.4 new ones, this would be a different fixer

IMO it would be better to implement it in the same fixer. But I'm not familiar with this one so I don't know whether it's easily feasable.

ktomk · 2020-04-20T08:12:40Z

@julienfalque I considered the fix here an easy catch - until I now changed the RuleSet and the tests fail and I don't understand what the test is testing when it fails. So better keep things smaller for this one and do it right.

If someone could take a look at the failing test for the fixer configuration and could give me some pointers would be great.

julienfalque · 2020-04-20T08:23:47Z

The failures are because:

the new ruleset contains the wrong rule (it should be php_unit_expectation);
the README file is out of date; you need to run ./php-cs-fixer readme > README.rst (after fixing previous issue).

kubawerlos · 2020-04-20T16:08:57Z

@julienfalque @ktomk also some other tests for rule sets are failing.

- Wrong rule used in rule-set - Skip new @PHPUnit84Migration:risky rule in integration tests Ref: PHP-CS-Fixer#4908

@kubawerlos

ruleset was missing. //cc @kubawerlos Ref: PHP-CS-Fixer#4908

- Wrong rule used in rule-set - Skip new @PHPUnit84Migration:risky rule in integration tests Ref: PHP-CS-Fixer#4908

julienfalque · 2020-04-24T08:08:19Z

@ktomk Looking good. Could add or update a test showing that an existing call to expectExceptionMessageRegExp() is replaced with expectExceptionMessageMatches()?

Ongoing with Phpunit 8.4 the expectExceptionMessageRegExp() method is deprecated in favor of the expectExceptionMessageMatches() method. This change set patches the rename of expectExceptionMessageMatches to expectExceptionMessageMatches into the expectation fixer. NOTE: If the original method call in the code has a different argument count than one, the behavior is undefined. Ref: PHP-CS-Fixer#4908

ktomk · 2020-04-24T17:17:49Z

@ktomk Looking good. Could add or update a test showing that an existing call to expectExceptionMessageRegExp() is replaced with expectExceptionMessageMatches()?

Actually this fixer did not come with such a fix which I noted early on, and that I was under the impression to have it a fixer of it's own (like on a base rename method fixer or similar).

However your last comment made me curious so I wrote a test for exactly this and patched the fixer. It's kinda patched in, so this needs review.

Ongoing with Phpunit 8.4 the expectExceptionMessageRegExp() method is deprecated in favor of the expectExceptionMessageMatches() method. This change set patches the rename of expectExceptionMessageMatchesRegExp to expectExceptionMessageMatches into the expectation fixer. NOTE: If the original method call in the code has a different argument count than one, the behavior is undefined. Ref: PHP-CS-Fixer#4908

tests/Fixer/PhpUnit/PhpUnitExpectationFixerTest.php

…etVersion has its set in RuleSet (kubawerlos) This PR was merged into the 2.15 branch. Discussion ---------- DX: add test to ensure each target version in PhpUnitTargetVersion has its set in RuleSet Would have help getting feedback faster in #4908. Commits ------- 968d095 DX: add test to ensure each target version in PhpUnitTargetVersion has its set in RuleSet

Ongoing with Phpunit 8.4 the expectExceptionMessageRegExp() method is deprecated in favor of the expectExceptionMessageMatches() method. This change set patches the rename of expectExceptionMessageMatchesRegExp to expectExceptionMessageMatches into the expectation fixer. NOTE: If the original method call in the code has a different argument count than one, the behavior is undefined. Ref: PHP-CS-Fixer#4908

ktomk · 2020-05-16T07:59:42Z

Hi there, just refreshing again as it starts to rot a bit here. Are there any blockers?

SpacePossum · 2020-05-23T12:02:37Z

Thank you @ktomk.

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from f142eba to ae02f94 Compare April 10, 2020 15:30

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from ae02f94 to ef279df Compare April 11, 2020 21:09

kubawerlos requested changes Apr 18, 2020

View reviewed changes

This was referenced Apr 18, 2020

Add rule to change PhpUnit ExpectedExceptionMessageRegExp #4924

Closed

DX: add test to ensure each target version in PhpUnitTargetVersion has its set in RuleSet #4931

Merged

julienfalque changed the base branch from 2.16 to master April 19, 2020 08:09

julienfalque added the kind/feature label Apr 19, 2020

SpacePossum changed the title ~~Update php_unit_expectation for Phpunit 8.4~~ PhpUnitExpectationFixer - update for Phpunit 8.4 Apr 19, 2020

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from ef279df to ae3ff0d Compare April 19, 2020 22:55

ktomk added a commit to ktomk/PHP-CS-Fixer that referenced this pull request Apr 19, 2020

fixup! Update php_unit_expectation for Phpunit 8.4

2abf80d

ruleset was missing. //cc @kubawerlos Ref: PHP-CS-Fixer#4908

ktomk added a commit to ktomk/PHP-CS-Fixer that referenced this pull request Apr 23, 2020

fixup! Update php_unit_expectation for Phpunit 8.4

4d9d1dc

- Wrong rule used in rule-set - Skip new @PHPUnit84Migration:risky rule in integration tests Ref: PHP-CS-Fixer#4908

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from 32da260 to 4936aaa Compare April 23, 2020 21:40

ktomk added a commit to ktomk/PHP-CS-Fixer that referenced this pull request Apr 23, 2020

fixup! Update php_unit_expectation for Phpunit 8.4

7b9420c

ruleset was missing. //cc @kubawerlos Ref: PHP-CS-Fixer#4908

ktomk added a commit to ktomk/PHP-CS-Fixer that referenced this pull request Apr 23, 2020

fixup! Update php_unit_expectation for Phpunit 8.4

2d9b562

- Wrong rule used in rule-set - Skip new @PHPUnit84Migration:risky rule in integration tests Ref: PHP-CS-Fixer#4908

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from ba65ed1 to 0bab66d Compare April 24, 2020 17:28

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from 0bab66d to debee43 Compare April 24, 2020 17:34

julienfalque reviewed Apr 24, 2020

View reviewed changes

tests/Fixer/PhpUnit/PhpUnitExpectationFixerTest.php Show resolved Hide resolved

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from debee43 to ff08f1f Compare May 7, 2020 17:06

ktomk force-pushed the php_unit_expectation-8.4-deprecations branch from ff08f1f to 65e63b5 Compare May 16, 2020 07:57

kubawerlos approved these changes May 16, 2020

View reviewed changes

julienfalque approved these changes May 16, 2020

View reviewed changes

julienfalque added the RTM Ready To Merge label May 16, 2020

SpacePossum added this to the 2.17.0 milestone May 17, 2020

PhpUnitExpectationFixer - update for Phpunit 8.4

ced5045

SpacePossum force-pushed the php_unit_expectation-8.4-deprecations branch from 65e63b5 to ced5045 Compare May 23, 2020 12:02

SpacePossum merged commit 1418aef into PHP-CS-Fixer:master May 23, 2020

SpacePossum removed the RTM Ready To Merge label May 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PhpUnitExpectationFixer - update for Phpunit 8.4 #4908

PhpUnitExpectationFixer - update for Phpunit 8.4 #4908

ktomk commented Apr 10, 2020 •

edited

Loading

kubawerlos left a comment

julienfalque commented Apr 19, 2020

julienfalque commented Apr 19, 2020

ktomk commented Apr 19, 2020 •

edited

Loading

julienfalque commented Apr 20, 2020

ktomk commented Apr 20, 2020 •

edited

Loading

julienfalque commented Apr 20, 2020

kubawerlos commented Apr 20, 2020

julienfalque commented Apr 24, 2020

ktomk commented Apr 24, 2020

ktomk commented May 16, 2020

SpacePossum commented May 23, 2020

PhpUnitExpectationFixer - update for Phpunit 8.4 #4908

PhpUnitExpectationFixer - update for Phpunit 8.4 #4908

Conversation

ktomk commented Apr 10, 2020 • edited Loading

kubawerlos left a comment

Choose a reason for hiding this comment

julienfalque commented Apr 19, 2020

julienfalque commented Apr 19, 2020

ktomk commented Apr 19, 2020 • edited Loading

julienfalque commented Apr 20, 2020

ktomk commented Apr 20, 2020 • edited Loading

julienfalque commented Apr 20, 2020

kubawerlos commented Apr 20, 2020

julienfalque commented Apr 24, 2020

ktomk commented Apr 24, 2020

ktomk commented May 16, 2020

SpacePossum commented May 23, 2020

ktomk commented Apr 10, 2020 •

edited

Loading

ktomk commented Apr 19, 2020 •

edited

Loading

ktomk commented Apr 20, 2020 •

edited

Loading