- Issue created by @marcoliver
- Merge request !7445Issue #3440169: When using drupalGet(), provide an associative array for $headers → (Closed) created by marcoliver
- Status changed to Needs review
9 months ago 11:55am 11 April 2024 - 🇩🇪Germany marcoliver Neuss, NRW, Germany
See MR.
Basically just Ctrl-Shift-F'ed Core for
drupalGet\(.*,.*,.*
and then manually combed through the results to find any offending calls. - Status changed to Needs work
9 months ago 12:27pm 11 April 2024 The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇫🇷France dydave
Thanks a lot Marc-Oliver (@marcoliver) for raising this issue and for your code contributions, it's greatly appreciated!
Just a quick comment to confirm the core module Toolbar test:
Drupal\Tests\toolbar\Functional\ToolbarAdminMenuTest::testSubtreesJsonRequest
currently seems to be failing 🔴 (10.3.x/10.4.x), see:
https://git.drupalcode.org/project/drupal/-/blob/10.3.x/core/modules/too...$ ./vendor/bin/phpunit ./web/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php PHPUnit 9.6.19 by Sebastian Bergmann and contributors. Testing Drupal\Tests\toolbar\Functional\ToolbarAdminMenuTest .....E... 9 / 9 (100%) Time: 01:23.484, Memory: 4.00 MB There was 1 error: 1) Drupal\Tests\toolbar\Functional\ToolbarAdminMenuTest::testSubtreesJsonRequest TypeError: Behat\Mink\Session::setRequestHeader(): Argument #1 ($name) must be of type string, int given, called in /var/www/html/web/core/tests/Drupal/Tests/UiHelperTrait.php on line 235 /var/www/html/vendor/behat/mink/src/Session.php:199 /var/www/html/web/core/tests/Drupal/Tests/UiHelperTrait.php:235 /var/www/html/web/core/modules/toolbar/tests/src/Functional/ToolbarAdminMenuTest.php:387 /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729 ERRORS! Tests: 9, Assertions: 207, Errors: 1.
I haven't had the time to look any further than that or really test the changes from the merge request (MR!7445), but we would definitely appreciate some reviews, testing and feedback.
Thanks in advance!
- Status changed to Needs review
7 months ago 1:36pm 29 May 2024 - Status changed to Needs work
7 months ago 2:33pm 29 May 2024 - 🇺🇸United States smustgrave
Have not reviewed but MR should be against 11.x as the latest dev branch.
- 🇫🇷France dydave
@marcoliver, when you get a moment, could you please change the target branch of the merge request to 11.x, as requested at #8 ?
It seems I'm unable to edit the merge request, even with push access to the issue fork.Otherwise if we need to create a new MR, let me know I would be glad to do so.
Thanks! - Merge request !8256Issue #3445896 by mstrelan, mondrake: PHPUnit\Runner\ErrorHandler::__construct... → (Closed) created by marcoliver
- Status changed to Needs review
7 months ago 8:02am 1 June 2024 - 🇩🇪Germany marcoliver Neuss, NRW, Germany
Sure thing! I created a new MR (8257) targeting 11.0.x.
- Status changed to Needs work
7 months ago 1:32pm 3 June 2024 - 🇩🇪Germany marcoliver Neuss, NRW, Germany
marcoliver → changed the visibility of the branch 11.x to hidden.
- Merge request !8277Issue #3440169: Applied header changes to tests again → (Open) created by marcoliver
- Status changed to Needs review
7 months ago 5:55am 4 June 2024 - 🇩🇪Germany marcoliver Neuss, NRW, Germany
Oops, my bad! MR 8277 now targets 11.x
- Status changed to RTBC
7 months ago 2:37pm 4 June 2024 - Status changed to Needs work
6 months ago 11:30am 12 June 2024 - 🇬🇧United Kingdom alexpott 🇪🇺🌍
This is a duplicate of 📌 Add deprecations for update to behat/* dependencies Active - it's fine if we continue in this issue as we've start here and not there. But we need to trigger a deprecation in \Drupal\Tests\UiHelperTrait::drupalGet() when
if (is_int($header_name)) {
.If this issue does not add the deprecation then we cannot prove we've fixed this in core.
- First commit to issue fork.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to hidden.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to active.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to hidden.
pooja_sharma → changed the visibility of the branch 11.x to active.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to active.
pooja_sharma → changed the visibility of the branch 11.x to hidden.
Addressed the mentioned changes, fixed the test failures case as well
Please review, moved NR
- Status changed to Needs review
6 months ago 5:07pm 17 June 2024 - Status changed to Needs work
6 months ago 5:45pm 17 June 2024 - 🇺🇸United States smustgrave
Left some comments about the link. Not sure if the intention is to point to https://www.drupal.org/node/3408184 → but currently it's pointing to a drupal issues vs CR.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to hidden.
- Status changed to Needs review
6 months ago 6:35pm 17 June 2024 pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to active.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to hidden.
pooja_sharma → changed the visibility of the branch 11.x to active.
pooja_sharma → changed the visibility of the branch 3440169-when-using-drupalget-11.x to active.
- Status changed to Needs work
6 months ago 7:17pm 17 June 2024 - 🇺🇸United States smustgrave
Comment #31 was a suggestion/question. But if that is the CR it needs to be updated as the CR doesn't match what is being done in the code. Either that's suppose to be the one used or a new one needs to be written.
@alexpott, I have reused change request https://www.drupal.org/node/3408184 →
can you please confirm is this correct or needs to be written new one?
Added change record for respective deprecations & updated change record nid in MR.
Rebased MR, Please review , moved NR
- Status changed to Needs review
6 months ago 12:36pm 21 June 2024 - Status changed to Needs work
6 months ago 3:33pm 23 June 2024 - 🇺🇸United States smustgrave
Would need to deprecate in 10.4 so this can be backported too.
- Status changed to Needs review
6 months ago 5:12pm 23 June 2024 Updated deprecate message version in 10.4
Please review, moved NR
- Status changed to RTBC
6 months ago 5:37pm 23 June 2024 - 🇬🇧United Kingdom alexpott 🇪🇺🌍
Committed ebe942c and pushed to 11.x. Thanks!
Committed and pushed 519cafc4ab to 11.0.x and 72ea6d707b to 10.4.x and 5ac4362491 to 10.3.x. Thanks!I've moved the deprecation to 11.1.0 (less disruptive) but backported the changes to 10.3.x as a test-only fix to keep everything aligned and make for easier backports.
diff --git a/core/tests/Drupal/Tests/UiHelperTrait.php b/core/tests/Drupal/Tests/UiHelperTrait.php index f3d9f6c164..a1ea81aec3 100644 --- a/core/tests/Drupal/Tests/UiHelperTrait.php +++ b/core/tests/Drupal/Tests/UiHelperTrait.php @@ -245,11 +245,11 @@ protected function drupalGet($path, array $options = [], array $headers = []) { $this->prepareRequest(); foreach ($headers as $header_name => $header_value) { if (is_int($header_name)) { - @trigger_error('Passing an integer as header name to ' . __METHOD__ . '() is deprecated in drupal:10.4.0 and will be removed from drupal:12.0.0. Update the calling code to pass the header name as a key. See https://www.drupal.org/node/3456178', E_USER_DEPRECATED); + @trigger_error('Passing an integer as header name to ' . __METHOD__ . '() is deprecated in drupal:11.1.0 and will be removed from drupal:12.0.0. Update the calling code to pass the header name as a key. See https://www.drupal.org/node/3456178', E_USER_DEPRECATED); [$header_name, $header_value] = explode(':', $header_value); } if (is_null($header_value)) { - @trigger_error('Using null as a header value to ' . __METHOD__ . '() is deprecated in drupal:10.4.0 and will be removed from drupal:12.0.0. Use an empty string instead. See https://www.drupal.org/node/3456233', E_USER_DEPRECATED); + @trigger_error('Using null as a header value to ' . __METHOD__ . '() is deprecated in drupal:11.1.0 and will be removed from drupal:12.0.0. Use an empty string instead. See https://www.drupal.org/node/3456233', E_USER_DEPRECATED); $header_value = ''; } $session->setRequestHeader($header_name, $header_value);
-
alexpott →
committed 5ac43624 on 10.3.x
Issue #3440169 by pooja_sharma, marcoliver, smustgrave, DYdave, alexpott...
-
alexpott →
committed 5ac43624 on 10.3.x
-
alexpott →
committed 72ea6d70 on 10.4.x
Issue #3440169 by pooja_sharma, marcoliver, smustgrave, DYdave, alexpott...
-
alexpott →
committed 72ea6d70 on 10.4.x
-
alexpott →
committed 519cafc4 on 11.0.x
Issue #3440169 by pooja_sharma, marcoliver, smustgrave, DYdave, alexpott...
-
alexpott →
committed 519cafc4 on 11.0.x
- Status changed to Fixed
6 months ago 8:33am 2 July 2024 -
alexpott →
committed ebe942c7 on 11.x
Issue #3440169 by pooja_sharma, marcoliver, smustgrave, DYdave, alexpott...
-
alexpott →
committed ebe942c7 on 11.x
Automatically closed - issue fixed for 2 weeks with no activity.