Problem/Motivation

This is still one of the slowest functional tests, there are a couple of methods that are easy enough to split out.

Some of the test methods in this test don't do a form submission, so it might be a candidate to factor out parts into a kernel test once some of the other lower hanging fruit is converted.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3612411

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

catch created an issue. See original summary.

catch’s picture

Status: Active » Needs review
smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

seems like a good split

larowlan’s picture

Status: Reviewed & tested by the community » Needs review

One question on the MR, fine to self RTBC

mstrelan’s picture

Status: Needs review » Needs work

These two methods can go straight to kernel tests, I'll push something up shortly. That will reduce the time from 42.993s -> 8.230s.

mstrelan’s picture

Issue summary: View changes
Status: Needs work » Needs review

Pulled some setup from ManageFieldsFunctionalTestBase and converted to kernel test. We can introduce a base class for related kernel tests if we do more conversions. Feel free to revert if it's too much scope creep.

smustgrave’s picture

Status: Needs review » Reviewed & tested by the community

Not to be decided here but first thought was why not use the trait FieldUiTestTrait but seems like it's only used in Functional tests.
1. Should we be doing a better job highlighting traits are for kernel/functional tests? It wasn't clear at all
2. Should there be one that is shared as more tests get converted to kernel

mstrelan’s picture

Yes, great questions. Initially I was using FieldUiTestTrait, and it was working, but phpstan was complaining because other (unused) methods in the trait calls methods like submitForm() that don't exist for KernelTestBase. I think as more tests are converted we can look at refactoring the trait(s). We could also baseline / ignore the errors instead of duplicating the method if that's preferable.

catch’s picture

One comment on the MR.

On the trait, we could maybe introduce a new trait that's compatible with kernel tests, then include that trait in the existing one that adds the additional methods? Probably worth doing before we convert many more of these but I think the one method copied here could be factored back out in the issue that does that.

mstrelan’s picture

Status: Reviewed & tested by the community » Needs review

If everyone is happy with the scope then lets do the trait split here.

catch’s picture

Title: Split a couple of methods out of ManageFieldsFunctionalTest » Split a couple of methods out of ManageFieldsFunctionalTest and convert them to kernel tests
Status: Needs review » Reviewed & tested by the community

That seems fine and the trait split looks good.

Not really my code any more so I think I can re-RTBC.

  • larowlan committed 6723b54a on 11.x
    task: #3612411 Split a couple of methods out of...

  • larowlan committed b65177d2 on main
    task: #3612411 Split a couple of methods out of...
larowlan’s picture

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

Committed and pushed b65177d2ce9 to main and 6723b54a6a7 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.

mstrelan’s picture

Follow up: #3613739: [PP-1] Convert ManageFieldsFunctionalTestBase and sub classes to Kernel tests

This reduces test time from over 5 minutes to under 1 minute. Relies on some trickery to get submitForm working.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.