Support doubling readonly classes#5804
Conversation
readonly classesreadonly classes
14c74b5 to
5af0410
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5804 +/- ##
============================================
+ Coverage 90.11% 90.13% +0.01%
- Complexity 6616 6630 +14
============================================
Files 697 697
Lines 20008 20026 +18
============================================
+ Hits 18031 18051 +20
+ Misses 1977 1975 -2 ☔ View full report in Codecov by Sentry. |
|
I was able to implement support for doubling However, this implementation breaks support for cloning test double objects on PHP 8.2 using I cannot be sure, but I assume that cloning test double objects is not commonly done and that this breakage is acceptable. The current state of this PR reflects this. Alternatively, we could delay support for doubling |
…nvocationHandler()
5af0410 to
d660f26
Compare
|
I may have found a solution to the problem explained in #5804 (comment). With the current state of this PR, using Using Using |
…is not supported on PHP 8.2
…mutually exclusive
PHPUnit refactored the double creation to support doubling
readonly classes [1], thus breaking classes where the class
constructor is called with constructor arguments, but calling
itself a class method which is mocked using the `onlyMethods()`
setup option. This leads now to some internal state setup issue
and is reported as `regression` [2] providing steps to reproduce
it.
This change adds `phpunit 11.2.0` as conflict to the monorepo
version to avoid using it in nightly tests for now, until a
proper regression fix has been implemented in phpunit.
Used command(s):
\
cat \
<<< $(jq --indent 1 --tab \
'."conflict" += {"phpunit/phpunit": "11.2.0"}' \
composer.json) > composer.json \
&& Build/Scripts/runTests.sh -s composer -- update --lock
[1] sebastianbergmann/phpunit#5804
[2] sebastianbergmann/phpunit#5857
Resolves: #104007
Releases: main
Change-Id: Ia384ff98a9ea4e318738a48bd9ff8afcc4faff15
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84533
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Reviewed-by: Simon Schaufelberger <simonschaufi+typo3@gmail.com>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Simon Schaufelberger <simonschaufi+typo3@gmail.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
PHPUnit changed the way how double creation is handled
internally and now emits an exception if a manually defined
`mock class name` should be registered. In some places
within the unit tests, manual mock class names have been
used, because the names are needed for cross referencing
at a later point. For this, different approaches have
been used, for example hardcoded MD5 hashes as strings
or creating hashes of static values like `md5('1')`,
and also reused hashes in different places or in
tests using data providers. This now leads to an
exception:
PHPUnit\Framework\MockObject\Generator\NameAlreadyInUseException
The name "b70551b2b2db62b6b15a9bbfcbd50614" is already in use
This change mitigates the issue by using
`StringUtility::getUniqueId('somePrefix')` as is already done
in other places.
As a sideeffect, four phpstan baseline entries can be removed.
Used command(s):
Build/Scripts/runTests.sh -s phpstanGenerateBaseline
[1] sebastianbergmann/phpunit#5804
Resolves: #104005
Releases: main, 12.4
Change-Id: Icc558844275c9ae9f67a0e7c20daa318f5ad6b41
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84530
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Andreas Wolf <andreas.wolf@typo3.org>
Reviewed-by: Andreas Wolf <andreas.wolf@typo3.org>
PHPUnit changed the way how double creation is handled
internally and now emits an exception if a manually defined
`mock class name` should be registered. In some places
within the unit tests, manual mock class names have been
used, because the names are needed for cross referencing
at a later point. For this, different approaches have
been used, for example hardcoded MD5 hashes as strings
or creating hashes of static values like `md5('1')`,
and also reused hashes in different places or in
tests using data providers. This now leads to an
exception:
PHPUnit\Framework\MockObject\Generator\NameAlreadyInUseException
The name "b70551b2b2db62b6b15a9bbfcbd50614" is already in use
This change mitigates the issue by using
`StringUtility::getUniqueId('somePrefix')` as is already done
in other places.
As a sideeffect, four phpstan baseline entries can be removed.
Used command(s):
Build/Scripts/runTests.sh -s phpstanGenerateBaseline
[1] sebastianbergmann/phpunit#5804
Resolves: #104005
Releases: main, 12.4
Change-Id: Icc558844275c9ae9f67a0e7c20daa318f5ad6b41
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84531
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
PHPUnit changed the way how double creation is handled
internally and now emits an exception if a manually defined
`mock class name` should be registered. In some places
within the unit tests, manual mock class names have been
used, because the names are needed for cross referencing
at a later point. For this, different approaches have
been used, for example hardcoded MD5 hashes as strings
or creating hashes of static values like `md5('1')`,
and also reused hashes in different places or in
tests using data providers. This now leads to an
exception:
PHPUnit\Framework\MockObject\Generator\NameAlreadyInUseException
The name "b70551b2b2db62b6b15a9bbfcbd50614" is already in use
This change mitigates the issue by using
`StringUtility::getUniqueId('somePrefix')` as is already done
in other places.
As a sideeffect, four phpstan baseline entries can be removed.
Used command(s):
Build/Scripts/runTests.sh -s phpstanGenerateBaseline
[1] sebastianbergmann/phpunit#5804
Resolves: #104005
Releases: main, 12.4
Change-Id: Icc558844275c9ae9f67a0e7c20daa318f5ad6b41
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84530
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Andreas Wolf <andreas.wolf@typo3.org>
Reviewed-by: Andreas Wolf <andreas.wolf@typo3.org>
PHPUnit changed the way how double creation is handled
internally and now emits an exception if a manually defined
`mock class name` should be registered. In some places
within the unit tests, manual mock class names have been
used, because the names are needed for cross referencing
at a later point. For this, different approaches have
been used, for example hardcoded MD5 hashes as strings
or creating hashes of static values like `md5('1')`,
and also reused hashes in different places or in
tests using data providers. This now leads to an
exception:
PHPUnit\Framework\MockObject\Generator\NameAlreadyInUseException
The name "b70551b2b2db62b6b15a9bbfcbd50614" is already in use
This change mitigates the issue by using
`StringUtility::getUniqueId('somePrefix')` as is already done
in other places.
As a sideeffect, four phpstan baseline entries can be removed.
Used command(s):
Build/Scripts/runTests.sh -s phpstanGenerateBaseline
[1] sebastianbergmann/phpunit#5804
Resolves: #104005
Releases: main, 12.4
Change-Id: Icc558844275c9ae9f67a0e7c20daa318f5ad6b41
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/84531
Tested-by: core-ci <typo3@b13.com>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
readonlypropertiesreadonlyclasses to be doubledcloneis used on the test double for areadonlyclass on PHP 8.2readonlyclasses