Skip to content

Commit

Permalink
Check validity of unit test dataProviders
Browse files Browse the repository at this point in the history
This change includes:
- detection of private providers
- detection of providers which do not exist
- detection of incorrect casing of the dataProvider declaration
- detection of providers which do not have a correct return type
- detection of providers which are not static
- detection of providers whose names start with test_

Fixes moodlehq#42
  • Loading branch information
andrewnicols committed Sep 17, 2023
1 parent 8e1fc14 commit ae762af
Show file tree
Hide file tree
Showing 24 changed files with 1,359 additions and 33 deletions.
15 changes: 15 additions & 0 deletions moodle-extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -81,4 +81,19 @@
-->

<!--
Detect issues with Unit Test dataProviders:
- private providers
- providers which do not exist
- providers whose name is prefixed with _test
- incorrect casing of dataProvider
- dataProviders which do not return an array or Iterable
- dataProviders which can be converted to a static method (PHPUnit 10 compatibility)
-->
<rule ref="moodle.PHPUnit.TestCaseProvider">
<properties>
<property name="autofixStaticProviders" value="true"/>
</properties>
</rule>

</ruleset>
31 changes: 9 additions & 22 deletions moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,16 @@ public function process(File $file, $pointer) {

// Before starting any check, let's look for various things.

// Get the moodle branch being analysed.
$moodleBranch = MoodleUtil::getMoodleBranch($file);
// If we aren't checking Moodle 4.0dev (400) and up, nothing to check.
// Make and exception for codechecker phpunit tests, so they are run always.
if (!MoodleUtil::meetsMinimumMoodleVersion($file, 400) && !MoodleUtil::isUnitTestRunning()) {
return; // @codeCoverageIgnore
}

// Detect if we are running PHPUnit.
$runningPHPUnit = defined('PHPUNIT_TEST') && PHPUNIT_TEST;
// If the file is not a unit test file, nothing to check.
if (!MoodleUtil::isUnitTest($file) && !MoodleUtil::isUnitTestRunning()) {
return; // @codeCoverageIgnore
}

// We have all we need from core, let's start processing the file.

Expand All @@ -70,24 +75,6 @@ public function process(File $file, $pointer) {
return; // @codeCoverageIgnore
}

// If we aren't checking Moodle 4.0dev (400) and up, nothing to check.
// Make and exception for codechecker phpunit tests, so they are run always.
if (isset($moodleBranch) && $moodleBranch < 400 && !$runningPHPUnit) {
return; // @codeCoverageIgnore
}

// If the file isn't under tests directory, nothing to check.
if (stripos($file->getFilename(), '/tests/') === false) {
return; // @codeCoverageIgnore
}

// If the file isn't called, _test.php, nothing to check.
// Make an exception for codechecker own phpunit fixtures here, allowing any name for them.
$fileName = basename($file->getFilename());
if (substr($fileName, -9) !== '_test.php' && !$runningPHPUnit) {
return; // @codeCoverageIgnore
}

// Iterate over all the classes (hopefully only one, but that's not this sniff problem).
$cStart = $pointer;
while ($cStart = $file->findNext(T_CLASS, $cStart + 1)) {
Expand Down
12 changes: 3 additions & 9 deletions moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public function process(File $file, $pointer) {
$moodleComponent = MoodleUtil::getMoodleComponent($file);

// Detect if we are running PHPUnit.
$runningPHPUnit = defined('PHPUNIT_TEST') && PHPUNIT_TEST;
$runningPHPUnit = MoodleUtil::isUnitTestRunning();

// We have all we need from core, let's start processing the file.

Expand All @@ -81,17 +81,11 @@ public function process(File $file, $pointer) {
return; // @codeCoverageIgnore
}

// If the file isn't under tests directory, nothing to check.
if (stripos($file->getFilename(), '/tests/') === false) {
// If the file is not a unit test file, nothing to check.
if (!MoodleUtil::isUnitTest($file) && !$runningPHPUnit) {
return; // @codeCoverageIgnore
}

// If the file isn't called, _test.php, nothing to check.
// Make an exception for codechecker own phpunit fixtures here, allowing any name for them.
$fileName = basename($file->getFilename());
if (substr($fileName, -9) !== '_test.php' && !$runningPHPUnit) {
return; // @codeCoverageIgnore
}

// In order to cover the duplicates detection, we need to set some
// properties (caches) here. It's extremely hard to do
Expand Down
Loading

0 comments on commit ae762af

Please sign in to comment.