Decouple TestFileCreationTrait::getTestFiles() from simpletest module

Let's check the option when we move the files into /core/tests/fixtures/files folder.

Let`s fix failed tests

+++ b/core/modules/migrate_drupal/tests/fixtures/drupal6.php
@@ -49567,7 +49567,7 @@
-  'value' => 's:29:"core/modules/simpletest/files";',
+  'value' => 's:29:"core/tests/fixtures/files";',

String length changed, so the s:29 is wrong, hence the unserialize failure in the migrations I assume

Lets see how this does.

Great) changes looks good. +1 for rtbc

So I was thinking about these files in that folder, should we have a .htaccess file in there with a deny from all directive to prevent people using them to fingerprint the version of Drupal?

I think #17 is a little out of scope here? Isn't that something we should do/discuss in a broader scope? Not sure what the added value would be doing it just here (but maybe I'm missing something).

+++ b/core/modules/file/tests/src/Functional/Views/RelationshipUserFileDataTest.php
@@ -67,7 +67,7 @@ public function testViewsHandlerRelationshipUserFileData() {
     ]);
     $file->enforceIsNew();
-    file_put_contents($file->getFileUri(), file_get_contents('core/modules/simpletest/files/image-1.png'));
+    file_put_contents($file->getFileUri(), file_get_contents('core/tests/fixtures/files/image-1.png'));
     $file->save();
 

Relative paths assume that cwd while running the test is Drupal root. That's fine for DrupalCI bots, but other testing workflow may fail in this context. IMHO we should have absolute paths and prepend an app.root or a __DIR__dance here (and all other cases).

#19 sounds like a good improvement, but also sounds out of scope here, the same problem already exists in HEAD. Can we do that in a follow-up? Or is there a good reason that this needs to be done here?

Tentatively setting back to RTBC.

+++ b/core/tests/Drupal/KernelTests/Core/Image/ToolkitGdTest.php
@@ -280,7 +280,7 @@ public function testManipulations() {
     foreach ($files as $file) {
       foreach ($operations as $op => $values) {
         // Load up a fresh image.
-        $image = $this->imageFactory->get(drupal_get_path('module', 'simpletest') . '/files/' . $file);
+        $image = $this->imageFactory->get('core/tests/fixtures/files/' . $file);
         $toolkit = $image->getToolkit();
         if (!$image->isValid()) {
           $this->fail(new FormattableMarkup('Could not load image %file.', ['%file' => $file]));
@@ -426,14 +426,14 @@ public function testManipulations() {

Probably this case is more evident: I suspect (but am not sure) drupal_get_path('module', 'simpletest') will return an absolute path whereas after the change it will be relative. Not changing from RTBC, but I think #19 would avoid any risk.

I tested patch in #14 by running PHPUnit from within the core subdirectory

$ cd $DRUPAL_PATH/core
$ ../vendor/bin/phpunit --group image,Image

and all turned OK, so my concerns in #19 and #21 are not relevant. All good. Sorry for the noise.

+++ b/core/tests/Drupal/Tests/TestFileCreationTrait.php
@@ -69,8 +69,8 @@ protected function getTestFiles($type, $size = NULL) {
         $file_system->copy($file->uri, PublicStream::basePath());
diff --git a/core/modules/simpletest/files/README.txt b/core/tests/fixtures/files/README.txt

diff --git a/core/modules/simpletest/files/README.txt b/core/tests/fixtures/files/README.txt
similarity index 100%

similarity index 100%
rename from core/modules/simpletest/files/README.txt

rename from core/modules/simpletest/files/README.txt
rename to core/tests/fixtures/files/README.txt

I thought about copying these instead of renaming, but can't really imagine that contrib is relying on these fixtures for tests. If it turns out they are, we might need to copy them back again, but committing this as is and making a note here.