[Testing] Locales extender doesn't work during integration tests

[clarkwinkelmann]

Bug Report

Current Behavior It is not possible to use translations defined by an extension during integration tests.

Steps to Reproduce

1. In extension extend.php, load translations using Flarum\Extend\Locales()
2. In integration tests, load extension with ->extension()
3. In integration tests, retrieve ->app()->getContainer()->make('translator')->trans('a-string-defined-by-extension')
4. The translation key is always returned. Dumping translator we can see the resource property only contains core.yml and validation.yml, and no other file

Expected Behavior Translations should be available during integration tests

Environment

• Flarum version: 1.0.1, dev-master

Possible Solution The problem is in https://github.com/flarum/testing/blob/v1.0.0/src/integration/Extend/OverrideExtensionManagerForTests.php#L31-L36

On line 31 the extension is enabled, which runs the lifecycle extenders. On line 36 the normal extenders run.

The Locales extender works by registering translations in $container->resolving(LocaleManager::class) https://github.com/flarum/core/blob/v1.0.2/src/Extend/Locales.php#L32 but the onEnable lifecycle hook resolves LocaleManager::class too early https://github.com/flarum/core/blob/v1.0.2/src/Extend/Locales.php#L61

This doesn't happen during normal Flarum operation because the onEnable hooks and extenders run in different requests (onEnable at the end of the extension enabling, then the extenders themselves only run on next request).

But during integration tests those run against the same container, which means the resolving callbacks in the Locales extender will never execute.

I'm not really sure how we could solve this properly. We could technically run the lifecycle hooks after the normal extenders, but since each extension is handled one by one, this would still break the extensions that are loaded after it. Also some extensions will expect the lifecycle hooks to have run before any extender is called.

One option could be to not run the lifecycle hooks during integration testing, or somehow make it configurable. Personally it would help for some of my other extensions to not run the hooks because it runs on every test and is really not necessary for the tests. I have not seen many extension use the hooks yet. Generally we use them to clear the cache, which can be skipped during integration tests since it's already running in debug mode.

We could add a special check just for the Locales extender since it appears to be the only one impacted by this issue.

Or lastly, we could switch from $container->resolving() to $container->extend(), which would register the translations even if the translator was already resolved. Changing this in Flarum for normal usage offers no benefit since on the first resolve the filesystem cache for translations is usually created. But in tests since debug mode is on, this might actually be a good solution.

Additional Context At first I thought the Formatting extender would be impacted by this as well but it actually isn't because it uses extend() which is able to still register the new configuration. We only need to manually clear the formatter cache manually once more to be able to use the registered formatting.

This is the workaround I used in my tests. Run this code on ->app()->getContainer() or as a custom extender:

$container->forgetInstance(LocaleManager::class);
// The existing translator instance is re-used, but that's not an issue, the core translations will just be pushed twice to it
$container->make(LocaleManager::class);