KernelTestBase fails to set up FileCache

What happens if you run this test via phpunit itself, which is the recommended way?

Maybe something like this?

I don't think we can really test this. I applied this locally and it fixes the issue for me.

+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -478,6 +479,29 @@ private function getCompiledContainerBuilder(array $modules) {
+  protected function initFileCache() {

Function doc block is missing.

Thanks for the fix. It's RTBC.

Before we add any more KTBTNG's we need to fix https://www.drupal.org/node/2553533 - each new test added increases the chance of random fail due to test id reuse.

#2553533: KernelTestBaseTNG™ is not cleaning up after itself is in and did a once over and this looks mostly ok with me and it does fix it but not sure about this.

+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -479,6 +480,32 @@ private function getCompiledContainerBuilder(array $modules) {
+    FileCacheFactory::setConfiguration($configuration);
+    FileCacheFactory::setPrefix(Settings::getApcuPrefix('file_cache', $this->root));

@@ -494,6 +521,8 @@ private function getCompiledContainerBuilder(array $modules) {
+    $this->initFileCache();
+

Do we want to reset these values each time we get a list of extensions? Currently the 2 places we call it we do need to initialize filecache but I'm not sure that is an appropriate assumption to build into the method.

Do we want to reset these values each time we get a list of extensions? Currently the 2 places we call it we do need to initialize filecache but I'm not sure that is an appropriate assumption to build into the method.

I think yeah we should avoid any sideeffects if possible. Pratically it doesn't matter at the moment because we use process isolation but still ... once we might get rid of it, its better to avoid it.

Well my point was its not a clear side effect of the function. We are in fact introducing a side effect by changing the value.

I noticed this needed a reroll while working on #2556855: Port ViewKernelTestBase to extend from KernelTestBaseTNG™

We should reset in tearDown() ...

Well, then FileCache should get one ...

Well actually I thought about a reset() method which just resets \Drupal\Component\FileCache\FileCache::$cached

Simply this then. Interdiff is from #16

+++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
@@ -495,6 +523,8 @@ private function getCompiledContainerBuilder(array $modules) {
+    $this->initFileCache();
+

Seems like we could move this to setUp() to match the reset in teardown. Not tied to this since dawehner seems to like it where it is but it still makes sense to me. Other then that, if a FileCache maintainer signs of on the new method I think we're good.

1. +++ b/core/lib/Drupal/Component/FileCache/FileCacheInterface.php
   @@ -61,4 +61,9 @@ public function set($filepath, $data);
   +  /**
   +   * Resets the static cache.
   +   */
   +  public static function reset();
   

   IMHO you don't need to add a static function to an interface.

2. +++ b/core/tests/Drupal/KernelTests/KernelTestBase.php
   @@ -495,6 +523,8 @@ private function getCompiledContainerBuilder(array $modules) {
      private function getExtensionsForModules(array $modules) {
   +    $this->initFileCache();
   
   @@ -625,6 +655,9 @@ protected function tearDown() {
   +    // Clean FileCache cache.
   +    FileCache::reset();
   +
   

   I like the idea to move it to setup() itself.

I also replaced string FQNs with ::class usages.

WFM. Thanks for putting up with my nit-picks guys.

Seems like a random fail, setting back to rtbc

These are different fails then the ones before, so i'm retesting this again.

Not sure why this is failing, EntityFileTest extends KernelTestBase but not KernelTestBaseTNG

The test failures of EntityFileTest are the same here as they are in in #2358319: Alt tag missing on user images, so I don't think these are related to this patch.

#2562453: EntityFileTest fails randomly is an issue for that.

As mentioned in some other tickets, there was also an issue with bot 3128 which was running these failing tests. Since it's been taking out of rotation it should pass again now.

Setting back to RTBC as per #30

+++ b/core/lib/Drupal/Component/FileCache/FileCache.php
@@ -153,4 +153,11 @@ public function delete($filepath) {
+  /**
+   * Resets the static cache.
+   */
+  public static function reset() {
+    static::$cached = [];
+  }

Why do we need the reset? Each KernelTestBase is run inside it's own process.

Why do we need the reset? Each KernelTestBase is run inside it's own process.

I hope we don't assume that.

@dawehner of course we assume that - in order to run KernelTestBaseTNG in the same process we have to remove all procedural code from Drupal.

I guess the point is, every time we build that assumption in we add one more hurdle to not having that assumption. Which isn't to say we are going to remove all the procedural code from Drupal, just that at some point we might have tests opt in to running in a separate process rather then that being the default or something like that. It seems like cleaning up is just the right thing to do.

Agreed with @dawehner and @neclimdul.

I am curious though as to why different FileCache instances share the same static property for caching data.

@dawehner of course we assume that - in order to run KernelTestBaseTNG in the same process we have to remove all procedural code from Drupal.

But we should try to remove the surface area of problems as much as possible. If we know about this static cache, we should clear it.

I think concerns have been addressed so going to put this back RTBC.

That's fair. We are just using the factory to do it. Better docs, still RTBC.

GET http://ec2-54-191-149-30.us-west-2.compute.amazonaws.com/checkout/test-d... returned 0 (0 bytes)

Let it be green while its green.

So this is completely lacking tests which is a shame. All we need to do is change \Drupal\system\Tests\Extension\ModuleHandlerTest to install system and change testModuleList() to start with the system module in the list.

And I'm still not a fan of the reset. It solves the issue in one place whereas we actually have more instances of this. What we need is a drupal_static() for classes - there is an issue for this somewhere. Can an @todo be added to remove this once that is done.

There we go

testbot....
GET http://ec2-54-186-69-54.us-west-2.compute.amazonaws.com/checkout/user/login returned 0

like it.

Committed 0f8b5b1 and pushed to 8.0.x. Thanks!