From 6a4a0fd013ec78037a71a3323c16835050bd1ffb Mon Sep 17 00:00:00 2001 From: Thiemo Kreuz Date: Mon, 20 Jan 2020 16:27:53 +0100 Subject: [PATCH] Replace ReferenceStack mocks with actual instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … if possible. In most cases it's possible to use the real object, and reach into it's private parts via TestingAccessWrapper. This is almost the same as using a mock, but I feel it's much more "light-weight". The main change is that there is no strict assertion any more for the number of ReferenceStack::pushInvalidRef() calls. Before this was mixed into the same array as the valid references, as elements set to "false". I think the test is as valueable as before without this extra check. If the rollback stack works or not is already covered by other tests. Change-Id: I90213557b164b3e43233a3dc393ee3f3d3d556a9 --- tests/phpunit/CiteIntegrationTest.php | 14 ++--- tests/phpunit/unit/CiteUnitTest.php | 73 ++++++++++++++------------- 2 files changed, 42 insertions(+), 45 deletions(-) diff --git a/tests/phpunit/CiteIntegrationTest.php b/tests/phpunit/CiteIntegrationTest.php index d66db6bee..40fc21524 100644 --- a/tests/phpunit/CiteIntegrationTest.php +++ b/tests/phpunit/CiteIntegrationTest.php @@ -36,14 +36,6 @@ class CiteIntegrationTest extends \MediaWikiIntegrationTestCase { global $wgCiteResponsiveReferences; $wgCiteResponsiveReferences = true; - $mockReferenceStack = $this->createMock( ReferenceStack::class ); - $mockReferenceStack->method( 'getGroups' )->willReturn( array_keys( $initialRefs ) ); - $mockReferenceStack->method( 'popGroup' )->willReturnCallback( function ( $group ) use ( - $initialRefs - ) { - return $initialRefs[$group]; - } ); - $mockErrorReporter = $this->createMock( ErrorReporter::class ); $mockErrorReporter->method( 'halfParsed' )->willReturnCallback( function ( $parser, ...$args ) { @@ -51,13 +43,17 @@ class CiteIntegrationTest extends \MediaWikiIntegrationTestCase { } ); + /** @var ReferenceStack $referenceStack */ + $referenceStack = TestingAccessWrapper::newFromObject( new ReferenceStack( $mockErrorReporter ) ); + $referenceStack->refs = $initialRefs; + $referencesFormatter = $this->createMock( ReferencesFormatter::class ); $referencesFormatter->method( 'formatReferences' )->willReturn( '' ); $cite = $this->newCite(); /** @var Cite $spy */ $spy = TestingAccessWrapper::newFromObject( $cite ); - $spy->referenceStack = $mockReferenceStack; + $spy->referenceStack = $referenceStack; $spy->errorReporter = $mockErrorReporter; $spy->referencesFormatter = $referencesFormatter; $spy->isSectionPreview = $isSectionPreview; diff --git a/tests/phpunit/unit/CiteUnitTest.php b/tests/phpunit/unit/CiteUnitTest.php index f2fd44440..025d0a9e5 100644 --- a/tests/phpunit/unit/CiteUnitTest.php +++ b/tests/phpunit/unit/CiteUnitTest.php @@ -481,11 +481,6 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { array $expectedErrors, array $expectedRefs ) { - /** @var (array|false)[] $pushedRefs Jumble of raw arguments, to roughly emulate - * ReferenceStack. - */ - $pushedRefs = []; - $mockParser = $this->createMock( Parser::class ); $mockParser->method( 'getStripState' ) ->willReturn( $this->createMock( StripState::class ) ); @@ -502,6 +497,10 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { } ); + /** @var ReferenceStack $referenceStack */ + $referenceStack = TestingAccessWrapper::newFromObject( new ReferenceStack( $mockErrorReporter ) ); + $referenceStack->refs = $initialRefs; + $mockFootnoteMarkFormatter = $this->createMock( FootnoteMarkFormatter::class ); $mockFootnoteMarkFormatter->method( 'linkRef' )->willReturn( '' ); @@ -511,33 +510,12 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { $spy->errorReporter = $mockErrorReporter; $spy->footnoteMarkFormatter = $mockFootnoteMarkFormatter; $spy->inReferencesGroup = $inReferencesGroup; - $spy->referenceStack = $this->createMock( ReferenceStack::class ); - $spy->referenceStack->method( 'getGroupRefs' )->willReturnCallback( - function ( $group ) use ( $initialRefs ) { - return $initialRefs[$group]; - } ); - $spy->referenceStack->method( 'hasGroup' )->willReturn( true ); - $spy->referenceStack->method( 'pushInvalidRef' )->willReturnCallback( - function () use ( &$pushedRefs ) { - $pushedRefs[] = false; - } - ); - $spy->referenceStack->method( 'pushRef' )->willReturnCallback( - function ( Parser $parser, StripState $stripState, ...$arguments ) use ( &$pushedRefs ) { - $pushedRefs[] = $arguments; - return [ 'name' => $arguments[1] ]; - } - ); - $spy->referenceStack->method( 'appendText' )->willReturnCallback( - function ( $group, $name, $text ) use ( &$pushedRefs ) { - $pushedRefs[] = [ 'appendText', $group, $name, $text ]; - } - ); + $spy->referenceStack = $referenceStack; $result = $spy->guardedRef( $mockParser, $text, $argv ); $this->assertSame( $expectOutput, $result ); $this->assertSame( $expectedErrors, $spy->mReferencesErrors ); - $this->assertSame( $expectedRefs, $pushedRefs ); + $this->assertSame( $expectedRefs, $referenceStack->refs ); } public function provideGuardedRef() { @@ -552,7 +530,16 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { '', [], [ - [ null, [ 'name' => 'a' ], '', 'a', null, null, null ] + '' => [ + 'a' => [ + 'count' => 0, + 'dir' => null, + 'key' => 1, + 'name' => 'a', + 'text' => null, + 'number' => 1, + ], + ], ] ], 'Empty in default references' => [ @@ -562,7 +549,7 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { [ '' => [] ], '', [ '(cite_error_references_no_key)' ], - [] + [ '' => [] ] ], 'Fallback to references group' => [ 'text', @@ -578,7 +565,9 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { '', [], [ - [ 'appendText', 'foo', 'a', 'text' ] + 'foo' => [ + 'a' => [ 'text' => 'text' ], + ], ] ], 'Successful ref' => [ @@ -591,7 +580,16 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { '', [], [ - [ 'text', [ 'name' => 'a' ], '', 'a', null, null, null ] + '' => [ + 'a' => [ + 'count' => 0, + 'dir' => null, + 'key' => 1, + 'name' => 'a', + 'text' => 'text', + 'number' => 1, + ], + ], ] ], 'Invalid ref' => [ @@ -604,7 +602,7 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { [], '(cite_error_ref_too_many_keys)', [], - [ false ] + [] ], 'Successful references ref' => [ 'text', @@ -620,7 +618,9 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { '', [], [ - [ 'appendText', '', 'a', 'text' ] + '' => [ + 'a' => [ 'text' => 'text' ], + ], ] ], 'Mismatched text in references' => [ @@ -639,7 +639,9 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { '', [], [ - [ 'appendText', '', 'a', ' (cite_error_references_duplicate_key|a)' ] + '' => [ + 'a' => [ 'text' => 'text-1 (cite_error_references_duplicate_key|a)' ], + ], ] ], ]; @@ -665,7 +667,6 @@ class CiteUnitTest extends \MediaWikiUnitTestCase { /** @var Cite $spy */ $spy = TestingAccessWrapper::newFromObject( $cite ); $spy->errorReporter = $this->createMock( ErrorReporter::class ); - $spy->referenceStack = $this->createMock( ReferenceStack::class ); $spy->guardedRef( $mockParser, 'text', [ Cite::BOOK_REF_ATTRIBUTE => 'a' ] ); }