Problem/Motivation
Functional tests have these parameters.
We didn't add them in #3390193: Add a drupalGet() method to KernelTestBase to keep things simple.
They be useful in kernel tests -- several of the tests identified in #3582242: Convert functional tests with suitably simple HTTP requests to kernel tests make use of URI options.
Steps to reproduce
Proposed resolution
Remaining tasks
- Commit 50e653a7ef6402d4df275de098aaab8c3ee5000a in https://git.drupalcode.org/issue/drupal-3390193/-/tree/3390193-add-drupa... could be cherry-picked to get this started.
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3582228
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
joachim commentedHeaders behave interestingly.
Mink's BrowserKitDriver::setRequestHeader() does this:
so headers are in ALL_CAPS.
And then when the request comes into the HTTP kernel, symfony's HeaderBag in the request does this:
The effect is that no matter what case your header name is in, it ends up in all lower case.
Comment #4
joachim commentedComment #5
joachim commentedComment #6
catchThis looks really solid to me and will unblock a lot of conversions.
Tagging for change record updates - we can update the CR we added for the original issue once this is in.
Comment #7
dwwPhpcs and cspell failed
Comment #8
dwwShould be green. Let's see what the bot thinks if anything else is up. Meanwhile, I'll try to review more closely if I have a chance later.
Comment #9
dwwBot is happy, phew.
This is mostly looking good, thanks! Added some nits / questions as MR threads. Nothing worth NW'ing over, but it'd be nice to address them somehow before RTBC (either applying suggestions or saying "thanks @dww, but that's pointless. moving on..." 😂).
Cheers,
-Derek
Comment #10
joachim commentedApplied suggestions.
Comment #11
joachim commentedI've moved common helpers to a new trait. That should make the diff easier to read, as it means that the new Kernel test is a smaller class, and it should be clear that the methods that have moved to UpdateTestAssertionsTrait are exact copies from UpdateTestBase.
Also, when reviewing, it'll make it clearer what changes from Functional -> Kernel if you diff against the FIRST commit of the branch, rather than main. That's because the first commit I made was JUST the file copy of UpdateContribTest.php (commit message 'copy browser test', currently SHA dde55743f25cffc55453fd0b353bedabed98c413). So diffing to that shows you the changes that have to be made for it to run as a Kernel test, and you can see clearly that there's some new setup stuff, and then the actual tests are identical apart from some drupalGet() calls (those are needed because we no longer run the batch UI, so we don't get automatically taken to a refreshed update page).
However -- I can't get gitlab to show me this, so might be a CLI only thing :(
Comment #12
mstrelan commentedDo we want to add test coverage for passing a Url object?
Comment #13
joachim commentedThere was already coverage in KernelTestHttpRequestTest, but I've added coverage of a Url object with options.
Comment #14
smustgrave commentedIs the only thing left for this CR updates?
Comment #15
joachim commentedI'm not sure we even need a CR update? https://www.drupal.org/node/3502609 doesn't need to cover every single parameter to the new drupalGet().
Comment #16
smustgrave commentedWhat about a simple addition to the CR, just to mention that option and header parameters are available like in functional tests?
Comment #17
joachim commentedIt actually already says this (oops!):
> The test trait \Drupal\Tests\HttpKernelUiHelperTrait provides methods drupalGet(), clickLink(), and assertSession() which work the same way as the corresponding methods in the test trait Drupal\Tests\UiHelperTrait for Browser tests.
Comment #18
smustgrave commentedThen yea not sure if more is needed.
99%
Is unrelated to this lol.
There is 1 open thread @dww unless you feel strongly maybe we open a follow up to fix UiHelperTrait and then this instance at the same time?
Going to mark though hope it's okay.
Comment #20
alexpottCommitted and pushed 255ff3edb62 to main and ac0833cf2e7 to 11.x. Thanks!
Comment #24
dwwThanks! Moving status for backport to 11.3.x and 10.x per #3390193-160: Add a drupalGet() method to KernelTestBase.