Skip to content

Commit

Permalink
Verify that sub namespaces in test match the expected location
Browse files Browse the repository at this point in the history
Added one more check to the unit tests naming sniff that now
verify, for testcase clases, that they are in the expected
subdirectory, following the general namespace rules for stuff
under the "classes" directory.

Note that we still aren't checking the correctness under "classes"
and, once that's implemented, surely some code here will be moved
to the general namespaces sniff (a note about that has been added
to code).

Also, note that we aren't still checking for level2 correctness, that
must be a valid API or "local", whenever that's possible it will be
checked.

So, this only validates that the location (sub-directories) of the
file matches the sub-namespaces of the class.

Covered with tests.
  • Loading branch information
stronk7 committed Dec 29, 2021
1 parent c0ef023 commit 7f87301
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 1 deletion.
23 changes: 23 additions & 0 deletions moodle/Sniffs/PHPUnit/TestCaseNamesSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
11 changes: 11 additions & 0 deletions moodle/tests/fixtures/phpunit/testcasenames_correct_level2ns.php
Original file line number Diff line number Diff line change
@@ -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() {
}
}
Original file line number Diff line number Diff line change
@@ -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() {
}
}
16 changes: 16 additions & 0 deletions moodle/tests/phpunit_testcasenames_test.php
Original file line number Diff line number Diff line change
Expand Up @@ -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' => [
Expand Down
2 changes: 1 addition & 1 deletion tests/behat/ui.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down

0 comments on commit 7f87301

Please sign in to comment.