Bartosz Dziewoński has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/391061 )

Change subject: ApiOptionsTest: Do not use ->at()
......................................................................

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
---
M tests/phpunit/includes/api/ApiOptionsTest.php
1 file changed, 18 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/61/391061/1

diff --git a/tests/phpunit/includes/api/ApiOptionsTest.php 
b/tests/phpunit/includes/api/ApiOptionsTest.php
index ef70626..7e45f4d 100644
--- a/tests/phpunit/includes/api/ApiOptionsTest.php
+++ b/tests/phpunit/includes/api/ApiOptionsTest.php
@@ -278,26 +278,13 @@
                $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 @@
                $this->mUserMock->expects( $this->once() )
                        ->method( 'resetOptions' );
 
-               $this->mUserMock->expects( $this->at( 5 ) )
-                       ->method( 'getOptions' );
-
-               $this->mUserMock->expects( $this->at( 6 ) )
+               $this->mUserMock->expects( $this->exactly( 2 ) )
                        ->method( 'setOption' )
-                       ->with( $this->equalTo( 'willBeHappy' ), 
$this->equalTo( 'Happy' ) );
-
-               $this->mUserMock->expects( $this->at( 7 ) )
-                       ->method( 'getOptions' );
-
-               $this->mUserMock->expects( $this->at( 8 ) )
-                       ->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 @@
                $this->mUserMock->expects( $this->never() )
                        ->method( 'resetOptions' );
 
-               $this->mUserMock->expects( $this->at( 4 ) )
+               $this->mUserMock->expects( $this->exactly( 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 ) )
-                       ->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' );

-- 
To view, visit https://gerrit.wikimedia.org/r/391061
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I30a814151a006e5f147eebb918344049807b2b97
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Bartosz Dziewoński <matma....@gmail.com>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to