Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

(bug 42638) Fix API action=options&reset=1 & unit tests

Browse source 
Change I98df55f2 broke action=options&reset=1, causing it to return an
error "No changes were requested" rather than resetting the options as
it should. Unfortunately, that change also broke the unit test that
would have caught this regression.

This changeset fixes the bug and the unit tests.

Change-Id: I7fe63640d54efab4572538e9d08f5b75c61243a4
This commit is contained in:
Brad Jorsch 2012-12-02 22:40:55 -05:00
parent abdda85097
commit fb7c95f567
2 changed files with 58 additions and 20 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
includes/api/ApiOptions.php
View file

@ -69,7 +69,7 @@ class ApiOptions extends ApiBase {
$newValue = isset( $params['optionvalue'] ) ? $params['optionvalue'] : null;
$changes[$params['optionname']] = $newValue;
}
if ( !count( $changes ) ) {
if ( !$changed && !count( $changes ) ) {
$this->dieUsage( 'No changes were requested', 'nochanges' );
}

76
tests/phpunit/includes/api/ApiOptionsTest.php
View file

@ -2,11 +2,14 @@
/**
* @group API
* @group Database
*/
class ApiOptionsTest extends MediaWikiLangTestCase {
private $mTested, $mUserMock, $mContext, $mSession;
private $mOldGetPreferencesHooks = false;
private static $Success = array( 'options' => 'success' );
protected function setUp() {
@ -16,8 +19,13 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
->disableOriginalConstructor()
->getMock();
// Set up groups
$this->mUserMock->expects( $this->any() )
->method( 'getEffectiveGroups' )->will( $this->returnValue( array( '*', 'user')) );
// Create a new context
$this->mContext = new DerivativeContext( new RequestContext() );
$this->mContext->getContext()->setTitle( Title::newFromText( 'Test' ) );
$this->mContext->setUser( $this->mUserMock );
$main = new ApiMain( $this->mContext );
@ -26,6 +34,36 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->mSession = array();
$this->mTested = new ApiOptions( $main, 'options' );
global $wgHooks;
if ( !isset( $wgHooks['GetPreferences'] ) ) {
$wgHooks['GetPreferences'] = array();
}
$this->mOldGetPreferencesHooks = $wgHooks['GetPreferences'];
$wgHooks['GetPreferences'][] = array( $this, 'hookGetPreferences' );
}
protected function tearDown() {
global $wgHooks;
if ( $this->mOldGetPreferencesHooks !== false ) {
$wgHooks['GetPreferences'] = $this->mOldGetPreferencesHooks;
$this->mOldGetPreferencesHooks = false;
}
parent::tearDown();
}
public function hookGetPreferences( $user, &$preferences ) {
foreach ( array( 'name', 'willBeNull', 'willBeEmpty', 'willBeHappy' ) as $k ) {
$preferences[$k] = array(
'type' => 'text',
'section' => 'test',
'label' => ' ',
);
}
return true;
}
private function getSampleRequest( $custom = array() ) {
@ -105,9 +143,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->fail( "UsageException was not thrown" );
}
/**
* @group Broken
*/
public function testReset() {
$this->mUserMock->expects( $this->once() )
->method( 'resetOptions' );
@ -125,9 +160,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response );
}
/**
* @group Broken
*/
public function testOptionWithValue() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
@ -146,9 +178,6 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response );
}
/**
* @group Broken
*/
public function testOptionResetValue() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
@ -166,22 +195,28 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response );
}
/**
* @group Broken
*/
public function testChange() {
$this->mUserMock->expects( $this->never() )
->method( 'resetOptions' );
$this->mUserMock->expects( $this->at( 1 ) )
->method( 'setOption' )
->with( $this->equalTo( 'willBeNull' ), $this->equalTo( null ) );
->method( 'getOptions' );
$this->mUserMock->expects( $this->at( 2 ) )
->method( 'setOption' )
->with( $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) );
->with( $this->equalTo( 'willBeNull' ), $this->equalTo( null ) );
$this->mUserMock->expects( $this->at( 3 ) )
->method( 'getOptions' );
$this->mUserMock->expects( $this->at( 4 ) )
->method( 'setOption' )
->with( $this->equalTo( 'willBeEmpty' ), $this->equalTo( '' ) );
$this->mUserMock->expects( $this->at( 5 ) )
->method( 'getOptions' );
$this->mUserMock->expects( $this->at( 6 ) )
->method( 'setOption' )
->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
@ -195,18 +230,21 @@ class ApiOptionsTest extends MediaWikiLangTestCase {
$this->assertEquals( self::$Success, $response );
}
/**
* @group Broken
*/
public function testResetChangeOption() {
$this->mUserMock->expects( $this->once() )
->method( 'resetOptions' );
$this->mUserMock->expects( $this->at( 2 ) )
->method( 'getOptions' );
$this->mUserMock->expects( $this->at( 3 ) )
->method( 'setOption' )
->with( $this->equalTo( 'willBeHappy' ), $this->equalTo( 'Happy' ) );
$this->mUserMock->expects( $this->at( 3 ) )
$this->mUserMock->expects( $this->at( 4 ) )
->method( 'getOptions' );
$this->mUserMock->expects( $this->at( 5 ) )
->method( 'setOption' )
->with( $this->equalTo( 'name' ), $this->equalTo( 'value' ) );