- 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
3 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
3 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
28 days ago 1:36pm 29 May 2024 - Status changed to Needs work
28 days 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
25 days 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
23 days 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
22 days ago 5:55am 4 June 2024 - 🇩🇪Germany marcoliver Neuss, NRW, Germany
Oops, my bad! MR 8277 now targets 11.x
- Status changed to RTBC
22 days ago 2:37pm 4 June 2024 - Status changed to Needs work
14 days 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
9 days ago 5:07pm 17 June 2024 - Status changed to Needs work
9 days 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
9 days 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
9 days 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
5 days ago 12:36pm 21 June 2024 - Status changed to Needs work
3 days 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
3 days ago 5:12pm 23 June 2024 - Status changed to RTBC
3 days ago 5:37pm 23 June 2024