This is just a small quick fix issue which came to light in

#2976394: Allow Symfony 4.4 to be installed in Drupal 8

Drupal9 will jump to Symfony 4.x -- and when we do a bug in CustomPageExceptionHtmlSubscriberTest
will become prominent.

Drupal\Tests\Core\EventSubscriber\CustomPageExceptionHtmlSubscriberTest::testHandleWithPostRequest
TypeError: Argument 3 passed to Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent::__construct() must be of the type integer, string given, called in /var/www/html/core/tests/Drupal/Tests/Core/EventSubscriber/CustomPageExceptionHtmlSubscriberTest.php on line 141

CommentFileSizeAuthor
#7 interdiff-2997771-4-7.txt5.92 KBmartin107
#7 2997771-7.patch4.57 KBmartin107
#4 interdiff-2997771-2-4.txt4.06 KBmartin107
#4 2997771-4.patch5.72 KBmartin107
#2 2997771-2.patch1.66 KBmartin107
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

martin107 created an issue. See original summary.

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Assigned: martin107 » Unassigned
Issue summary: View changes
Status: Active » Needs review
Issue tags: +Quick fix
FileSize
2997771-2.patch1.66 KB

Here is the patch

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Issue summary: View changes

and becomes a

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Title: CustomPageExceptionHtmlSubscriberTest: Prepare for Drupal9 » GetResponseForExceptionEvent: Prepare for Drupal9
FileSize
2997771-4.patch5.72 KB
interdiff-2997771-2-4.txt4.06 KB

The misconception is threaded through many other test files.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Issue tags: +Symfony 4
longwave’s picture

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

This looks like the right fix, I checked the Symfony docs and the third constructor argument is documented as

The request type the kernel is currently processing; one of HttpKernelInterface::MASTER_REQUEST or HttpKernelInterface::SUB_REQUEST

so passing arbitrary strings here was incorrect.

However:

+++ b/core/modules/serialization/tests/src/Unit/EventSubscriber/DefaultExceptionSubscriberTest.php
@@ -27,7 +28,7 @@ public function testOn4xx() {
+    $event = new GetResponseForExceptionEvent($kernel->reveal(), $request, KernelInterface::MASTER_REQUEST, $e);

The MASTER_REQUEST constant actually belongs to Symfony\Component\HttpKernel\HttpKernelInterface (KernelInterface extends this).

martin107’s picture

martin107 CreditAttribution: martin107 as a volunteer commented
Status: Needs work » Needs review
FileSize
2997771-7.patch4.57 KB
interdiff-2997771-4-7.txt5.92 KB

Thank you for taking the time to pick over my thinking.

Fixed.

longwave’s picture

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

Perfect!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 0ab2dc4 and pushed to 8.7.x. Thanks!

  • catch committed 0ab2dc4 on 8.7.x 
    Issue #2997771 by martin107, longwave: GetResponseForExceptionEvent:...

Status: Fixed » Closed (fixed)

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