Merge pull request #167 from stronk7/verify_level2_test_namespace

Verify that sub namespaces in test match the expected location
moodlehq · Dec 30, 2021 · e43d72c · e43d72c
2 parents c0ef023 + 7f87301
commit e43d72c
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 1 deletion.
diff --git a/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php b/moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php
@@ -200,6 +200,8 @@ public function process(File $file, $pointer) {
             }
         }
 
+        // TODO: When all the namespace general rules Sniff is created, all these namespace checks
+        // should be moved there and reused here.
         // Validate 1st level namespace.
 
         if ($namespace && $moodleComponent) {
@@ -208,6 +210,27 @@ public function process(File $file, $pointer) {
                 $file->addError('PHPUnit class namespace "%s" does not match expected file namespace "%s"', $nsStart,
                     'UnexpectedNS', [$namespace, $moodleComponent]);
             }
+
+            // Verify that level2 and down match the directory structure under tests. Soft warn if not (till we fix all).
+            $bspos = strpos(trim($namespace, ' \\'), '\\');
+            if ($bspos !== false) { // Only if there are level2 and down namespace.
+                $relns = str_replace('\\', '/', substr(trim($namespace, ' \\'), $bspos + 1));
+
+                // Calculate the relative path under tests directory.
+                $dirpos = strrpos(trim(dirname($file->getFilename()), ' /') . '/', '/tests/');
+                $reldir = str_replace('\\', '/', substr(trim(dirname($file->getFilename()), ' /'), $dirpos + 7));
+
+                // Warning if the relative namespace does not match the relative directory.
+                if ($reldir !== $relns) {
+                    $file->addWarning('PHPUnit class "%s", with namespace "%s", currently located at "tests/%s" directory, '.
+                        'does not match its expected location at "tests/%s"', $nsStart,
+                        'UnexpectedLevel2NS', [$fdqnClass, $namespace, $reldir, $relns]);
+                }
+
+                // TODO: When we have APIs (https://docs.moodle.org/dev/Core_APIs) somewhere at hand (in core)
+                // let's add here an error when incorrect ones are used. See MDL-71096 about it.
+            }
+
         }
 
         if (!$namespace && $moodleComponent) {

diff --git a/moodle/tests/fixtures/phpunit/testcasenames_correct_level2ns.php b/moodle/tests/fixtures/phpunit/testcasenames_correct_level2ns.php
@@ -0,0 +1,11 @@
+<?php
+namespace local_codechecker\fixtures\phpunit; // Not correct level2, but we aren't checking that. Just correct location.
+defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
+
+/**
+ * Correct class, using correct location matching sub-namespaces.
+ */
+class testcasenames_correct_level2ns extends local_codechecker_testcase {
+    public function test_something() {
+    }
+}
diff --git a/moodle/tests/fixtures/phpunit/testcasenames_unexpected_level2ns.php b/moodle/tests/fixtures/phpunit/testcasenames_unexpected_level2ns.php
@@ -0,0 +1,11 @@
+<?php
+namespace local_codechecker\level2\level3;
+defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
+
+/**
+ * Correct class, just the namespace level2 and level3 don't correspond with the expected location.
+ */
+class testcasenames_unexpected_level2ns extends local_codechecker_testcase {
+    public function test_something() {
+    }
+}
diff --git a/moodle/tests/phpunit_testcasenames_test.php b/moodle/tests/phpunit_testcasenames_test.php
@@ -71,6 +71,22 @@ public function provider_phpunit_testcasenames() {
                 ],
                 'warnings' => [],
             ],
+            'UnexpectedLevel2NS' => [
+                'fixture' => 'fixtures/phpunit/testcasenames_unexpected_level2ns.php',
+                'errors' => [
+                    8 => 1,
+                ],
+                'warnings' => [
+                    2 => 'does not match its expected location at "tests/level2/level3"'
+                ],
+            ],
+            'CorrectLevel2NS' => [
+                'fixture' => 'fixtures/phpunit/testcasenames_correct_level2ns.php',
+                'errors' => [
+                    8 => 1,
+                ],
+                'warnings' => [],
+            ],
             'MissingNS' => [
                 'fixture' => 'fixtures/phpunit/testcasenames_missing_ns.php',
                 'errors' => [

diff --git a/tests/behat/ui.feature b/tests/behat/ui.feature
@@ -42,7 +42,7 @@ Feature: Codechecker UI works as expected
       | path                            | exclude            | seen                          | notseen      |
       | local/codechecker/moodle/tests  | */tests/fixtures/* | Files found: 3                | Invalid path |
       | local/codechecker/moodle/tests  | */tests/fixtures/* | moodlestandard_test.php       | Invalid path |
-      | local/codechecker/moodle/tests/ | *PHPC*, *moodle_*  | Files found: 28               | Invalid path |
+      | local/codechecker/moodle/tests/ | *PHPC*, *moodle_*  | Files found: 30               | Invalid path |
       | local/codechecker/moodle/tests/ | *PHPC*, *moodle_*  | Line 1 of the opening comment | moodle_php   |
       | local/codechecker/moodle/tests/ | *PHPC*, *moodle_*  | Inline comments must end      | /phpcompat   |
       | local/codechecker/moodle/tests/ | *PHPC*, *moodle_*  | Inline comments must end      | /phpcompat   |