Problem/Motivation

In #3252010: Lock PHPUnit to 9.5 we added a dev dependency on Prophecy PhpUnit, we can remove the compatibility check we had for this when we supported both PHPUnit 8 and 9.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#9 3254199-9.patch8.84 KBlongwave
#8 3254199-7.patch9.53 KBlongwave
#5 3254199-5.patch3.76 KBlongwave
#3 3254199-3.patch1.53 KBlongwave
#2 3254199.patch860 byteslongwave

Comments

longwave created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: Remove Prophecy PhpUnit compatibility checjk » Remove Prophecy PhpUnit compatibility check
Status: Active » Needs review
StatusFileSize
new 3254199.patch860 bytes
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 3254199-3.patch1.53 KB

Actually we can remove part of the upgrade command as well.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Needs work

We should probably move the use ProphecyTrait; to test base classes now, or even only use it in tests that want to use Prophecy?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review
StatusFileSize
new 3254199-5.patch3.76 KB

Prophecy is widely used in unit and kernel tests so let's make it available in those base classes, but we only use it in one functional test so I only added it to that specific class.

Status: Needs review » Needs work

The last submitted patch, 5: 3254199-5.patch, failed testing. View results

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

Fixed fails from #5, these are tests that extend PHPUnit's TestCase directly.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 3254199-7.patch9.53 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
StatusFileSize
new 3254199-9.patch8.84 KB

Reroll.

Status: Needs review » Needs work

The last submitted patch, 9: 3254199-9.patch, failed testing. View results

mglaman’s picture

mglaman
Primary language English
Location WI, USA
commented 
+++ b/core/tests/Drupal/Tests/UnitTestCase.php
@@ -12,6 +12,7 @@
@@ -27,6 +28,7 @@ abstract class UnitTestCase extends TestCase {

@@ -27,6 +28,7 @@ abstract class UnitTestCase extends TestCase {
 
   use PhpUnitWarnings;
   use PhpUnitCompatibilityTrait;
+  use ProphecyTrait;
   use ExpectDeprecationTrait;

Will the trait not be added to all test cases? I know I've used prophecy mocks in Kernel tests.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs work » Needs review

It's added to KernelTestBase and UnitTestCase in this patch. I'm not sure I see the usefulness in adding it to all functional tests.

neclimdul’s picture

neclimdul
He/Him
commented
Status: Needs review » Reviewed & tested by the community

Makes sense to me.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Resolved the conflicts in core/tests/Drupal/Tests/Component/DependencyInjection/Dumper/OptimizedPhpArrayDumperTest.php on commit and ran the test locally.

Committed and pushed eaa010f97a to 10.1.x and b1305b5866 to 10.0.x. Thanks!

  • alexpott committed eaa010f on 10.1.x 
    Issue #3254199 by longwave: Remove Prophecy PhpUnit compatibility check

  • alexpott committed b1305b5 on 10.0.x 
    Issue #3254199 by longwave: Remove Prophecy PhpUnit compatibility check...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.