Move parser tests back to /unit

Using `new LanguageEn()` involved a global, so use a MockObject instead.
Also fix LoggerFactory usage in Tokenizer to use DI instead.

Change-Id: I94d03f9459ab6444e239386eb96a0c2434bfe3dc
This commit is contained in:
Daimona Eaytoy 2019-09-02 10:25:56 +02:00
parent 7079151eac
commit ce8539e2a5
8 changed files with 58 additions and 39 deletions

View file

@ -171,7 +171,7 @@
"ApiAbuseLogPrivateDetails": "includes/api/ApiAbuseLogPrivateDetails.php",
"NormalizeThrottleParameters": "maintenance/normalizeThrottleParameters.php",
"AbuseFilterConsequencesTest": "tests/phpunit/AbuseFilterConsequencesTest.php",
"AbuseFilterParserTestCase": "tests/phpunit/AbuseFilterParserTestCase.php",
"AbuseFilterParserTestCase": "tests/phpunit/unit/AbuseFilterParserTestCase.php",
"FixOldLogEntries": "maintenance/fixOldLogEntries.php"
},
"ResourceModules": {

View file

@ -120,7 +120,7 @@ class AFPTreeParser {
* @return AFPSyntaxTree
*/
public function parse( $code ) : AFPSyntaxTree {
$tokenizer = new AbuseFilterTokenizer( $this->cache );
$tokenizer = new AbuseFilterTokenizer( $this->cache, $this->logger );
$this->mTokens = $tokenizer->getTokens( $code );
$this->mPos = 0;

View file

@ -392,7 +392,7 @@ class AbuseFilterParser {
*/
public function intEval( $code ) {
// Reset all class members to their default value
$tokenizer = new AbuseFilterTokenizer( $this->cache );
$tokenizer = new AbuseFilterTokenizer( $this->cache, $this->logger );
$this->mTokens = $tokenizer->getTokens( $code );
$this->mPos = 0;
$this->mShortCircuit = false;

View file

@ -1,6 +1,6 @@
<?php
use MediaWiki\Logger\LoggerFactory;
use Psr\Log\LoggerInterface;
/**
* Tokenizer for AbuseFilter rules.
@ -74,10 +74,17 @@ class AbuseFilterTokenizer {
private $cache;
/**
* @param BagOStuff $cache
* @var LoggerInterface
*/
public function __construct( BagOStuff $cache ) {
private $logger;
/**
* @param BagOStuff $cache
* @param LoggerInterface $logger
*/
public function __construct( BagOStuff $cache, LoggerInterface $logger ) {
$this->cache = $cache;
$this->logger = $logger;
}
/**
@ -102,7 +109,7 @@ class AbuseFilterTokenizer {
$this->getCacheKey( $code ),
BagOStuff::TTL_DAY,
function () use ( $code ) {
return self::tokenize( $code );
return $this->tokenize( $code );
}
);
@ -113,13 +120,13 @@ class AbuseFilterTokenizer {
* @param string $code
* @return array[]
*/
private static function tokenize( $code ) {
private function tokenize( $code ) {
$tokens = [];
$curPos = 0;
do {
$prevPos = $curPos;
$token = self::nextToken( $code, $curPos );
$token = $this->nextToken( $code, $curPos );
$tokens[ $token->pos ] = [ $token, $curPos ];
} while ( $curPos !== $prevPos );
@ -133,7 +140,7 @@ class AbuseFilterTokenizer {
* @throws AFPException
* @throws AFPUserVisibleException
*/
private static function nextToken( $code, &$offset ) {
private function nextToken( $code, &$offset ) {
$matches = [];
$start = $offset;
@ -182,8 +189,7 @@ class AbuseFilterTokenizer {
$base = $baseChar ? self::$bases[$baseChar] : 10;
if ( $base !== 10 && preg_match( self::$baseCharsRe[$base], $input ) ) {
// Only report success for now
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
$logger->info(
$this->logger->info(
'Successfully parsed a non-decimal number with new syntax. ' .
'Base: {number_base}, number: {number_input}',
[ 'number_base' => $base, 'number_input' => $input ]
@ -208,8 +214,7 @@ class AbuseFilterTokenizer {
if ( preg_match( self::$baseCharsRe[$base], $input ) ) {
if ( $base !== 10 ) {
// Old syntax, this is deprecated
$logger = LoggerFactory::getInstance( 'AbuseFilter' );
$logger->warning(
$this->logger->warning(
'Found non-decimal number. Base: {number_base}, number: {number_input}',
[ 'number_base' => $base, 'number_input' => $input ]
);

View file

@ -56,7 +56,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
* @return Generator|array
*/
public function readTests() {
$testPath = __DIR__ . "/../parserTests";
$testPath = __DIR__ . "/../../parserTests";
$testFiles = glob( $testPath . "/*.t" );
foreach ( $testFiles as $testFile ) {
@ -679,7 +679,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
$code = "if ( 1==1 ) then ( 1 ) else ( $funcCode ) end;";
// AbuseFilterParser skips the parentheses altogether, so this is not supposed to work
$parser = new AbuseFilterCachingParser(
new LanguageEn(),
$this->getLanguageMock(),
new EmptyBagOStuff(),
new NullLogger()
);
@ -925,7 +925,7 @@ class AbuseFilterParserTest extends AbuseFilterParserTestCase {
/** @var PHPUnit\Framework\MockObject\MockObject|AbuseFilterParser $mock */
$mock = $this->getMockBuilder( AbuseFilterParser::class )
->setConstructorArgs(
[ new LanguageEn(), new EmptyBagOStuff(), new NullLogger() ]
[ $this->getLanguageMock(), new EmptyBagOStuff(), new NullLogger() ]
)
->setMethods( [ 'logEmptyOperand' ] )
->getMock();

View file

@ -20,35 +20,27 @@
* @license GPL-2.0-or-later
*/
use PHPUnit\Framework\MockObject\MockObject;
/**
* Helper for parser-related tests
*/
abstract class AbuseFilterParserTestCase extends MediaWikiIntegrationTestCase {
abstract class AbuseFilterParserTestCase extends MediaWikiUnitTestCase {
/**
* @return AbuseFilterParser[]
*/
protected function getParsers() {
static $parsers = null;
if ( !$parsers ) {
// We're not interested in caching or logging; tests should call respectively setCache
// and setLogger if they want to test any of those.
$contLang = new LanguageEn();
$cache = new EmptyBagOStuff();
$logger = new \Psr\Log\NullLogger();
// We're not interested in caching or logging; tests should call respectively setCache
// and setLogger if they want to test any of those.
$contLang = $this->getLanguageMock();
$cache = new EmptyBagOStuff();
$logger = new \Psr\Log\NullLogger();
$parser = new AbuseFilterParser( $contLang, $cache, $logger );
$parser->toggleConditionLimit( false );
$cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger );
$cachingParser->toggleConditionLimit( false );
$parsers = [ $parser, $cachingParser ];
} else {
// Reset so that already executed tests don't influence new ones
$parsers[0]->resetState();
$parsers[0]->clearFuncCache();
$parsers[1]->resetState();
$parsers[1]->clearFuncCache();
}
return $parsers;
$parser = new AbuseFilterParser( $contLang, $cache, $logger );
$parser->toggleConditionLimit( false );
$cachingParser = new AbuseFilterCachingParser( $contLang, $cache, $logger );
$cachingParser->toggleConditionLimit( false );
return [ $parser, $cachingParser ];
}
/**
@ -77,4 +69,26 @@ abstract class AbuseFilterParserTestCase extends MediaWikiIntegrationTestCase {
$this->fail( "Exception $excep not thrown in $caller. Parser: $pname." );
}
}
/**
* Get a mock of LanguageEn with only the methods we need in the parser
*
* @return Language|MockObject
*/
protected function getLanguageMock() {
$lang = $this->getMockBuilder( LanguageEn::class )
->disableOriginalConstructor()
->getMock();
$lang->expects( $this->any() )
->method( 'uc' )
->willReturnCallback( function ( $x ) {
return mb_strtoupper( $x );
} );
$lang->expects( $this->any() )
->method( 'lc' )
->willReturnCallback( function ( $x ) {
return mb_strtolower( $x );
} );
return $lang;
}
}

View file

@ -119,7 +119,7 @@ class AbuseFilterTokenizerTest extends AbuseFilterParserTestCase {
*/
public function testCaching( $code ) {
$cache = new HashBagOStuff();
$tokenizer = new AbuseFilterTokenizer( $cache );
$tokenizer = new AbuseFilterTokenizer( $cache, new Psr\Log\NullLogger() );
$key = $tokenizer->getCacheKey( $code );