ApiOptionsTest: Do not use ->at()
authorBartosz Dziewoński <matma.rex@gmail.com>
Mon, 13 Nov 2017 18:42:33 +0000 (19:42 +0100)
committerBartosz Dziewoński <matma.rex@gmail.com>
Wed, 15 Nov 2017 11:04:09 +0000 (11:04 +0000)
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

tests/phpunit/includes/api/ApiOptionsTest.php

index ef70626..7e45f4d 100644 (file)
@@ -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' );