Merge pull request #216 from stronk7/dont_overwite_phpunit_xml

Don't overwrite the phpunit.xml file so much

Author: stronk7

docs/CHANGELOG.md

@@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 The format of this change log follows the advice given at [Keep a CHANGELOG](http://keepachangelog.com).
 
 ## [Unreleased]
+### Changed
+- Plugin bundled `phpunit.xml` files are not overwritten or modified ever.
+- For Moodle 3.9 and up, when the plugin is missing any `tests/coverage.php` file, core defaults (`lib.php`, `locallib.php`, `classes/`, ...) will be applied. Previously, all the `*.php` files were applied by default (note that older Moodle versions will continue getting them).
 
 ## [3.4.10] - 2023-03-14
 ### Changed

src/Bridge/MoodlePlugin.php

@@ -197,6 +197,16 @@ public function hasFilesWithName($pattern): bool
         return count($result) !== 0;
     }
 
+    /**
+     * Determine if the plugin has a (phpunit.xml) configuration file.
+     *
+     * @return bool
+     */
+    public function hasPhpUnitConfig(): bool
+    {
+        return is_file($this->directory . '/phpunit.xml');
+    }
+
     /**
      * Determine if the plugin has any Nodejs dependencies.
      *

src/Installer/TestSuiteInstaller.php

@@ -134,10 +134,15 @@ public function getPostInstallProcesses(): array
                 $this->moodle->directory . '/admin/tool/phpunit/cli/util.php',
                 '--buildconfig',
             ]);
-            $processes[] = new MoodleProcess([
-                $this->moodle->directory . '/admin/tool/phpunit/cli/util.php',
-                '--buildcomponentconfigs',
-            ]);
+            // Only create the PHPUnit config file (phpunit.xml) if it does not exist.
+            // This is to avoid overwriting the file if it has been customized by the developer.
+            if (!$this->plugin->hasPHPUnitConfig()) {
+                $this->getOutput()->debug('Build PHPUnit component config');
+                $processes[] = new MoodleProcess([
+                    $this->moodle->directory . '/admin/tool/phpunit/cli/util.php',
+                    '--buildcomponentconfigs',
+                ]);
+            }
         }
 
         return $processes;
@@ -157,24 +162,35 @@ public function injectPHPUnitFilter(): void
         // section is already configured following it. Nothing to do here.
         $coverage = $this->plugin->directory . '/tests/coverage.php';
         // If the file exists and we are Moodle >= 3.7.
-        // TODO: Remove the branch condition when 3.6 becomes unsupported by moodle-local-ci.
+        // TODO: Remove the branch condition when 3.6 becomes unsupported by moodle-plugin-ci.
         if ($this->moodle->getBranch() >= 37 && is_readable($coverage)) {
             return;
         }
 
+        // If the coverage.php file does not exist, and we are Moodle < 3.9, then try to inject the default
+        // filter/coverage information. Note that, for Moodle 3.9 and up, the filter/coverage information
+        // with good defaults is already present in the phpunit.xml file. See MDL-72701.
+        // TODO: Remove the whole code in the function after this line, when 3.8 becomes unsupported by moodle-plugin-ci.
+        if ($this->moodle->getBranch() >= 39) {
+            return;
+        }
+
         $files     = $this->getCoverageFiles();
         $filterXml = $this->getFilterXml($files);
         $subject   = file_get_contents($config);
         $count     = 0;
 
         // Replace existing filter.
         $contents = preg_replace('/<coverage>(.|\n)*<\/coverage>/m', trim($filterXml), $subject, 1, $count);
-        // TODO: Remove this when 3.10 becomes unsupported by moodle-local-ci.
+        // TODO: Remove this when 3.10 becomes unsupported by moodle-plugin-ci.
         if ($this->moodle->getBranch() < 311) {
             $contents = preg_replace('/<filter>(.|\n)*<\/filter>/m', trim($filterXml), $subject, 1, $count);
         }
 
         // Or if no existing filter, inject the filter.
+        // We only do this for Moodle < 3.9, because in Moodle 3.9+ the filter with better defaults
+        // is already present in the phpunit.xml file. See MDL-72701.
+        // TODO: Remove this when 3.8 becomes unsupported by moodle-plugin-ci.
         if ($count === 0) {
             $contents  = str_replace('</phpunit>', $filterXml . '</phpunit>', $subject, $count);
         }

tests/Bridge/MoodlePluginTest.php

@@ -45,6 +45,17 @@ public function testHasUnitTests()
         $this->assertTrue($plugin->hasUnitTests());
     }
 
+    public function testHasPhpUnitConfig()
+    {
+        // Our plugins doesn't have a phpunit.xml file.
+        $plugin = new MoodlePlugin($this->pluginDir);
+        $this->assertFalse($plugin->hasPhpUnitConfig());
+
+        // Let's create one.
+        $this->fs->dumpFile($this->pluginDir . '/phpunit.xml', '');
+        $this->assertTrue($plugin->hasPhpUnitConfig());
+    }
+
     public function testNoUnitTests()
     {
         // Remove the only unit test file.

tests/Fixture/phpunit/phpunit-expected-311.xml

@@ -112,12 +112,12 @@ public function testPHPUnitXMLFile311()
             new DummyExecute()
         );
 
-        // Test Moodle 3.11 PHPUnit XML file.
+        // Test Moodle 3.11 PHPUnit XML file. Nothing is changed.
         $this->fs->copy(__DIR__ . '/../Fixture/phpunit/phpunit-311.xml', $xmlFile, true);
         $installer->injectPHPUnitFilter();
-        $this->assertXmlFileEqualsXmlFile(__DIR__ . '/../Fixture/phpunit/phpunit-expected-311.xml', $xmlFile);
+        $this->assertXmlFileEqualsXmlFile(__DIR__ . '/../Fixture/phpunit/phpunit-311.xml', $xmlFile);
 
-        // Test Moodle 3.11 PHPUnit XML file when coverage.php is available.
+        // Test Moodle 3.11 PHPUnit XML file when coverage.php is available. Nothing is changed either.
         $this->fs->copy(__DIR__ . '/../Fixture/phpunit/phpunit-311.xml', $xmlFile, true);
         $this->fs->copy(__DIR__ . '/../Fixture/phpunit/coverage.php', $this->pluginDir . '/tests/coverage.php', true);
         $installer->injectPHPUnitFilter();