Skip to content

Conversation

@cosmastech
Copy link
Contributor

@cosmastech cosmastech commented Nov 4, 2025

Building off of #57623

This change has two benefits:

  1. We cache the Application@configurationIsCached() check, so this will benefit anyone running the app to reduce unnecessary repeated file_exists() for the cached config. This has been moved into its own PR: [12.x] Cache the result of configurationIsCached() #57665
  2. Creates a testing trait for caching the configuration for all tests

On our pipeline, I observed the following:

  • Original: 07.52.903
  • WithCachedRoutes: 05:41.584
  • WithCachedRoutes and WithCachedConfig: 04:28.769 🤯

@cosmastech cosmastech marked this pull request as draft November 4, 2025 12:41
@cosmastech cosmastech marked this pull request as ready for review November 5, 2025 22:52
{
$app->instance('config_loaded_from_cache', true); // I'm not sure this is actually needed

LoadConfiguration::setAlwaysUseConfig(static fn () => CachedState::$cachedConfig);
Copy link
Contributor Author

@cosmastech cosmastech Nov 6, 2025

Choose a reason for hiding this comment

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

Is there any concern here about someone storing an object in cache that can be modified between tests? We could serialize/unserialize it on each test. This would cost some CPU cycles for each test, but could insulate us from that problem. Not sure how often people are storing mutable objects in their configurations 🤔

@taylorotwell taylorotwell merged commit cafe1ac into laravel:12.x Nov 7, 2025
66 checks passed
@santigarcor
Copy link
Contributor

@cosmastech is this one compatible with brianium/paratest?

@cosmastech
Copy link
Contributor Author

@cosmastech is this one compatible with brianium/paratest?

Yes, that's how I was testing it.

@santigarcor
Copy link
Contributor

@cosmastech is this one compatible with brianium/paratest?

Yes, that's how I was testing it.

Are you using any specific command? Because in our tests it isn't getting the right connections and throwing this error:

Base table or view not found: 1146 Table 'test_4_test_4.users'

@cosmastech
Copy link
Contributor Author

@cosmastech is this one compatible with brianium/paratest?

Yes, that's how I was testing it.

Are you using any specific command? Because in our tests it isn't getting the right connections and throwing this error:

Base table or view not found: 1146 Table 'test_4_test_4.users'

🤔 and it's working fine without these added?

I was just running it via php artisan test --parallel

@santigarcor
Copy link
Contributor

🤔 and it's working fine without these added?

I was just running it via php artisan test --parallel

With the WithCachedRoutes trait it works fine, but once I add the WithCachedConfig it breaks 🙃 .

This is the command I'm using php artisan test --parallel --processes=4

BTW thank you for this great addition.

@cosmastech
Copy link
Contributor Author

cosmastech commented Nov 13, 2025

Glad you are able to see benefits from this.

I just created a new repo with a few dumb tests in it. I had no problem running the processes in parallel. Would you mind creating a repo that I can test against?

Following the steps outlined here would might make it easier to debug as well #57689 (comment) 🙏

Ooh, I see where the issue is coming from. Can you tell me what other traits you have applied in the test?

@santigarcor
Copy link
Contributor

I think I found the issue too. Initially I added it to the base TestCase, then I removed it and added to to a test class which has DatabaseTransactions and then the others didn't fail except for that class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants