Problem/Motivation

1) Drupal\Tests\Core\EventSubscriber\PsrResponseSubscriberTest::testConvertsControllerResult
Class "Symfony\Component\HttpKernel\Event\ViewEvent" is declared "final" and cannot be mocked.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#10 interdiff-6-10.txt868 byteshardik_patel_12
#10 3162031-10.patch3.05 KBhardik_patel_12
#6 interdiff_2-6.txt1.77 KBravi.shankar
#6 3162031-6.patch2.84 KBravi.shankar
#2 3162031-2.patch2.47 KBlongwave

Comments

catch created an issue. See original summary.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review
StatusFileSize
new 3162031-2.patch2.47 KB

This also passes for me if I modify ViewEvent to be final in Symfony 4.

catch’s picture

catch
he/him
Primary language English
commented

Very nice

+++ b/core/tests/Drupal/Tests/Core/EventSubscriber/PsrResponseSubscriberTest.php
@@ -84,14 +82,12 @@ public function testDoesNotConvertControllerResult() {
    */
   protected function createEventMock($controller_result) {
-    $event = $this->getMockBuilder('Symfony\Component\HttpKernel\Event\ViewEvent')
-      ->disableOriginalConstructor()

Should we change ::createEventMock() to ::createEvent()? Otherwise looks great.

longwave’s picture

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

Yeah, I did think about renaming the method just after I posted the patch. NW to do that.

ravi.shankar’s picture

ravi.shankar commented
Assigned: Unassigned » ravi.shankar

I'll work on this.

ravi.shankar’s picture

ravi.shankar commented
Assigned: ravi.shankar » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 3162031-6.patch2.84 KB
new interdiff_2-6.txt1.77 KB

Here I have made changes as per comment #3.

catch’s picture

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

#6 looks right to me, thanks!

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/tests/Drupal/Tests/Core/EventSubscriber/PsrResponseSubscriberTest.php
@@ -83,15 +81,13 @@ public function testDoesNotConvertControllerResult() {
    * Sets up an alias event that return $controllerResult.
    *
    * @param mixed $controller_result
    *   The return Object.
    *
    * @return \Symfony\Component\HttpKernel\Event\GetResponseForControllerResultEvent|\PHPUnit\Framework\MockObject\MockObject
    *   A mock object to test.

The docblock needs updating to match, let's remove the word "alias" and change "return" to "returns" in the description, fix the return typehint to the ViewEvent object, and remove the word "mock" from the description here too.

hardik_patel_12’s picture

hardik_patel_12 commented

I think we have to change return type of createEventMock() method also


-   * @return \Symfony\Component\HttpKernel\Event\ViewEvent|\PHPUnit\Framework\MockObject\MockObject
-   *   A mock object to test.
+   * @return \Symfony\Component\HttpKernel\Event\ViewEvent
+   *   A ViewEvent object to test.
     protected function createEventMock($controller_result) {
hardik_patel_12’s picture

hardik_patel_12 commented
Status: Needs work » Needs review
StatusFileSize
new 3162031-10.patch3.05 KB
new interdiff-6-10.txt868 bytes

Updating docblock of createEvent() method , kindly review.

longwave’s picture

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

Looks good, thanks!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Version: 9.1.x-dev » 9.0.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed 67ceb1be5c to 9.1.x and 0bc02e8efd to 9.0.x. Thanks!

Backported to 9.0.x as this is a test-only change.

  • alexpott committed 67ceb1b on 9.1.x 
    Issue #3162031 by ravi.shankar, Hardik_Patel_12, longwave, catch: [...

  • alexpott committed 0bc02e8 on 9.0.x 
    Issue #3162031 by ravi.shankar, Hardik_Patel_12, longwave, catch: [...

Status: Fixed » Closed (fixed)

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