Problem/Motivation

The TestWaitTerminate middleware is added for all functional tests, but it's only needed by tests that use WaitTerminateTestTrait. This is likely to be a small performance hit on functional and functional js tests.

I think we could move the middleware to a testing module, then enable that module on all the tests that use the trait

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3436971

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
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Title: Move the TestWaitTerminate middlewar to a module » Move the TestWaitTerminate middleware to a module
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Title: Move the TestWaitTerminate middleware to a module » Move the TestWaitTerminate middleware to be added conditionally

From @alexpott:

You could do something like \Drupal\Core\Test\TestSetupTrait::$strictConfigSchema - just write the middleware directly to the services file and be done… and not use state at all.

Berdir made their first commit to this issue’s fork.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I had a look at this, and that makes sense, I'm just not exactly sure how change it to that from where we are. The current trait sets state flag which I guess you can do anytime within the test. We could rebuild the container when the method is called and write the file then, but that seems complicated and requires reading, parsing and writing it again, or writing another file that we include?

That method currently needs to be called after setup obviously, while a new one would need to be called before to avoid extra rebuilds and complexity.

Maybe we should just deprecate the trait entirely in favor of a flag in the test base or a trait used there, like \Drupal\Core\Test\TestSetupTrait::$strictConfigSchema. For BC, we check if the trait is present and then do it to?

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented

Could we set the flag in the same trait, deprecate the method (docs-only deprecation I guess to start with because it'll break tests if we @trigger_error), and use it like that?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented

I think the logic for writing the services.yml file needs to be in \Drupal\Core\Test\FunctionalTestSetupTrait::prepareSettings, that's why my proposal is to deprecate the trait entirely and just have the flag there.