Skip to content

[12.x] Add disableConfigCache method to the WithCachedConfig trait #57790

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: 12.x
Choose a base branch
from
+17 −2
Draft

[12.x] Add disableConfigCache method to the WithCachedConfig trait #57790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Illuminate/Foundation/Testing/CachedState.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@

class CachedState
{
public static array $cachedRoutes;
public static array $cachedConfig;
public static ?array $cachedRoutes = null;
public static ?array $cachedConfig = null;
}
15 changes: 15 additions & 0 deletions src/Illuminate/Foundation/Testing/WithCachedConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,19 @@ protected function markConfigCached(Application $app): void

LoadConfiguration::alwaysUse(static fn () => CachedState::$cachedConfig);
}

/**
* Disable the configuration cache for the current test.
*
* @return $this
*/
protected function disableConfigCache()
{
$this->app->instance('config_loaded_from_cache', false);

CachedState::$cachedConfig = null;
Copy link
Contributor

@cosmastech cosmastech Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you want to remove the current cached config, since the next test which wants a cached config will have to build it.

Copy link
Contributor Author

@SanderMuller SanderMuller Nov 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why, but without this my tests still fail.

Copy link
Contributor

@cosmastech cosmastech Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you ever envision even one test in a class still wanting the original cached config?

While I know it's not pretty, I wonder if you could just override setUpWithCachedConfig() and markConfigCached() for the entire test class, making them no-ops?

Copy link
Contributor Author

@SanderMuller SanderMuller Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you ever envision even one test in a class still wanting the original cached config?

Potentially, though it's possible to isolate these kind of tests.
But I can also imagine more Laravel devs having the need to have some kind of config testing, which conflicts with this caching, but still wanting to by-default cache the config, and having an option to disable it for a single test. What do you think?

Copy link
Contributor

@cosmastech cosmastech Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This technically works, but the devex feels a little unusual, and it's got some footguns in it.

  1. If you don't need the cache, we shouldn't build it at all. We definitely shouldn't clear the memo of cached config, since it's going to delay the tests after it which need to rebuild it. This might be surprising to some devs since calling this method will make at least 1 subsequent test slower as it rebuilds the cache.
  2. Having to call this method in the setUp feels weird, since that means every test in your file is going to be under the same constraints. In which case, just making those methods no-ops is probably easiest (or just not applying the trait to this particular test, which may be a pain since they're children of your root test).

We could add a property check like this, but it doesn't really offer the flexibility to set it on a per-test basis since it happens in TestCase@setUp():

image

You might be better off just adding a method like:

protected function withFreshConfig(): void
{
    \Illuminate\Foundation\Bootstrap\LoadConfiguration::alwaysUse(null);
    $this->app->make(\Illuminate\Foundation\Bootstrap\LoadConfiguration::class)->bootstrap($this->app);
}

With that method, you could call it at the start of each test method, rather than in the test setUp.

Let me know your thoughts.

Copy link
Contributor Author

@SanderMuller SanderMuller Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about the devex, and think the ->withFreshConfig() API feels much better.

The proposed implementation worked for most of my use-cases, however not all. It seems that this doesn't refresh the config of packages (I guess it doesn't merge them in), at least I am getting an exception when running a test where config('request-factories') returns null in the following snippet:

namespace Worksome\RequestFactories;

...

final class RequestFactoriesServiceProvider extends ServiceProvider
{
    public function register(): void
    {
        ...
        $this->app->singleton(Finder::class, fn () => new ConfigBasedFinder(config('request-factories')));
    }

If I call $this->refreshApplication(); after $this->withFreshConfig() then the test passes again (not sure if that info helps)

Copy link
Contributor

@cosmastech cosmastech Nov 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense. Config is merged when the Provider is booted, which would explain why it's not working that way.

LoadConfiguration::alwaysUse(null);

return $this;
}
}