gerrit at wikimedia
Aug 11, 2013, 12:25 PM
Post #1 of 1
(8 views)
Permalink
|
|
[MediaWiki-commits] [Gerrit] Refactor BotEditTest - change (mediawiki...Wikibase)
|
|
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 < jeblad [at] gmail >
* @author Daniel Kinzler < daniel.kinzler [at] wikimedia >
* @author Anja Jentzsch < anja.jentzsch [at] wikimedia >
+ * @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 <addshorewiki [at] gmail>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits [at] lists
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
|
