-
Notifications
You must be signed in to change notification settings - Fork 72
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add new TestCaseCovers sniff to control @Covers tags in unit tests
This new feature will be in charge of validating the presence and correctness of PHPUnit coverage annotations/tags. Restrictions: - It's only applied to Moodle 4.0 and up. - It's only applied to files under /tests directories. - It's only applied to files named `_test.php`. - It's only applied to classes named `_test` or `_testcase`. - it's only applied to methods named `test_ `. Warnings implemented: - Warn if both @Covers and @coversNothing information are missing. - Warn when there are contradictory mixed @coversNothing (at class) and @Covers (at method) combinations. - Warn about redundant @coversNothing present both at class and method. Errors implemented: - Error when there are contradictory @coversNothing and @Covers in the same class or method. - Error if the @coversDefaultClass is used in method. - Error if the @coversNothing tag has any value. - Error if the @Covers or @coversDefaultClass tags don't have any value. - Error if the @coversDefaultClass tag doesn't start with \ (FQCN). - Error if the @coversDefaultClass tag has :: (reserved for @Covers). - Error if the @Covers tag doesn't start with \ (FQCN) or :: (method). 96.67% line coverage achieved. Impact notes: See that, while adding @Covers annotations to various unit tests, it's also mandatory to add a description to them. That's because the local_moodlecheck (in charge of examining phpdoc blocks) accepts the absence of phpdoc blocks, but once they exist, the description in mandatory. Not sure if we should relax that in moodlecheck or no (being strict, I think it's correct).
- Loading branch information
Showing
13 changed files
with
734 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,279 @@ | ||
<?php | ||
// This file is part of Moodle - http://moodle.org/ | ||
// | ||
// Moodle is free software: you can redistribute it and/or modify | ||
// it under the terms of the GNU General Public License as published by | ||
// the Free Software Foundation, either version 3 of the License, or | ||
// (at your option) any later version. | ||
// | ||
// Moodle is distributed in the hope that it will be useful, | ||
// but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
// GNU General Public License for more details. | ||
// | ||
// You should have received a copy of the GNU General Public License | ||
// along with Moodle. If not, see <http://www.gnu.org/licenses/>. | ||
|
||
namespace MoodleCodeSniffer\moodle\Sniffs\PHPUnit; | ||
|
||
// phpcs:disable moodle.NamingConventions | ||
|
||
use PHP_CodeSniffer\Sniffs\Sniff; | ||
use PHP_CodeSniffer\Files\File; | ||
use PHP_CodeSniffer\Util\Tokens; | ||
use MoodleCodeSniffer\moodle\Util\MoodleUtil; | ||
|
||
/** | ||
* Checks that a test file has the @coversxxx annotations properly defined. | ||
* | ||
* @package local_codechecker | ||
* @copyright 2022 onwards Eloy Lafuente (stronk7) {@link https://stronk7.com} | ||
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later | ||
*/ | ||
class TestCaseCoversSniff implements Sniff { | ||
|
||
/** | ||
* Register for open tag (only process once per file). | ||
*/ | ||
public function register() { | ||
return array(T_OPEN_TAG); | ||
} | ||
|
||
/** | ||
* Processes php files and perform various checks with file. | ||
* | ||
* @param File $file The file being scanned. | ||
* @param int $pointer The position in the stack. | ||
*/ | ||
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); | ||
|
||
// We have all we need from core, let's start processing the file. | ||
|
||
// Get the file tokens, for ease of use. | ||
$tokens = $file->getTokens(); | ||
|
||
// In various places we are going to ignore class/method prefixes (private, abstract...) | ||
// and whitespace, create an array for all them. | ||
$skipTokens = Tokens::$methodPrefixes + [T_WHITESPACE => T_WHITESPACE]; | ||
|
||
// We only want to do this once per file. | ||
$prevopentag = $file->findPrevious(T_OPEN_TAG, $pointer - 1); | ||
if ($prevopentag !== false) { | ||
return; | ||
} | ||
|
||
// If we aren't checking Moodle 4.0dev (400) and up, nothing to check. | ||
if (isset($moodleBranch) && $moodleBranch < 400) { | ||
// Make and exception for codechecker phpunit tests, so they are run always. | ||
if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) { | ||
return; | ||
} | ||
} | ||
|
||
// If the file isn't under tests directory, nothing to check. | ||
if (strpos($file->getFilename(), '/tests/') === false) { | ||
return; | ||
} | ||
|
||
// If the file isn't called, _test.php, nothing to check. | ||
$fileName = basename($file->getFilename()); | ||
if (substr($fileName, -9) !== '_test.php') { | ||
// Make an exception for codechecker own phpunit fixtures here, allowing any name for them. | ||
if (!defined('PHPUNIT_TEST') || !PHPUNIT_TEST) { | ||
return; | ||
} | ||
} | ||
|
||
// 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)) { | ||
$class = $file->getDeclarationName($cStart); | ||
$classCovers = false; // To control when the class has a @covers tag. | ||
$classCoversNothing = false; // To control when the class has a @coversNothing tag. | ||
|
||
// Only if the class is extending something. | ||
// TODO: We could add a list of valid classes once we have a class-map available. | ||
if (!$file->findNext(T_EXTENDS, $cStart + 1, $tokens[$cStart]['scope_opener'])) { | ||
continue; | ||
} | ||
|
||
// Ignore non ended "_test|_testcase" classes. | ||
if (substr($class, -5) !== '_test' && substr($class, -9) != '_testcase') { | ||
continue; | ||
} | ||
|
||
// Let's see if the class has any phpdoc block (first non skip token must be end of phpdoc comment). | ||
$docPointer = $file->findPrevious($skipTokens, $cStart - 1, null, true); | ||
|
||
// Found a phpdoc block, let's look for @covers, @coversNothing and @coversDefaultClass tags. | ||
if ($tokens[$docPointer]['code'] === T_DOC_COMMENT_CLOSE_TAG) { | ||
$docStart = $tokens[$docPointer]['comment_opener']; | ||
while ($docPointer) { // Let's look upwards, until the beginning of the phpdoc block. | ||
$docPointer = $file->findPrevious(T_DOC_COMMENT_TAG, $docPointer - 1, $docStart); | ||
if ($docPointer) { | ||
$docTag = trim($tokens[$docPointer]['content']); | ||
switch ($docTag) { | ||
case '@covers': | ||
$classCovers = $docPointer; | ||
// Validate basic syntax (FQCN or ::). | ||
$this->checkCoversTagsSyntax($file, $docPointer, '@covers'); | ||
break; | ||
case '@coversNothing': | ||
$classCoversNothing = $docPointer; | ||
// Validate basic syntax (empty). | ||
$this->checkCoversTagsSyntax($file, $docPointer, '@coversNothing'); | ||
break; | ||
case '@coversDefaultClass': | ||
// Validate basic syntax (FQCN). | ||
$this->checkCoversTagsSyntax($file, $docPointer, '@coversDefaultClass'); | ||
break; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// Both @covers and @coversNothing, that's a mistake. 2 errors. | ||
if ($classCovers && $classCoversNothing) { | ||
$file->addError('Class %s has both @covers and @coversNothing tags, good contradiction', | ||
$classCovers, 'ContradictoryClass', [$class]); | ||
$file->addError('Class %s has both @covers and @coversNothing tags, good contradiction', | ||
$classCoversNothing, 'ContradictoryClass', [$class]); | ||
} | ||
|
||
// Iterate over all the methods in the class. | ||
$mStart = $cStart; | ||
while ($mStart = $file->findNext(T_FUNCTION, $mStart + 1, $tokens[$cStart]['scope_closer'])) { | ||
$method = $file->getDeclarationName($mStart); | ||
$methodCovers = false; // To control when the method has a @covers tag. | ||
$methodCoversNothing = false; // To control when the method has a @coversNothing tag. | ||
|
||
// Ignore non test_xxxx() methods. | ||
if (strpos($method, 'test_') !== 0) { | ||
continue; | ||
} | ||
|
||
// Let's see if the method has any phpdoc block (first non skip token must be end of phpdoc comment). | ||
$docPointer = $file->findPrevious($skipTokens, $mStart - 1, null, true); | ||
|
||
// Found a phpdoc block, let's look for @covers and @coversNothing tags. | ||
if ($tokens[$docPointer]['code'] === T_DOC_COMMENT_CLOSE_TAG) { | ||
$docStart = $tokens[$docPointer]['comment_opener']; | ||
while ($docPointer) { // Let's look upwards, until the beginning of the phpdoc block. | ||
$docPointer = $file->findPrevious(T_DOC_COMMENT_TAG, $docPointer - 1, $docStart); | ||
if ($docPointer) { | ||
$docTag = trim($tokens[$docPointer]['content']); | ||
switch ($docTag) { | ||
case '@covers': | ||
$methodCovers = $docPointer; | ||
// Validate basic syntax (FQCN or ::). | ||
$this->checkCoversTagsSyntax($file, $docPointer, '@covers'); | ||
break; | ||
case '@coversNothing': | ||
$methodCoversNothing = $docPointer; | ||
// Validate basic syntax (empty). | ||
$this->checkCoversTagsSyntax($file, $docPointer, '@coversNothing'); | ||
break; | ||
case '@coversDefaultClass': | ||
// Not allowed in methods. | ||
$file->addError('Method %s() has @coversDefaultClass tag, only allowed in classes', | ||
$docPointer, 'DefaultClassNotAllowed', [$method]); | ||
break; | ||
} | ||
} | ||
} | ||
} | ||
|
||
// No @covers or @coversNothing at any level, that's a missing one. | ||
if (!$classCovers && !$classCoversNothing && !$methodCovers && !$methodCoversNothing) { | ||
$file->addWarning('Test method %s() is missing any coverage information, own or at class level', | ||
$mStart, 'Missing', [$method]); | ||
} | ||
|
||
// Both @covers and @coversNothing, that's a mistake. 2 errors. | ||
if ($methodCovers && $methodCoversNothing) { | ||
$file->addError('Method %s() has both @covers and @coversNothing tags, good contradiction', | ||
$methodCovers, 'ContradictoryMethod', [$method]); | ||
$file->addError('Method %s() has both @covers and @coversNothing tags, good contradiction', | ||
$methodCoversNothing, 'ContradictoryMethod', [$method]); | ||
} | ||
|
||
// Found @coversNothing at class, and @covers at method, strange. Warning. | ||
if ($classCoversNothing && $methodCovers) { | ||
$file->addWarning('Class %s has @coversNothing, but there are methods covering stuff', | ||
$classCoversNothing, 'ContradictoryMixed', [$class]); | ||
$file->addWarning('Test method %s() is covering stuff, but class has @coversNothing', | ||
$methodCovers, 'ContradictoryMixed', [$method]); | ||
} | ||
|
||
// Found @coversNothing at class and method, redundant. Warning. | ||
if ($classCoversNothing && $methodCoversNothing) { | ||
$file->addWarning('Test method %s() has @coversNothing, but class also has it, redundant', | ||
$methodCoversNothing, 'Redundant', [$method]); | ||
} | ||
|
||
// Advance until the end of the method, if possible, to find the next one quicker. | ||
$mStart = $tokens[$mStart]['scope_closer'] ?? $pointer + 1; | ||
} | ||
} | ||
} | ||
|
||
/** | ||
* Perform a basic syntax cheking of the values of the @coversXXX tags. | ||
* | ||
* @param File $file The file being scanned | ||
* @param int $pointer pointer to the token that contains the tag. Calculations are based on that. | ||
* @param string $tag $coversXXX tag to be checked. Verifications are different based on that. | ||
* @return void | ||
*/ | ||
protected function checkCoversTagsSyntax(File $file, int $pointer, string $tag) { | ||
// Get the file tokens, for ease of use. | ||
$tokens = $file->getTokens(); | ||
|
||
if ($tag === '@coversNothing') { | ||
// Check that there isn't whitespace and string. | ||
if ($tokens[$pointer + 1]['code'] === T_DOC_COMMENT_WHITESPACE && | ||
$tokens[$pointer + 2]['code'] === T_DOC_COMMENT_STRING) { | ||
$file->addError('Wrong %s annotation, it must be empty', | ||
$pointer, 'NotEmpty', [$tag]); | ||
} | ||
} | ||
|
||
if ($tag === '@covers' || $tag === '@coversDefaultClass') { | ||
// Check that there is whitespace and string. | ||
if ($tokens[$pointer + 1]['code'] !== T_DOC_COMMENT_WHITESPACE || | ||
$tokens[$pointer + 2]['code'] !== T_DOC_COMMENT_STRING) { | ||
$file->addError('Wrong %s annotation, it must contain some value', | ||
$pointer, 'Empty', [$tag]); | ||
// No value, nothing else to check. | ||
return; | ||
} | ||
} | ||
|
||
if ($tag === '@coversDefaultClass') { | ||
// Check that value begins with \ (FQCN). | ||
if (strpos($tokens[$pointer + 2]['content'], '\\') !== 0) { | ||
$file->addError('Wrong %s annotation, it must be FQCN (\\ prefixed)', | ||
$pointer, 'NoFQCN', [$tag]); | ||
} | ||
// Check that value does not contain :: (method). | ||
if (strpos($tokens[$pointer + 2]['content'], '::') !== false) { | ||
$file->addError('Wrong %s annotation, cannot point to a method (contains ::)', | ||
$pointer, 'WrongMethod', [$tag]); | ||
} | ||
} | ||
|
||
if ($tag === '@covers') { | ||
// Check value begins with \ (FQCN) or :: (method). | ||
if (strpos($tokens[$pointer + 2]['content'], '\\') !== 0 && | ||
strpos($tokens[$pointer + 2]['content'], '::') !== 0) { | ||
$file->addError('Wrong %s annotation, it must be FQCN (\\ prefixed) or point to method (:: prefixed)', | ||
$pointer, 'NoFQCNOrMethod', [$tag]); | ||
} | ||
} | ||
} | ||
} |
18 changes: 18 additions & 0 deletions
18
moodle/tests/fixtures/phpunit/testcasecovers_contradiction.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
<?php | ||
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. | ||
|
||
// Covering nothing and something is a contradiction. | ||
|
||
/** | ||
* @covers ::something | ||
* @coversNothing | ||
*/ | ||
class contradiction_test extends base_test { | ||
/** | ||
* @coversNothing | ||
* @covers ::something | ||
*/ | ||
public function test_something() { | ||
// Nothing to test. | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
<?php | ||
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. | ||
|
||
// A class with 3 methods, using all the covers options correctly. | ||
|
||
/** | ||
* @coversDefaultClass \some\namespace\one | ||
* @covers ::all() | ||
*/ | ||
class correct_test extends base_test { | ||
/** | ||
* @covers ::one() | ||
*/ | ||
public function test_one() { | ||
// Nothing to test. | ||
} | ||
|
||
/** | ||
* @covers ::two() | ||
* @covers \some\namespace\two::two() | ||
*/ | ||
public function test_two() { | ||
// Nothing to test. | ||
} | ||
|
||
/** | ||
* @coversNothing | ||
*/ | ||
public function test_three() { | ||
// Nothing to test. | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
<?php | ||
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. | ||
|
||
// Wrong @covers. | ||
|
||
/** | ||
* @covers ::ok | ||
* @covers \ok | ||
* @covers wrong | ||
* @covers | ||
*/ | ||
class covers_test extends base_test { | ||
/** | ||
* @covers ::ok | ||
* @covers \ok | ||
* @covers \ok::ok | ||
* @covers wrong | ||
* @covers | ||
*/ | ||
public function test_something() { | ||
// Nothing to test. | ||
} | ||
} |
22 changes: 22 additions & 0 deletions
22
moodle/tests/fixtures/phpunit/testcasecovers_coversdefaultclass.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
i<?php | ||
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. | ||
|
||
// Wrong @coversDefaultClass. | ||
|
||
/** | ||
* @coversDefaultClass \ok | ||
* @coversDefaultClass wrong | ||
* @coversDefaultClass \wrong::wrong | ||
* @coversDefaultClass | ||
*/ | ||
class coversdefaultclass_test extends base_test { | ||
/** | ||
* @coversDefaultClass ::wrong | ||
* @coversDefaultClass wrong | ||
* @coversDefaultClass | ||
* @covers ::something | ||
*/ | ||
public function test_something() { | ||
// Nothing to test. | ||
} | ||
} |
16 changes: 16 additions & 0 deletions
16
moodle/tests/fixtures/phpunit/testcasecovers_coversnothing.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<?php | ||
defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. | ||
|
||
// Wrong @coversNothing. | ||
|
||
/** | ||
* @coversNothing wrong | ||
*/ | ||
class coversnothing_test extends base_test { | ||
/** | ||
* @coversNothing wrong | ||
*/ | ||
public function test_something() { | ||
// Nothing to test. | ||
} | ||
} |
Oops, something went wrong.