Skip to content

Commit 8b2a060

Browse files
committed
New moodle.PHPUnit.TestCaseNames Sniff
Sniff in charge of verifying various things related with PHPUnit test cases names (files and classes and namespaces). Like: - Error: _test.php files missing any valid test case. - Error: incorrectly names test cases (not ended with _test[case] - Warning: test case classes not matching test case file names. - Error: found duplicate test case names. - Error: found test case name proposed for another test case. - Error: test case namespace not matching expected component namespace. - Warning: test case namespace missing, suggesting a correct one. - Error: found duplicate proposed test case names. - Error: found proposed test case name existing for another test case. Practically covered with unit tests. And behat tests updated. Also included, small modification to local_codechecker_testcase base class that now, when an incorrect fixture file is passed, it fails with error (previously it was exiting silently and was hard to detect what was happening when a typo in the fixture name was used).
1 parent 5d3ad60 commit 8b2a060

13 files changed

+537
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,271 @@
1+
<?php
2+
// This file is part of Moodle - http://moodle.org/
3+
//
4+
// Moodle is free software: you can redistribute it and/or modify
5+
// it under the terms of the GNU General Public License as published by
6+
// the Free Software Foundation, either version 3 of the License, or
7+
// (at your option) any later version.
8+
//
9+
// Moodle is distributed in the hope that it will be useful,
10+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
11+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12+
// GNU General Public License for more details.
13+
//
14+
// You should have received a copy of the GNU General Public License
15+
// along with Moodle. If not, see <http://www.gnu.org/licenses/>.
16+
17+
/**
18+
* Checks that a test file has a class name matching the file name.
19+
*
20+
* @package local_codechecker
21+
* @copyright 2021 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com}
22+
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
23+
*/
24+
25+
namespace MoodleCodeSniffer\moodle\Sniffs\PHPUnit;
26+
27+
// phpcs:disable moodle.NamingConventions
28+
29+
use PHP_CodeSniffer\Sniffs\Sniff;
30+
use PHP_CodeSniffer\Files\File;
31+
use PHP_CodeSniffer\Util\Tokens;
32+
use MoodleCodeSniffer\moodle\Util\MoodleUtil;
33+
34+
class TestCaseNamesSniff implements Sniff {
35+
36+
/**
37+
* List of classes that have been found during checking.
38+
*
39+
* @var array
40+
*/
41+
protected $foundClasses = [];
42+
43+
/**
44+
* List of classes that have been proposed during checking.
45+
*
46+
* @var array
47+
*/
48+
protected $proposedClasses = [];
49+
50+
/**
51+
* Register for open tag (only process once per file).
52+
*/
53+
public function register() {
54+
return array(T_OPEN_TAG);
55+
}
56+
57+
/**
58+
* Processes php files and perform various checks with file, namespace and class names.
59+
* inclusion.
60+
*
61+
* @param File $file The file being scanned.
62+
* @param int $pointer The position in the stack.
63+
*/
64+
public function process(File $file, $pointer) {
65+
66+
// Before starting any check, let's look for various things.
67+
68+
// Guess moodle component (from $file being processed).
69+
$moodleComponent = MoodleUtil::getMoodleComponent($file);
70+
71+
// We have all we need from core, let's start processing the file.
72+
73+
// Get the file tokens, for ease of use.
74+
$tokens = $file->getTokens();
75+
76+
// We only want to do this once per file.
77+
$prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1);
78+
if ($prevopentag !== false) {
79+
return;
80+
}
81+
82+
// If the file isn't under tests directory, nothing to check.
83+
if (strpos($file->getFilename(), '/tests/') === false) {
84+
return;
85+
}
86+
87+
// If the file isn't called, _test.php, nothing to check.
88+
$fileName = basename($file->getFilename());
89+
if (substr($fileName, -9) !== '_test.php') {
90+
// Make an exception for codechecker own phpunit fixtures here, allowing any name for them.
91+
if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) {
92+
return;
93+
}
94+
}
95+
96+
// In order to cover the duplicates detection, we need to set some
97+
// properties (caches) here. It's extremely hard to do
98+
// this via mocking / extending (at very least for this humble developer).
99+
if (defined('PHPUNIT_TEST') && PHPUNIT_TEST) {
100+
$this->prepareCachesForPHPUnit();
101+
}
102+
103+
// Get the class namespace.
104+
$namespace = '';
105+
$nsStart = 0;
106+
if ($nsStart = $file->findNext(T_NAMESPACE, ($pointer + 1))) {
107+
$nsEnd = $file->findNext([T_NS_SEPARATOR, T_STRING, T_WHITESPACE], ($nsStart + 1), null, true);
108+
$namespace = strtolower(trim($file->getTokensAsString(($nsStart + 1), ($nsEnd - $nsStart - 1))));
109+
}
110+
$pointer = $nsEnd ?? $pointer; // When possible, move the pointer to after the namespace name.
111+
112+
// Get the name of the 1st class in the file (this Sniff doesn't detects multiple),
113+
// verify that it extends something and that has a test_ method.
114+
$class = '';
115+
$classFound = false;
116+
while ($cStart = $file->findNext(T_CLASS, $pointer)) {
117+
$pointer = $cStart + 1; // Move the pointer to the class start.
118+
119+
// Only if the class is extending something.
120+
// TODO: We could add a list of valid classes once we have a class-map available.
121+
if (!$file->findNext(T_EXTENDS, $cStart + 1, $tokens[$cStart]['scope_opener'])) {
122+
continue;
123+
}
124+
125+
// Verify that the class has some test_xxx method.
126+
$method = '';
127+
$methodFound = false;
128+
while ($mStart = $file->findNext(T_FUNCTION, $pointer, $tokens[$cStart]['scope_closer'])) {
129+
$pointer = $tokens[$mStart]['scope_closer']; // Next iteration look after the end of current method.
130+
if (strpos($file->getDeclarationName($mStart), 'test_') === 0) {
131+
$methodFound = true;
132+
$method = $file->getDeclarationName($mStart);
133+
break;
134+
}
135+
}
136+
137+
// If we have found a test_ method, this is our class (the 1st having one).
138+
if ($methodFound) {
139+
$classFound = true;
140+
$class = $file->getDeclarationName($cStart);
141+
$class = strtolower(trim($class));
142+
break;
143+
}
144+
$pointer = $tokens[$cStart]['scope_closer']; // Move the pointer to the class end.
145+
}
146+
147+
// No testcase class found, this is plain-wrong.
148+
if (!$classFound) {
149+
$file->addError('PHPUnit test file missing any valid testcase class declaration', 0, 'Missing');
150+
return; // If arrived here we don't have a valid class, we are finished.
151+
}
152+
153+
// All the following checks assume that a valid class has been found.
154+
155+
// Error if the found classname is "strange" (not "_test|_testcase" ended).
156+
if (substr($class, -5) !== '_test' && substr($class, -9) != '_testcase') {
157+
$file->addError('PHPUnit irregular testcase name found: %s (_test/_testcase ended expected)', $cStart,
158+
'Irregular', [$class]);
159+
}
160+
161+
// Check if the file name and the class name match, warn if not.
162+
$baseName = pathinfo($fileName, PATHINFO_FILENAME);
163+
if ($baseName !== $class) {
164+
$file->addWarning('PHPUnit testcase name "%s" does not match file name "%s"', $cStart,
165+
'NoMatch', [$class, $baseName]);
166+
}
167+
168+
// Check if the class has been already found (this is useful when running against a lot of files).
169+
$fdqnClass = $namespace ? $namespace . '\\' . $class : $class;
170+
if (isset($this->foundClasses[$fdqnClass])) {
171+
// Already found, this is a dupe class name, error!
172+
foreach ($this->foundClasses[$fdqnClass] as $exists) {
173+
$file->addError('PHPUnit testcase "%s" already exists at "%s" line %s', $cStart,
174+
'DuplicateExists', [$fdqnClass, $exists['file'], $exists['line']]);
175+
}
176+
} else {
177+
// Create the empty element.
178+
$this->foundClasses[$fdqnClass] = [];
179+
}
180+
181+
// Add the new element.
182+
$this->foundClasses[$fdqnClass][] = [
183+
'file' => $file->getFilename(),
184+
'line' => $tokens[$cStart]['line'],
185+
];
186+
187+
// Check if the class has been already proposed (this is useful when running against a lot of files).
188+
if (isset($this->proposedClasses[$fdqnClass])) {
189+
// Already found, this is a dupe class name, error!
190+
foreach ($this->proposedClasses[$fdqnClass] as $exists) {
191+
$file->addError('PHPUnit testcase "%s" already proposed for "%s" line %s. You ' .
192+
'may want to change the testcase name (file and class)', $cStart,
193+
'ProposedExists', [$fdqnClass, $exists['file'], $exists['line']]);
194+
}
195+
}
196+
197+
// Validate 1st level namespace.
198+
199+
if ($namespace && $moodleComponent) {
200+
// Verify that the namespace declared in the class matches the namespace expected for the file.
201+
if (strpos($namespace . '\\', $moodleComponent . '\\') !== 0) {
202+
$file->addError('PHPUnit class namespace "%s" does not match expected file namespace "%s"', $nsStart,
203+
'UnexpectedNS', [$namespace, $moodleComponent]);
204+
}
205+
}
206+
207+
if (!$namespace && $moodleComponent) {
208+
$file->addWarning('PHUnit class "%s" does not have any namespace. It is recommended to add it to the "%s" ' .
209+
'namespace, using more levels if needed, in order to match the code being tested', $cStart,
210+
'MissingNS', [$fdqnClass, $moodleComponent]);
211+
212+
// Check if the proposed class has been already proposed (this is useful when running against a lot of files).
213+
$fdqnProposed = $moodleComponent . '\\' . $fdqnClass;
214+
if (isset($this->proposedClasses[$fdqnProposed])) {
215+
// Already found, this is a dupe class name, error!
216+
foreach ($this->proposedClasses[$fdqnProposed] as $exists) {
217+
$file->addError('Proposed PHPUnit testcase "%s" already proposed for "%s" line %s. You ' .
218+
'may want to change the testcase name (file and class)', $cStart,
219+
'DuplicateProposed', [$fdqnProposed, $exists['file'], $exists['line']]);
220+
}
221+
} else {
222+
// Create the empty element.
223+
$this->proposedClasses[$fdqnProposed] = [];
224+
}
225+
226+
// Add the new element.
227+
$this->proposedClasses[$fdqnProposed][] = [
228+
'file' => $file->getFilename(),
229+
'line' => $tokens[$cStart]['line'],
230+
];
231+
232+
// Check if the proposed class has been already found (this is useful when running against a lot of files).
233+
if (isset($this->foundClasses[$fdqnProposed])) {
234+
// Already found, this is a dupe class name, error!
235+
foreach ($this->foundClasses[$fdqnProposed] as $exists) {
236+
$file->addError('Proposed PHPUnit testcase "%s" already exists at "%s" line %s. You ' .
237+
'may want to change the testcase name (file and class)', $cStart,
238+
'ExistsProposed', [$fdqnProposed, $exists['file'], $exists['line']]);
239+
}
240+
}
241+
}
242+
}
243+
244+
/**
245+
* Prepare found and proposed caches for PHPUnit.
246+
*
247+
* It's near impossible to extend or mock this class from PHPUnit in order
248+
* to get the caches pre-filled with some values that will cover some
249+
* of the logic of the sniff (at least for this developer).
250+
*
251+
* So we fill them here when it's detected that we are running PHPUnit.
252+
*/
253+
private function prepareCachesForPHPUnit() {
254+
$this->foundClasses['local_codechecker\testcasenames_duplicate_exists'][] = [
255+
'file' => 'phpunit_fake_exists',
256+
'line' => -999,
257+
];
258+
$this->foundClasses['local_codechecker\testcasenames_exists_proposed'][] = [
259+
'file' => 'phpunit_fake_exists',
260+
'line' => -999,
261+
];
262+
$this->proposedClasses['local_codechecker\testcasenames_duplicate_proposed'][] = [
263+
'file' => 'phpunit_fake_proposed',
264+
'line' => -999,
265+
];
266+
$this->proposedClasses['local_codechecker\testcasenames_proposed_exists'][] = [
267+
'file' => 'phpunit_fake_proposed',
268+
'line' => -999,
269+
];
270+
}
271+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
namespace local_codechecker;
3+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
4+
5+
/**
6+
* Correct class but with name not matching the file name.
7+
*/
8+
class testcasenames_duplicate_exists extends \local_codechecker_testcase {
9+
public function test_something() {
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
3+
4+
/**
5+
* Correct class but with name not matching the file name.
6+
*/
7+
class testcasenames_duplicate_proposed extends \local_codechecker_testcase {
8+
public function test_something() {
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
3+
4+
/**
5+
* Correct class but with name not matching the file name.
6+
*/
7+
class testcasenames_exists_proposed extends \local_codechecker_testcase {
8+
public function test_something() {
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
3+
4+
/**
5+
* Class not extending anything
6+
*/
7+
class testcasenames_notextending {
8+
// This class does not extend anything.
9+
}
10+
11+
/**
12+
* Class missing any test_ method
13+
*/
14+
class testcasenames_notestmethod extends local_codechecker_testcase {
15+
public function notest_something() {
16+
// This method is not a unit test.
17+
}
18+
public function notest_either() {
19+
// This method is not a unit test.
20+
}
21+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
3+
4+
/**
5+
* Correct class but with missing namespace (and irregular test name).
6+
*/
7+
class testcasenames_missing_ns extends \local_codechecker_testcase {
8+
public function test_something() {
9+
}
10+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
namespace local_codechecker;
3+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
4+
5+
/**
6+
* Correct class but with name not matching the file name.
7+
*/
8+
class testcasenames_nomatch_test extends local_codechecker_testcase {
9+
public function test_something() {
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
namespace local_codechecker;
3+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
4+
5+
/**
6+
* Correct class but with name not matching the file name.
7+
*/
8+
class testcasenames_proposed_exists extends \local_codechecker_testcase {
9+
public function test_something() {
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
namespace local_codechecker;
3+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
4+
5+
/**
6+
* Correct class but with incorrect name (not ended in _test or _testcase)
7+
*/
8+
class testcasenames_test_testcase_irregular extends local_codechecker_testcase {
9+
public function test_something() {
10+
}
11+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<?php
2+
namespace local_wrong;
3+
defined("MOODLE_INTERNAL") || die(); // Make this always the 1st line in all CS fixtures.
4+
5+
/**
6+
* Correct class but with name not matching the file name.
7+
*/
8+
class testcasenames_unexpected_ns extends \local_codechecker_testcase {
9+
public function test_something() {
10+
}
11+
}

0 commit comments

Comments
 (0)