Skip to content

Commit 28f900e

Browse files
committed
Don't overwrite the phpunit.xml file so much
Fixes #203 and #146 This solves two cases where moodle-plugin-ci was being "too aggressive" managing the phpunit.xml files. 1. If the plugin comes with its own phpunit.xml file, give to it absolute precedence, so it's not overwritten or modified ever. 2. If the plugin doesn't have any tests/coverage.php file, instead of applying moodle-plugin-ci defaults, let's allow core to apply its own ones. See MDL-72701 for more info about core coverage defaults. We'll continue applying our defaults for Moodle < 3.9.
1 parent e30aab8 commit 28f900e

File tree

6 files changed

+45
-48
lines changed

6 files changed

+45
-48
lines changed

docs/CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
99
The format of this change log follows the advice given at [Keep a CHANGELOG](http://keepachangelog.com).
1010

1111
## [Unreleased]
12+
### Changed
13+
- Plugin bundled `phpunit.xml` files are not overwritten or modified ever.
14+
- 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).
1215

1316
## [3.4.10] - 2023-03-14
1417
### Changed

src/Bridge/MoodlePlugin.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,16 @@ public function hasFilesWithName($pattern)
207207
return count($result) !== 0;
208208
}
209209

210+
/**
211+
* Determine if the plugin has a (phpunit.xml) configuration file.
212+
*
213+
* @return bool
214+
*/
215+
public function hasPhpUnitConfig(): bool
216+
{
217+
return is_file($this->directory.'/phpunit.xml');
218+
}
219+
210220
/**
211221
* Determine if the plugin has any Nodejs dependencies.
212222
*

src/Installer/TestSuiteInstaller.php

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,11 @@ public function getPostInstallProcesses()
130130
if ($this->plugin->hasUnitTests()) {
131131
$this->getOutput()->debug('Build PHPUnit config');
132132
$processes[] = new MoodleProcess(sprintf('%s/admin/tool/phpunit/cli/util.php --buildconfig', $this->moodle->directory));
133-
$processes[] = new MoodleProcess(sprintf('%s/admin/tool/phpunit/cli/util.php --buildcomponentconfigs', $this->moodle->directory));
133+
// Only create the PHPUnit config file (phpunit.xml) if it does not exist.
134+
// This is to avoid overwriting the file if it has been customized by the developer.
135+
if (!$this->plugin->hasPHPUnitConfig()) {
136+
$processes[] = new MoodleProcess(sprintf('%s/admin/tool/phpunit/cli/util.php --buildcomponentconfigs', $this->moodle->directory));
137+
}
134138
}
135139

136140
return $processes;
@@ -150,24 +154,35 @@ public function injectPHPUnitFilter()
150154
// section is already configured following it. Nothing to do here.
151155
$coverage = $this->plugin->directory.'/tests/coverage.php';
152156
// If the file exists and we are Moodle >= 3.7.
153-
// TODO: Remove the branch condition when 3.6 becomes unsupported by moodle-local-ci.
157+
// TODO: Remove the branch condition when 3.6 becomes unsupported by moodle-plugin-ci.
154158
if ($this->moodle->getBranch() >= 37 && is_readable($coverage)) {
155159
return;
156160
}
157161

162+
// If the coverage.php file does not exist, and we are Moodle < 3.9, then try to inject the default
163+
// filter/coverage information. Note that, for Moodle 3.9 and up, the filter/coverage information
164+
// with good defaults is already present in the phpunit.xml file. See MDL-72701.
165+
// TODO: Remove the whole code in the function after this line, when 3.8 becomes unsupported by moodle-plugin-ci.
166+
if ($this->moodle->getBranch() >= 39) {
167+
return;
168+
}
169+
158170
$files = $this->getCoverageFiles();
159171
$filterXml = $this->getFilterXml($files);
160172
$subject = file_get_contents($config);
161173
$count = 0;
162174

163175
// Replace existing filter.
164176
$contents = preg_replace('/<coverage>(.|\n)*<\/coverage>/m', trim($filterXml), $subject, 1, $count);
165-
// TODO: Remove this when 3.10 becomes unsupported by moodle-local-ci.
177+
// TODO: Remove this when 3.10 becomes unsupported by moodle-plugin-ci.
166178
if ($this->moodle->getBranch() < 311) {
167179
$contents = preg_replace('/<filter>(.|\n)*<\/filter>/m', trim($filterXml), $subject, 1, $count);
168180
}
169181

170182
// Or if no existing filter, inject the filter.
183+
// We only do this for Moodle < 3.9, because in Moodle 3.9+ the filter with better defaults
184+
// is already present in the phpunit.xml file. See MDL-72701.
185+
// TODO: Remove this when 3.8 becomes unsupported by moodle-plugin-ci.
171186
if ($count === 0) {
172187
$contents = str_replace('</phpunit>', $filterXml.'</phpunit>', $subject, $count);
173188
}

tests/Bridge/MoodlePluginTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ public function testHasUnitTests()
4545
$this->assertTrue($plugin->hasUnitTests());
4646
}
4747

48+
public function testHasPhpUnitConfig()
49+
{
50+
// Our plugins doesn't have a phpunit.xml file.
51+
$plugin = new MoodlePlugin($this->pluginDir);
52+
$this->assertFalse($plugin->hasPhpUnitConfig());
53+
54+
// Let's create one.
55+
$this->fs->dumpFile($this->pluginDir.'/phpunit.xml', '');
56+
$this->assertTrue($plugin->hasPhpUnitConfig());
57+
}
58+
4859
public function testNoUnitTests()
4960
{
5061
// Remove the only unit test file.

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

Lines changed: 0 additions & 42 deletions
This file was deleted.

tests/Installer/TestSuiteInstallerTest.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,12 @@ public function testPHPUnitXMLFile311()
112112
new DummyExecute()
113113
);
114114

115-
// Test Moodle 3.11 PHPUnit XML file.
115+
// Test Moodle 3.11 PHPUnit XML file. Nothing is changed.
116116
$this->fs->copy(__DIR__.'/../Fixture/phpunit/phpunit-311.xml', $xmlFile, true);
117117
$installer->injectPHPUnitFilter();
118-
$this->assertXmlFileEqualsXmlFile(__DIR__.'/../Fixture/phpunit/phpunit-expected-311.xml', $xmlFile);
118+
$this->assertXmlFileEqualsXmlFile(__DIR__.'/../Fixture/phpunit/phpunit-311.xml', $xmlFile);
119119

120-
// Test Moodle 3.11 PHPUnit XML file when coverage.php is available.
120+
// Test Moodle 3.11 PHPUnit XML file when coverage.php is available. Nothing is changed either.
121121
$this->fs->copy(__DIR__.'/../Fixture/phpunit/phpunit-311.xml', $xmlFile, true);
122122
$this->fs->copy(__DIR__.'/../Fixture/phpunit/coverage.php', $this->pluginDir.'/tests/coverage.php', true);
123123
$installer->injectPHPUnitFilter();

0 commit comments

Comments
 (0)