Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/77864
Change subject: Refactor some assertions in ModifyEntityTestBase
......................................................................
Refactor some assertions in ModifyEntityTestBase
Change-Id: Ie38b1ccfd7bec1f2cd49164bfc23f627d3f6b812
This makes reading tests easier as well as removing
some uplicate assertions that are not needed
---
M repo/tests/phpunit/includes/api/EditEntityTest.php
M repo/tests/phpunit/includes/api/GetEntitiesTest.php
M repo/tests/phpunit/includes/api/LangAttributeBase.php
M repo/tests/phpunit/includes/api/ModifyEntityTestBase.php
M repo/tests/phpunit/includes/api/SetAliasesTest.php
M repo/tests/phpunit/includes/api/SetSiteLinkTest.php
6 files changed, 102 insertions(+), 55 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/64/77864/1
diff --git a/repo/tests/phpunit/includes/api/EditEntityTest.php
b/repo/tests/phpunit/includes/api/EditEntityTest.php
index b49a5f3..60bc354 100644
--- a/repo/tests/phpunit/includes/api/EditEntityTest.php
+++ b/repo/tests/phpunit/includes/api/EditEntityTest.php
@@ -141,7 +141,8 @@
self::$users['wbeditor']->user
);
- $this->assertSuccess( $res, 'entity', 'id' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'id' );
$this->assertRegExp( '/^p\d+$/',
$res['entity']['id'],
'Expected a qualfied property ID with prefix' );
@@ -163,7 +164,8 @@
self::$users['wbeditor']->user
);
- $this->assertSuccess( $res, 'entity', 'id' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'id' );
$this->assertEntityEquals( self::$expect, $res['entity'] );
$this->assertRegExp( '/^q\d+$/',
$res['entity']['id'],
@@ -216,11 +218,13 @@
self::$users['wbeditor']->user
);
- $this->assertSuccess( $res, 'entity', 'id' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'id' );
//NOTE: lastrevid should be here even though the edit didn't
create a new revision,
// because the content stayed the same.
- $this->assertSuccess( $res, 'entity', 'lastrevid' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'lastrevid' );
$this->assertEntityEquals( self::$expect, $res['entity'] );
}
@@ -271,7 +275,8 @@
null,
self::$users['wbeditor']->user
);
- $this->assertSuccess( $query, 'entities', self::$id, 'id' );
+ $this->assertResultSuccess( $query );
+ $this->assertResultHasKeyInPath( $query, 'entities', self::$id,
'id' );
// these sets of failing data must be merged with an existing
entity
$goodData = array(
@@ -293,7 +298,8 @@
null,
self::$users['wbeditor']->user
);
- $this->assertSuccess( $res, 'entity', 'id' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res,
'entity', 'id' );
$this->assertEntityEquals( self::$expect,
$res['entity'] );
}
catch ( \UsageException $e ) {
@@ -318,7 +324,8 @@
null,
self::$users['wbeditor']->user
);
- $this->assertSuccess( $res, 'entity', 'id' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'id' );
$this->assertEntityEquals( array( 'id' => self::$id ),
$res['entity'] );
}
catch ( \UsageException $e ) {
@@ -555,10 +562,11 @@
);
// check returned keys
-------------------------------------------
- $this->assertSuccess( $res, 'entity' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entity' );
$entity = $res['entity'];
- $this->assertSuccess( $res, 'entity', 'id' );
- $this->assertSuccess( $res, 'entity', 'lastrevid' );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'id' );
+ $this->assertResultHasKeyInPath( $res, 'entity', 'lastrevid' );
// check return value
-------------------------------------------
$this->assertEquals( \Wikibase\Item::ENTITY_TYPE,
$res['entity']['type'] );
diff --git a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
index fe59043..1437389 100644
--- a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
+++ b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
@@ -77,26 +77,32 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
$this->assertEntityEquals( $item, $res['entities'][$id] );
$this->assertEquals( 1, count( $res['entities'] ), "requesting
a single item should return exactly one item entry" );
// This should be correct for all items we are testing
$this->assertEquals( \Wikibase\Item::ENTITY_TYPE,
$res['entities'][$id]['type'] );
// The following comes from the props=info which is included by
default
// Only check if they are there and seems valid, can't do much
more for the moment (or could for title but then we are testing assumptions)
- $this->assertSuccess( $res, 'entities', $id, 'pageid' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'pageid' );
$this->assertTrue( is_integer( $res['entities'][$id]['pageid']
) );
$this->assertTrue( 0 < $res['entities'][$id]['pageid'] );
- $this->assertSuccess( $res, 'entities', $id, 'ns' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id, 'ns' );
$this->assertTrue( is_integer( $res['entities'][$id]['ns'] ) );
$this->assertTrue( 0 <= $res['entities'][$id]['ns'] );
- $this->assertSuccess( $res, 'entities', $id, 'title' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id, 'title'
);
$this->assertTrue( is_string( $res['entities'][$id]['title'] )
);
$this->assertTrue( 0 < strlen( $res['entities'][$id]['title'] )
);
- $this->assertSuccess( $res, 'entities', $id, 'lastrevid' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'lastrevid' );
$this->assertTrue( is_integer(
$res['entities'][$id]['lastrevid'] ) );
$this->assertTrue( 0 < $res['entities'][$id]['lastrevid'] );
- $this->assertSuccess( $res, 'entities', $id, 'modified' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'modified' );
$this->assertTrue( is_string( $res['entities'][$id]['modified']
) );
$this->assertTrue( 0 < strlen(
$res['entities'][$id]['modified'] ) );
$this->assertRegExp(
'/^(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})Z$/',
@@ -131,7 +137,8 @@
'ids' => $ids )
);
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
$this->assertEntityEquals( $item, $res['entities'][$id] );
$this->assertEquals( 1, count( $res['entities'] ) );
}
@@ -172,7 +179,8 @@
$item = $this->getEntityOutput( $handle );
$id = $item['id'];
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
$this->assertEntityEquals( $item, $res['entities'][$id] );
$this->assertEquals( 1, count( $res['entities'] ), "requesting
a single item should return exactly one item entry" );
}
@@ -196,7 +204,8 @@
$item = $this->getEntityOutput( $handle );
$id = $item['id'];
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
$this->assertEntityEquals( $item, $res['entities'][$id] );
$this->assertEquals( 1, count( $res['entities'] ), "requesting
a single item should return exactly one item entry" );
@@ -286,7 +295,8 @@
'ids' => $badid,
) );
- $this->assertSuccess( $res, 'entities', $badid, 'missing' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $badid,
'missing' );
$this->assertEquals( 1, count( $res['entities'] ), "requesting
a single item should return exactly one item entry" );
}
@@ -332,12 +342,14 @@
'titles' => 'klaijehrnqowienxcqopweiu',
) );
- $this->assertSuccess( $res, 'entities' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities' );
$keys = array_keys( $res['entities'] );
$this->assertEquals( 1, count( $keys ), "requesting a single
item should return exactly one item entry" );
- $this->assertSuccess( $res, 'entities', $keys[0], 'missing' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $keys[0],
'missing' );
}
/**
@@ -357,12 +369,14 @@
'normalize' => true
) );
- $this->assertSuccess( $res, 'entities' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities' );
$keys = array_keys( $res['entities'] );
$this->assertEquals( 1, count( $keys ), "requesting a single
item should return exactly one item entry" );
- $this->assertSuccess( $res, 'entities', $keys[0], 'missing' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $keys[0],
'missing' );
// The normalization that has been applied should be noted
$this->assertEquals(
@@ -392,7 +406,8 @@
'ids' => join( '|', $ids ),
) );
- $this->assertSuccess( $res, 'entities' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities' );
$this->assertEquals( count( $ids ), count( $res['entities'] ),
"the actual number of items differs from the number of requested items" );
foreach ( $ids as $id ) {
@@ -418,7 +433,8 @@
'titles' => join( '|', $titles )
) );
- $this->assertSuccess( $res, 'entities' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities' );
$this->assertEquals( count( $titles ), count( $res['entities']
), "the actual number of items differs from the number of requested items" );
foreach ( $handles as $handle ) {
@@ -455,7 +471,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
$item = $res['entities'][$id];
@@ -509,7 +526,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id );
if ( $expectedProps === false ) {
$this->assertArrayHasKey( 'warnings', $res );
@@ -541,7 +559,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id, 'sitelinks' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'sitelinks' );
foreach ( $res['entities'][$id]['sitelinks'] as $link ) {
$this->assertArrayNotHasKey( 'url', $link );
@@ -556,7 +575,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id, 'sitelinks' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'sitelinks' );
foreach ( $res['entities'][$id]['sitelinks'] as $link ) {
$this->assertArrayHasKey( 'url', $link, "SiteLinks in
the result must have the 'url' key set!" );
@@ -588,7 +608,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id, 'sitelinks' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'sitelinks' );
$last = '';
foreach ( $res['entities'][$id]['sitelinks'] as $link ) {
$this->assertArrayHasKey( 'site', $link );
@@ -607,7 +628,8 @@
'ids' => $id )
);
- $this->assertSuccess( $res, 'entities', $id, 'sitelinks' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id,
'sitelinks' );
$last = 'zzzzzzzz';
foreach ( $res['entities'][$id]['sitelinks'] as $link ) {
$this->assertArrayHasKey( 'site', $link );
@@ -633,7 +655,8 @@
);
if ( $usekeys ) {
- $this->assertSuccess( $res, 'entities', $id );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities', $id
);
foreach ( array( 'sitelinks' => 'site', 'alias' =>
false, 'labels' => 'language', 'descriptions' => 'language' ) as $prop =>
$field) {
if ( array_key_exists( $prop,
$res['entities'][$id] ) ) {
foreach ( $res['entities'][$id][$prop]
as $key => $value ) {
@@ -648,7 +671,8 @@
}
}
else {
- $this->assertSuccess( $res, 'entities' );
+ $this->assertResultSuccess( $res );
+ $this->assertResultHasKeyInPath( $res, 'entities' );
$keys = array_keys( $res['entities'] );
$n = $keys[0];
diff --git a/repo/tests/phpunit/includes/api/LangAttributeBase.php
b/repo/tests/phpunit/includes/api/LangAttributeBase.php
index 7ca28be..49a279f 100644
--- a/repo/tests/phpunit/includes/api/LangAttributeBase.php
+++ b/repo/tests/phpunit/includes/api/LangAttributeBase.php
@@ -103,7 +103,7 @@
}
}
- $this->assertSuccess( $apiResponse );
+ $this->assertResultSuccess( $apiResponse );
$item = $apiResponse['entity'];
$section = "{$attr}s";
diff --git a/repo/tests/phpunit/includes/api/ModifyEntityTestBase.php
b/repo/tests/phpunit/includes/api/ModifyEntityTestBase.php
index 54d3d86..e931f03 100644
--- a/repo/tests/phpunit/includes/api/ModifyEntityTestBase.php
+++ b/repo/tests/phpunit/includes/api/ModifyEntityTestBase.php
@@ -638,15 +638,41 @@
/**
* Asserts that the given API response represents a successful call.
- * Optionally, also asserts the existence of some path in the result,
represented by any additional parameters.
*
* @param array $response
- * @param string $path1 first path element (optional)
- * @param string $path2 second path element (optional)
- * @param ...
*/
- public function assertSuccess( $response ) {
+ public function assertResultSuccess( $response ) {
$this->assertArrayHasKey( 'success', $response, "Missing
'success' marker in response." );
+ $this->assertResultHasEntityType( $response );
+ }
+
+ /**
+ * Asserts the existence of some path in the result, represented by any
additional parameters.
+ * @param array $response
+ * @param param string $path1 first path element (optional)
+ * @param param string $path2 second path element (optional)
+ * @param param $ ...
+ */
+ public function assertResultHasKeyInPath( $response ){
+ $path = func_get_args();
+ array_shift( $path );
+
+ $obj = $response;
+ $p = '/';
+
+ foreach ( $path as $key ) {
+ $this->assertArrayHasKey( $key, $obj, "Expected key
$key under path $p in the response." );
+
+ $obj = $obj[ $key ];
+ $p .= "/$key";
+ }
+ }
+
+ /**
+ * Asserts that the given API response has a valid entity type if the
result contains an entity
+ * @param array $response
+ */
+ public function assertResultHasEntityType( $response ){
if ( isset( $response['entity'] ) ) {
if ( isset( $response['entity']['type'] ) ) {
@@ -661,18 +687,6 @@
}
}
- $path = func_get_args();
- array_shift( $path );
-
- $obj = $response;
- $p = '/';
-
- foreach ( $path as $key ) {
- $this->assertArrayHasKey( $key, $obj, "Expected key
$key under path $p in the response." );
-
- $obj = $obj[ $key ];
- $p .= "/$key";
- }
}
/**
diff --git a/repo/tests/phpunit/includes/api/SetAliasesTest.php
b/repo/tests/phpunit/includes/api/SetAliasesTest.php
index 30e7a2b..33cda78 100644
--- a/repo/tests/phpunit/includes/api/SetAliasesTest.php
+++ b/repo/tests/phpunit/includes/api/SetAliasesTest.php
@@ -99,11 +99,12 @@
list( $apiResponse,, ) = $this->doApiRequestWithToken( $req,
null, self::$users['wbeditor']->user );
- $this->assertSuccess( $apiResponse );
+ $this->assertResultSuccess( $apiResponse );
// check return value
--------------------------------------------------
if ( $expected ) {
- $this->assertSuccess( $apiResponse, 'entity', 'aliases'
);
+ $this->assertResultSuccess( $apiResponse );
+ $this->assertResultHasKeyInPath( $apiResponse,
'entity', 'aliases' );
$aliases = self::flattenArray(
$apiResponse['entity']['aliases'], 'language', 'value', true );
$actual = isset( $aliases[ $langCode ] ) ? $aliases[
$langCode ] : array();
diff --git a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
index c00275c..731eff4 100644
--- a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
@@ -184,7 +184,7 @@
}
// check the response -------------------------------
- //$this->assertSuccess( $res, 'entity', 'sitelinks', 0 );
+ //$this->assertResultSuccess( $res, 'entity', 'sitelinks', 0 );
if ( $expectedTitle !== false ) {
$this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
}
--
To view, visit https://gerrit.wikimedia.org/r/77864
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie38b1ccfd7bec1f2cd49164bfc23f627d3f6b812
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
