[Testing] TestCase::extension() causes core extension extenders to run twice, fails tests

[clarkwinkelmann]

Bug Report

Current Behavior Calling $this->extension('kilowhat-formulaire') in my integration tests causes the error RuntimeException: Route tags.index on method GET already exists. This is because when extensions are registered through $this->extension(), Flarum runs all extenders for all extensions again. Since core extensions that might be installed are already marked enabled in the database as part of the setup.php script, they run both in ExtensionServiceProvider and again in OverrideExtensionManagerForTests

Steps to Reproduce

1. Setup community extension integration tests according to https://docs.flarum.org/extend/testing.html
2. Install flarum/tags as a dependency of the community extension
3. Create integration test with $this->extension('community-extension') in the setUp
4. Run composer setup
5. Run composer test

Observe tests erroring with error RuntimeException: Route tags.index on method GET already exists.

Expected Behavior See possible solutions.

Environment

• Flarum version: beta 16

Possible Solution Solution 1: we don't enable any core extension in the database level, so that ExtensionServiceProvider don't try running any extender, and require the developer to specify all extensions including core extensions they need in TestCase::extension()

Solution 2: in OverrideExtensionManagerForTests, don't run the extenders for extensions already enabled in the database. It's not easy because that class actually cannot control that, it just calls ExtensionManager::extend()

A alternative to solution 2 would be to somehow make the change to enabled extensions before ExtensionServiceProvider runs, so that we don't need to run and extender inside OverrideExtensionManagerForTests

Additional Context Encountered with my Formulaire extension that depends on flarum/tags.

[askvortsov1]

I like Solution 1. We should be able to set that to an empty array as an additional setup step, maybe?

[clarkwinkelmann]

The only challenge with solution 1 is that the code responsible for white-listing default extensions is part of our installation script for Flarum itself, separate from the tests setup. It's just called by the tests setup.

I've always thought we needed to make the whitelist of extensions configurable somehow, that would be useful for custom Flarum skeletons that could include other bundled extensions. The same configurable option in the installer could be used by the tests.

[askvortsov1]

The only challenge with solution 1 is that the code responsible for white-listing default extensions is part of our installation script for Flarum itself, separate from the tests setup. It's just called by the tests setup.

I've always thought we needed to make the whitelist of extensions configurable somehow, that would be useful for custom Flarum skeletons that could include other bundled extensions. The same configurable option in the installer could be used by the tests.

We could use the const (https://github.com/flarum/core/blob/2536e495b3cbcb5b60403236188a8cca91386610/src/Install/Steps/EnableBundledExtensions.php#L69-L69) as a default option if no argument is provided (we'd need to add a null-default constructor argument to that step), and otherwise allow passing in a list (we'd need to add a new method to Installation)?

[askvortsov1]

The 2 PRs linked above close this issue.