Add content from HttpKernelUiHelperTrait::drupalGet() to $this->content for use by AssertContentTrait

phpstan was confused. I updated the baseline. I think that's the right solution, but I opened a thread about it for more discussion.

(Obviously, the "better" solution is to actually fix the naughtiness that phpstan is flagging, but that's for another issue, not here).

What about checking if $this->content exists, and if so, populating it? Then, hypothetically, something that wanted drupalGet() but didn’t need all of AssertContentTrait could use 1 trait but not the other? Moot point for 99% of Kernel tests that extend KTB which would use both traits. But then it’s (slightly?) cleaner?

Pushed a commit to implement #8 for consideration. Feel free to revert it if you think this isn't the right approach. But wanted to share what I had in mind in code form (and to have the bot confirm all's well with this approach).

Wow, yeah. I messed up the variable name the first time, but with the follow-up commit, the MR diff is now tiny, and the pipeline is happy. This seems like the best solution to me.

See attached classes. Here's what happens if you try to run each of them:

% php FooClass.php

Fatal error: BarTrait and BazTrait define the same property ($content) in the composition of Foo. However, the definition differs and is considered incompatible. Class was composed in /.../FooClass.php on line 13

% php Foo2Class.php
baz

So it's possible to have 2 traits with the same property, if they're "compatible". Seems a little fragile, though, and I vote for my solution from #10 as more resilient.

My understanding of the request to put this in its own trait started here:

#3390193-42: Add a drupalGet() method to KernelTestBase from @mstrelan:

I'd be interested to see if we can use something like this in Drupal Test Traits, to perform a drupalGet without the browser. So for that it would be nice to have these in a trait, and not necessarily with the words "kernel test" in it. But having the theme stuff in that same trait would not be useful in that regard, but perhaps wouldn't hurt.

It's been a few years since I had a project that was using DTT, so I don't have an easy way to verify if this is indeed possible/desirable.

Anyway, some version of this change is clearly necessary before we get much further converting Functional to Kernel, so we should resolve this soon. Tagging for Framework manager review to hopefully get some eyes on this and decide the correct path forward.

Thanks!
-Derek

I feel like I'm missing something but what's the downside to #15.B? Is it that the property might exist on a class that can't really use it? If so I'm not sure that's enough of a problem to worry about, seems easier to deal with than adding property_exists() checks.

KernelTestBase and AssertContentTrait

Assuming KernelTestBase here was supposed to be another trait.

But I think it's OK in this case if two test traits define the same property, we have more confusing things in the test system than that.

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.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. 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.

Pushed some commits to fix this. Random fails (I believe) in Nightwatch, but otherwise, back to normal.

core/tests/Drupal/KernelTests/KernelTestBase.php currently has use AssertContentTrait;.

What we said we weren't doing was including use AssertContentTrait; in both KernelTestBase and inside HttpKernelUiHelperTrait. I had proposed conditionally setting $this->content if it exists. You proposed adding a duplicate protected $content (which needs to be protected string $content to match AssertContentTrait) in HttpKernelUiHelperTrait, setting that, and relying on the fact that both traits use the same protected member.

However, when you removed AssertContentTrait from KTB, the pipeline failed spectacularly (caught early by phpstan) with 100s of undefined function calls as existing tests using methods from AssertContentTrait couldn't find them:

https://git.drupalcode.org/issue/drupal-3582985/-/pipelines/822313
https://git.drupalcode.org/issue/drupal-3582985/-/jobs/9849448

If you look at the combined changes for this MR, we're no longer touching KernelTestBase at all (which is good). We're just adding the duplicate protected string $content to HttpKernelUiHelperTrait, setting it with the output from drupalGet() and updating some comments.

p.s. Since @catch weighed in at #18 that he preferred duplicate members instead of conditionally setting this value, we don't need a formal framework manager review.

Sorry if #28 was confusing. I just edited it to replace “to” with “instead of”. Hopefully that’s more clear. But yes, #29 matches what @catch said in #18, what I was trying to say in #28, and what the MR currently implements.