Problem/Motivation

Symfony\Contracts\EventDispatcher\EventDispatcher::dispatch() has an object return type hint from Symfony 5.

To prepare for this, we can return an object when mocking it with prophecy.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#13 interdiff.3162008.11-13.txt1.01 KBmikelutz
#13 3162008-13.drupal.patch2.46 KBmikelutz
#11 3162008_1.patch1.45 KBcatch
#8 interdiff-7-8.txt909 byteshardik_patel_12
#8 3162008-8.patch2.5 KBhardik_patel_12
#7 3162008.patch1.45 KBcatch
#7 3162008-with-return-type-hint.patch2.21 KBcatch
#4 test-only.patch2.17 KBcatch
#3 3162008.patch1.4 KBcatch
#2 3162008.patch1.5 KBcatch

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 3162008.patch1.5 KB

There may be more, but here's a start.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new 3162008.patch1.4 KB

Patch was stacked on the parent issue, this should be standalone.

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new test-only.patch2.17 KB

Not sure whether this will work, but trying to add the return type hint to the current version of the class, to see whether we can flush more out. If not, we should be able to do it via the Symfony 5 update in the parent issue.

The last submitted patch, 3: 3162008.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 4: test-only.patch, failed testing. View results

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new 3162008-with-return-type-hint.patch2.21 KB
new 3162008.patch1.45 KB

OK didn't find any other instances, but did find a missing use statement in the actual patch.

hardik_patel_12’s picture

hardik_patel_12 commented
StatusFileSize
new 3162008-8.patch2.5 KB
new interdiff-7-8.txt909 bytes

Changes needs to be done also in,

core/modules/layout_builder/tests/src/Kernel/LayoutBuilderElementTest.php 

-     $event_dispatcher = $this->prophesize(EventDispatcherInterface::class);
+    $event_dispatcher->dispatch(Argument::any(), Argument::any())->willReturn(new Event());
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

The event dispatcher isn't actually called in LayoutBuilderElementTest, it is only mocked to satisfy the constructor, so I am not sure #8 is required.

catch’s picture

catch
he/him
Primary language English
commented

hmm there's a test failure in SectionComponentTest (which I think we should update for here), but not LayoutBuilderElementTest.

https://dispatcher.drupalci.org/job/drupal_patches/55318/testReport/Layo...

catch’s picture

catch
he/him
Primary language English
commented
StatusFileSize
new 3162008_1.patch1.45 KB

Re-uploading #7.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs review » Needs work

We do need to update sectioncomponenttest here.

mikelutz’s picture

mikelutz
He/Him
Primary language English
Location Michigan, USA
commented
Status: Needs work » Needs review
StatusFileSize
new 3162008-13.drupal.patch2.46 KB
new interdiff.3162008.11-13.txt1.01 KB
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Looks OK to me, this fixes the fails uncovered in #4.

alexpott’s picture

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

Committed 75acede and pushed to 9.1.x. Thanks!

  • alexpott committed 75acede on 9.1.x 
    Issue #3162008 by catch, mikelutz, Hardik_Patel_12, longwave: Prepare...

Status: Fixed » Closed (fixed)

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