From: Bartosz Dziewoński <matma.rex@gmail.com>
Date: Mon, 13 Nov 2017 18:42:33 +0000 (+0100)
Subject: ApiOptionsTest: Do not use ->at()
X-Git-Tag: 1.31.0-rc.0~1496
X-Git-Url: http://git.cyclocoop.org/fichier?a=commitdiff_plain;h=f1ca6b8ca9c2dc65014bc8ae79f02158d2dc52c3;p=lhc%2Fweb%2Fwiklou.git

ApiOptionsTest: Do not use ->at()

Quoting PHPUnit docs:

  The $index parameter for the at() matcher refers to the index,
  starting at zero, in all method invocations for a given mock object.
  Exercise caution when using this matcher as it can lead to brittle
  tests which are too closely tied to specific implementation details.

Indeed these test cases would break horribly with unintuitive error
messages ("Mocked method does not exist") if anything in preferences
or API code called any additional methods on the mocked user. For
example, it relied on the caching in Preferences::getPreferences(),
which is being removed in I92390120a16448383a25e9ba2dd35a434a2f21bf.

I'm pretty sure all that matters here is that all the setOption()
calls with different arguments happen, so let's test just that.

Change-Id: I30a814151a006e5f147eebb918344049807b2b97
---

diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php b/tests/phpunit/includes/api/ApiOptionsTest.php
index ef70626120..7e45f4da12 100644
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -278,26 +278,13 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->mUserMock->expects( $this->never() )
 			->method( 'resetOptions' );
 
-		$this->mUserMock->expects( $this->at( 2 ) )
-			->method( 'getOptions' );
-
-		$this->mUserMock->expects( $this->at( 5 ) )
+		$this->mUserMock->expects( $this->exactly( 3 ) )
 			->method( 'setOption' )
-			->with( $this->equalTo( 'willBeNull' ), $this->identicalTo( null ) );
-
-		$this->mUserMock->expects( $this->at( 6 ) )
-			->method( 'getOptions' );
-
-		$this->mUserMock->expects( $this->at( 7 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) );
-
-		$this->mUserMock->expects( $this->at( 8 ) )
-			->method( 'getOptions' );
-
-		$this->mUserMock->expects( $this->at( 9 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
+			->withConsecutive(
+				[ $this->equalTo( 'willBeNull' ), $this->identicalTo( null ) ],
+				[ $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) ],
+				[ $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) ]
+			);
 
 		$this->mUserMock->expects( $this->once() )
 			->method( 'saveSettings' );
@@ -315,19 +302,12 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->mUserMock->expects( $this->once() )
 			->method( 'resetOptions' );
 
-		$this->mUserMock->expects( $this->at( 5 ) )
-			->method( 'getOptions' );
-
-		$this->mUserMock->expects( $this->at( 6 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
-
-		$this->mUserMock->expects( $this->at( 7 ) )
-			->method( 'getOptions' );
-
-		$this->mUserMock->expects( $this->at( 8 ) )
+		$this->mUserMock->expects( $this->exactly( 2 ) )
 			->method( 'setOption' )
-			->with( $this->equalTo( 'name' ), $this->equalTo( 'value' ) );
+			->withConsecutive(
+				[ $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) ],
+				[ $this->equalTo( 'name' ), $this->equalTo( 'value' ) ]
+			);
 
 		$this->mUserMock->expects( $this->once() )
 			->method( 'saveSettings' );
@@ -348,21 +328,14 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
 		$this->mUserMock->expects( $this->never() )
 			->method( 'resetOptions' );
 
-		$this->mUserMock->expects( $this->at( 4 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'testmultiselect-opt1' ), $this->identicalTo( true ) );
-
-		$this->mUserMock->expects( $this->at( 5 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'testmultiselect-opt2' ), $this->identicalTo( null ) );
-
-		$this->mUserMock->expects( $this->at( 6 ) )
-			->method( 'setOption' )
-			->with( $this->equalTo( 'testmultiselect-opt3' ), $this->identicalTo( false ) );
-
-		$this->mUserMock->expects( $this->at( 7 ) )
+		$this->mUserMock->expects( $this->exactly( 4 ) )
 			->method( 'setOption' )
-			->with( $this->equalTo( 'testmultiselect-opt4' ), $this->identicalTo( false ) );
+			->withConsecutive(
+				[ $this->equalTo( 'testmultiselect-opt1' ), $this->identicalTo( true ) ],
+				[ $this->equalTo( 'testmultiselect-opt2' ), $this->identicalTo( null ) ],
+				[ $this->equalTo( 'testmultiselect-opt3' ), $this->identicalTo( false ) ],
+				[ $this->equalTo( 'testmultiselect-opt4' ), $this->identicalTo( false ) ]
+			);
 
 		$this->mUserMock->expects( $this->once() )
 			->method( 'saveSettings' );