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
| Comment | File | Size | Author |
|---|
Issue fork drupal-3613739
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 #3
mstrelan commentedThe 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
submitFormin a separate issue, but this might make it easier to review that issue.Comment #4
mstrelan commentedI'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.
Comment #5
needs-review-queue-bot commentedThe 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.
Comment #6
mstrelan commentedNeed an actual review, no point sending it in to rebase hell
Comment #7
catchDidn'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.
Comment #8
mstrelan commented@catch I think the main thing to review is the concept of the HttpKernelTestBrowser for handling the quirks.
Comment #9
catchThat 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.
Comment #10
mstrelan commentedScoping 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?
Comment #11
catchThat 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?
Comment #12
mstrelan commentedThe isolateRequestStack part of HttpKernelTestBrowser works around that issue.
Comment #13
catchSince that issue also converts a test it's probably better to do the change over here and postpone this one.
Comment #14
mstrelan commentedHave updated #3566881: Add a submitForm() method to HttpKernelUiHelperTrait to reflect what has been discussed here. Postponing this until that is in.