Problem/Motivation

Following on from #3612411: Split a couple of methods out of ManageFieldsFunctionalTest and convert them to kernel tests we can convert all of the base class and its children to kernel tests. This relies on #3566881: Add a submitForm() method to HttpKernelUiHelperTrait and a couple other additions to the HttpKernelTestBrowser introduced in that issue. Namely, resetting static properties for form state and seen IDs. These probably could use a more holistic solution that would also help with long running php processes, but in the meantime we can be pragmatic and reset these in kernel test requests. I've included everything in this issue.

This reduces test time from over 5 minutes to under 1 minute.

Before: 5:06.971
After: 47.739s

This is reduced slightly since some conversions are already done in #3612411: Split a couple of methods out of ManageFieldsFunctionalTest and convert them to kernel tests.

This conversion also removes duplication that was introduced in the above issue, as the original test base class no longer exists.

I've also moved most of Functional/ManageFieldsTest in to Kernel/ManageFieldsTest, except for one test that couldn't be converted. The combined total of the moved tests here dropped from 110.36s to 32.81s, with only minimal changes required to the test methods.

Steps to reproduce

Proposed resolution

Postponed on #3566881: Add a submitForm() method to HttpKernelUiHelperTrait

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3613739

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

mstrelan created an issue. See original summary.

mstrelan’s picture

Status: Active » Needs review

The diff was easier to review before #3612411 was committed because these were mostly detected as file moves with a few minor changes. Probably we should postpone on adding submitForm in a separate issue, but this might make it easier to review that issue.

mstrelan’s picture

Issue summary: View changes

I've also moved most of Functional/ManageFieldsTest in to Kernel/ManageFieldsTest, except for one test that couldn't be converted. The combined total of the moved tests here dropped from 110.36s to 32.81s, with only minimal changes required to the test methods.

needs-review-queue-bot’s picture

Status: Needs review » Needs work
StatusFileSize
new91 bytes

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.

mstrelan’s picture

Status: Needs work » Needs review
Issue tags: +no-needs-review-bot

Need an actual review, no point sending it in to rebase hell

catch’s picture

Didn't do a line by line review, but can't see anything to complain about. The new traits look fine and the actual test changes are minimal which is a good sign.

mstrelan’s picture

@catch I think the main thing to review is the concept of the HttpKernelTestBrowser for handling the quirks.

catch’s picture

That bit I did review and it looks great. We have #1852090: Cached render elements can have duplicate HTML IDs open to remove the static cache from Html ID handling but that issue can remove the workaround here.

mstrelan’s picture

Scoping question: should we postpone this on #3566881: Add a submitForm() method to HttpKernelUiHelperTrait and update that with the latest versions from this MR? Or just handle it all in this issue?

catch’s picture

That issue says it's blocked on #2505339: Stop using getMainRequest() to build $form['#action'], does this also include that or did it turn out not to be an issue?

mstrelan’s picture

The isolateRequestStack part of HttpKernelTestBrowser works around that issue.

catch’s picture

Since that issue also converts a test it's probably better to do the change over here and postpone this one.

mstrelan’s picture

Title: Convert ManageFieldsFunctionalTestBase and sub classes to Kernel tests » [PP-1] Convert ManageFieldsFunctionalTestBase and sub classes to Kernel tests
Issue summary: View changes
Status: Needs review » Postponed

Have updated #3566881: Add a submitForm() method to HttpKernelUiHelperTrait to reflect what has been discussed here. Postponing this until that is in.