Merge pull request #59 from andrewnicols/unitTests

Check validity of unit test dataProviders
moodlehq · Sep 17, 2023 · 76ce969 · 76ce969
2 parents 8e1fc14 + ae762af
commit 76ce969
Show file tree

Hide file tree

Showing 24 changed files with 1,359 additions and 33 deletions.
diff --git a/moodle-extra/ruleset.xml b/moodle-extra/ruleset.xml
@@ -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>
diff --git a/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php b/moodle/Sniffs/PHPUnit/TestCaseCoversSniff.php
@@ -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.
 
@@ -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)) {

diff --git a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php
@@ -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.
 
@@ -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