Problem/Motivation

This test installs the entity schema for every entity in entity_test module, and then checks if they need updates, in a lot of methods that otherwise have nothing to do with that module. We can save some time by avoiding entity_test module and also selectively installing all entity schemas only when necessary.

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

Issue fork drupal-3615585

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:

About issue forks

Comments

catch created an issue. See original summary.

catch opened merge request !16617

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

Not sure how much this does, but to really optimize it a lot more we'd need to split entity_test into multiple modules probably so that there's less entity types in the system at any one time or completely remove the dependency on entity_test in this test.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Not sure how to measure but seems to be passing, minus the almost consistent JS failures now.

mstrelan’s picture

mstrelan commented
Status: Reviewed & tested by the community » Needs work

Found some more optimisations.

For reference, here are the test times from one run on my machine:

main: 143.2s
MR (d37bcab4): 82.2s
additional optimisations: 42.8s

catch’s picture

catch
he/him
Primary language English
commented

That sounds worth doing. I was wondering about opening an issue top split entity_test module up so it's closer to a module per entity type or something but that seemed like a lot of work. This might achieve the same thing.

mstrelan’s picture

mstrelan commented
Status: Needs work » Needs review

OK I pushed my changes. I was a little concerned about the words "Multiple Types" in the class name EntityDefinitionUpdateMultipleTypesTest but then found it was only after splitting in #3492944: Split up EntityDefinitionUpdateTest. Are we losing coverage here or is this fine?

Also note a few minor assertion changes. We can roll those functions back if needed.

catch’s picture

catch
he/him
Primary language English
commented

It's only called multiple types because it explicitly relies on a handful of different entity types. The implicit extra entity types I really don't think we are losing anything except wasted time and CPU running the test.