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

Command icon 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

joachim created an issue. See original summary.

joachim’s picture

Headers behave interestingly.

Mink's BrowserKitDriver::setRequestHeader() does this:

        $name = str_replace('-', '_', strtoupper($name));

so headers are in ALL_CAPS.

And then when the request comes into the HTTP kernel, symfony's HeaderBag in the request does this:

        $key = strtr($key, self::UPPER, self::LOWER);

The effect is that no matter what case your header name is in, it ends up in all lower case.

joachim’s picture

Status: Active » Needs review
joachim’s picture

Issue summary: View changes
catch’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs change record updates

This 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.

dww’s picture

Status: Reviewed & tested by the community » Needs work

Phpcs and cspell failed

dww’s picture

Status: Needs work » Needs review

Should 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.

dww’s picture

Bot 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

joachim’s picture

Applied suggestions.

joachim’s picture

I'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 :(

mstrelan’s picture

Do we want to add test coverage for passing a Url object?

joachim’s picture

There was already coverage in KernelTestHttpRequestTest, but I've added coverage of a Url object with options.

smustgrave’s picture

Is the only thing left for this CR updates?

joachim’s picture

I'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().

smustgrave’s picture

What about a simple addition to the CR, just to mention that option and header parameters are available like in functional tests?

joachim’s picture

It 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.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record updates

Then yea not sure if more is needed.

99%

   ✘ Install profile config overwrite
       ┐
       ├ Error: Call to a member function label() on null
       │
       │ /builds/core/modules/config/tests/src/Functional/ConfigInstallProfileOverrideTest.php:141

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.

alexpott made their first commit to this issue’s fork.

alexpott’s picture

Version: main » 11.x-dev
Category: Feature request » Task
Status: Reviewed & tested by the community » Fixed

Committed and pushed 255ff3edb62 to main and ac0833cf2e7 to 11.x. Thanks!

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed ac0833cf on 11.x
    task: #3582228 add $options and $headers parameters to...

  • alexpott committed 255ff3ed on main
    task: #3582228 add $options and $headers parameters to...
dww’s picture

Status: Fixed » Patch (to be ported)

Thanks! Moving status for backport to 11.3.x and 10.x per #3390193-160: Add a drupalGet() method to KernelTestBase.