Addshore has uploaded a new change for review. https://gerrit.wikimedia.org/r/78672
Change subject: Refactor BotEditTest ...................................................................... Refactor BotEditTest Refactored and added tests for more of the modules There are a few todos in the file including tests for claims which should be added after refactoring their tests. Change-Id: I1cecd98df7edb85a2cf8411e55a601e9278b57e5 --- M repo/tests/phpunit/includes/api/BotEditTest.php 1 file changed, 87 insertions(+), 81 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/72/78672/2 diff --git a/repo/tests/phpunit/includes/api/BotEditTest.php b/repo/tests/phpunit/includes/api/BotEditTest.php index c5826a4..2bf278a 100644 --- a/repo/tests/phpunit/includes/api/BotEditTest.php +++ b/repo/tests/phpunit/includes/api/BotEditTest.php @@ -11,10 +11,6 @@ * This testset only checks the validity of the calls and correct handling of tokens and users. * Note that we creates an empty database and then starts manipulating testusers. * - * BE WARNED: the tests depend on execution order of the methods and the methods are interdependent, - * so stuff will fail if you move methods around or make changes to them without matching changes in - * all methods depending on them. - * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -40,6 +36,7 @@ * @author John Erling Blad < jeb...@gmail.com > * @author Daniel Kinzler < daniel.kinz...@wikimedia.de > * @author Anja Jentzsch < anja.jentz...@wikimedia.de > + * @author Adam Shorland * * @group API * @group Wikibase @@ -58,11 +55,15 @@ */ class BotEditTest extends WikibaseApiTestCase { - protected static $baseOfItemIds; + private static $hasSetup; public function setUp() { global $wgUser; parent::setUp(); + + if( !isset( self::$hasSetup ) ){ + $this->initTestEntities( array( 'Empty' ) ); + } ApiTestCase::$users['wbbot'] = new TestUser( 'Apitestbot', @@ -78,6 +79,9 @@ /** * This initiates a cascade that fails if there are no * production-like environment + * + * WARNING: This should always be run before any other tests that depend on it... + * * @group API */ function testTokensAndRights() { @@ -93,84 +97,93 @@ } } - /** - * @group API - * @depends testTokensAndRights - */ - function testSetItemTop() { - $req = array( - 'action' => 'wbeditentity', - 'summary' => 'Some reason', - 'data' => '{}', - 'new' => 'item', + public static function provideData() { + return array( + array(//0 + 'p' => array( 'handle' => 'Empty', 'bot' => '', 'action' => 'wbsetlabel', 'language' => 'en', 'value' => 'ALabel' ), + 'e' => array( 'bot' => true ) ), + array(//1 + 'p' => array( 'handle' => 'Empty', 'action' => 'wbsetlabel', 'language' => 'en', 'value' => 'ALabel2' ), + 'e' => array( 'bot' => false ) ), + array(//2 + 'p' => array( 'handle' => 'Empty', 'bot' => '', 'action' => 'wbsetdescription', 'language' => 'de', 'value' => 'ADesc' ), + 'e' => array( 'bot' => true ) ), + array(//3 + 'p' => array( 'handle' => 'Empty', 'action' => 'wbsetdescription', 'language' => 'de', 'value' => 'ADesc2' ), + 'e' => array( 'bot' => false ) ), + array(//4 + 'p' => array( 'handle' => 'Empty', 'bot' => '', 'action' => 'wbsetaliases', 'language' => 'de', 'set' => 'ali1' ), + 'e' => array( 'bot' => true ) ), + array(//5 + 'p' => array( 'handle' => 'Empty', 'action' => 'wbsetaliases', 'language' => 'de', 'set' => 'ali2' ), + 'e' => array( 'bot' => false ) ), + array(//6 + 'p' => array( 'handle' => 'Empty', 'bot' => '', 'action' => 'wbsetsitelink', 'linksite' => 'enwiki', 'linktitle' => 'PageEn' ), + 'e' => array( 'bot' => true ) ), + array(//7 + 'p' => array( 'handle' => 'Empty', 'action' => 'wbsetsitelink', 'linksite' => 'dewiki', 'linktitle' => 'PageDe' ), + 'e' => array( 'bot' => false ) ), + array(//8 + 'p' => array( 'bot' => '', 'action' => 'wblinktitles', 'tosite' => 'enwiki', 'totitle' => 'PageEn', 'fromsite' => 'svwiki', 'fromtitle' => 'SvPage' ), + 'e' => array( 'bot' => true ) ), + array(//9 + 'p' => array( 'action' => 'wblinktitles', 'tosite' => 'dewiki', 'totitle' => 'PageDe', 'fromsite' => 'nowiki', 'fromtitle' => 'NoPage' ), + 'e' => array( 'bot' => false ) ), + array(//10 + 'p' => array( 'bot' => '', 'action' => 'wbeditentity', 'new' => 'item', 'data' => '{}' ), + 'e' => array( 'bot' => true, 'new' => true ) ), + array(//11 + 'p' => array( 'action' => 'wbeditentity', 'new' => 'item', 'data' => '{}' ), + 'e' => array( 'bot' => false, 'new' => true ) ), + //todo claims, references, qualifiers ); - - $second = $this->doApiRequestWithToken( $req, null, self::$users['wbbot']->user ); - - self::$baseOfItemIds = preg_replace( '/^[^\d]+/', '', $second[0]['entity']['id'] ); - - $this->assertArrayHasKey( 'success', $second[0], - "Must have an 'success' key in the second result from the API" ); - $this->assertArrayHasKey( 'entity', $second[0], - "Must have an 'entity' key in the second result from the API" ); - $this->assertArrayHasKey( 'id', $second[0]['entity'], - "Must have an 'id' key in the 'entity' from the second result from the API" ); } /** - * @group API + * @dataProvider provideData * @depends testTokensAndRights - * @dataProvider providerCreateItem */ - function testCreateItem( $handle, $bot, $new, $data ) { - $req = array( - 'action' => 'wbeditentity', - 'summary' => 'Some reason', - 'data' => $data, - ); - - if ( !$new ) { - $req['id'] = 'q'.self::$baseOfItemIds; - } else { - $req['new'] = 'item'; + public function testBotEdits( $params, $expected ) { + // -- do the request -------------------------------------------------- + if( array_key_exists( 'handle', $params ) ){ + $params['id'] = EntityTestHelper::getId( $params['handle'] ); + unset( $params['handle'] ); } - if ( $bot ) { - $req['bot'] = true; + list( $result,, ) = $this->doApiRequestWithToken( $params, null, self::$users['wbbot']->user ); + + // -- check the result ------------------------------------------------ + $this->assertArrayHasKey( 'success', $result, "Missing 'success' marker in response." ); + $this->assertResultHasEntityType( $result ); + $this->assertArrayHasKey( 'entity', $result, "Missing 'entity' section in response." ); + $this->assertArrayHasKey( 'lastrevid', $result['entity'] , 'entity should contain lastrevid key' ); + $myid = $result['entity']['id']; + //@todo remove crappy hack the below while fixing bug:52732 (linktitles currently doesn't return an id with a q...) + if( $params['action'] == 'wblinktitles' ){ + $myid = 'q'.$myid; } - $second = $this->doApiRequestWithToken( $req, null ,self::$users['wbbot']->user ); - - self::$baseOfItemIds = preg_replace( '/^[^\d]+/', '', $second[0]['entity']['id'] ); - $myid = 'q'.self::$baseOfItemIds; - - $this->assertArrayHasKey( 'success', $second[0], - "Must have an 'success' key in the second result from the API" ); - $this->assertArrayHasKey( 'entity', $second[0], - "Must have an 'entity' key in the second result from the API" ); - $this->assertArrayHasKey( 'id', $second[0]['entity'], - "Must have an 'id' key in the 'entity' from the second result from the API" ); - $this->assertEquals( $myid, $second[0]['entity']['id'], - "Must have the value '{$myid}' for the 'id' in the result from the API" ); - - $req = array( - 'action' => 'query', - 'list' => 'recentchanges', - 'rcprop' => 'title|flags', - 'rctoponly' => '1', - 'rclimit' => 50, // hope that no more than 50 edits where made in the last second + // -- get the recentchanges ------------------------------------------- + $rcRequest = array( + 'action' => 'query', + 'list' => 'recentchanges', + 'rcprop' => 'title|flags', + 'rctoponly' => '1', + 'rclimit' => 5, // hope that no more than 50 edits where made in the last second ); - $third = $this->doApiRequest( $req, null, false, self::$users['wbbot']->user ); + + //@todo this really makes this test slow, is there a better way? + $rcResult = $this->doApiRequest( $rcRequest, null, false, self::$users['wbbot']->user ); - $this->assertArrayHasKey( 'query', $third[0], - "Must have a 'query' key in the result from the API" ); - $this->assertArrayHasKey( 'recentchanges', $third[0]['query'], + // -- check the recent changes result --------------------------------- + $this->assertArrayHasKey( 'query', $rcResult[0], "Must have a 'query' key in the result from the API" ); + $this->assertArrayHasKey( 'recentchanges', $rcResult[0]['query'], "Must have a 'recentchanges' key in 'query' subset of the result from the API" ); //NOTE: the order of the entries in recentchanges is undefined if multiple // edits were done in the same second. $change = null; $itemNs = NamespaceUtils::getEntityNamespace( CONTENT_MODEL_WIKIBASE_ITEM ); - foreach ( $third[0]['query']['recentchanges'] as $rc ) { + foreach ( $rcResult[0]['query']['recentchanges'] as $rc ) { $title = Title::newFromText( $rc['title'] ); // XXX: strtoupper is a bit arcane, would ne nice to have a utility function for prefixed id -> title. if ( ( $title->getNamespace() == $itemNs ) && ( $title->getText() === strtoupper( $myid ) ) ) { @@ -181,21 +194,14 @@ $this->assertNotNull( $change, 'no change matching ID ' . $myid . ' found in recentchanges feed!' ); - $this->assertTrue( $new == array_key_exists( 'new', $change ), - "Must" . ( $new ? '' : ' not ' ) . "have a 'new' key in the rc-entry of the result from the API" ); - $this->assertTrue( $bot == array_key_exists( 'bot', $change ), - "Must" . ( $bot ? '' : ' not ' ) . "have a 'bot' key in the rc-entry of the result from the API" ); - } - - function providerCreateItem() { - return array( - array( 'One', false, true, "{}" ), - array( 'One', true, false, '{ "labels": { "nn": { "language": "nn", "value": "Karasjok" } } }' ), - array( 'One', false, false, '{ "descriptions": { "nn": { "language": "nn", "value": "Small place in Finnmark" } } }' ), - array( 'Two', true, true, "{}" ), - array( 'Two', true, false, '{ "labels": { "nn": { "language": "nn", "value": "Kautokeino" } } }' ), - array( 'Two', false, false, '{ "descriptions": { "nn": { "language": "nn", "value": "Small place in Finnmark" } } }' ), - ); + if( array_key_exists( 'new', $expected ) ){ + $this->assertTrue( $expected['new'] == array_key_exists( 'new', $change ), + "Must" . ( $expected['new'] ? '' : ' not ' ) . "have a 'new' key in the rc-entry of the result from the API" ); + } + if( array_key_exists( 'bot', $expected ) ){ + $this->assertTrue( $expected['bot'] == array_key_exists( 'bot', $change ), + "Must" . ( $expected['bot'] ? '' : ' not ' ) . "have a 'bot' key in the rc-entry of the result from the API" ); + } } } -- To view, visit https://gerrit.wikimedia.org/r/78672 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I1cecd98df7edb85a2cf8411e55a601e9278b57e5 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/Wikibase Gerrit-Branch: master Gerrit-Owner: Addshore <addshorew...@gmail.com> Gerrit-Reviewer: jenkins-bot _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits