jenkins-bot has submitted this change and it was merged.
Change subject: Tidy up api tests involving exceptions
......................................................................
Tidy up api tests involving exceptions
Change-Id: Ic0e5457f7774cfbd9e01f1f692a33eb4d101c1be
---
M repo/tests/phpunit/includes/api/EditPageTest.php
M repo/tests/phpunit/includes/api/GetClaimsTest.php
M repo/tests/phpunit/includes/api/GetEntitiesTest.php
M repo/tests/phpunit/includes/api/LinkTitlesTest.php
M repo/tests/phpunit/includes/api/RemoveQualifiersTest.php
M repo/tests/phpunit/includes/api/RemoveReferencesTest.php
M repo/tests/phpunit/includes/api/SetAliasesTest.php
M repo/tests/phpunit/includes/api/SetClaimValueTest.php
M repo/tests/phpunit/includes/api/SetQualifierTest.php
M repo/tests/phpunit/includes/api/SetReferenceTest.php
M repo/tests/phpunit/includes/api/SetSiteLinkTest.php
M repo/tests/phpunit/includes/api/SetStatementRankTest.php
12 files changed, 102 insertions(+), 189 deletions(-)
Approvals:
Tobias Gritschacher: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/tests/phpunit/includes/api/EditPageTest.php
b/repo/tests/phpunit/includes/api/EditPageTest.php
index 6249d77..4c6f5bd 100644
--- a/repo/tests/phpunit/includes/api/EditPageTest.php
+++ b/repo/tests/phpunit/includes/api/EditPageTest.php
@@ -64,20 +64,14 @@
$text = json_encode( $data );
// try to update the item with valid data via the edit action
- try {
- $this->doApiRequestWithToken(
- array(
- 'action' => 'edit',
- 'pageid' =>
$content->getTitle()->getArticleID(),
- 'text' => $text,
- )
- );
-
- $this->fail( "Updating an items should not be possible
via action=edit" );
- } catch ( \UsageException $ex ) {
- //ok, pass
- //print "\n$ex\n";
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken(
+ array(
+ 'action' => 'edit',
+ 'pageid' =>
$content->getTitle()->getArticleID(),
+ 'text' => $text,
+ )
+ );
}
diff --git a/repo/tests/phpunit/includes/api/GetClaimsTest.php
b/repo/tests/phpunit/includes/api/GetClaimsTest.php
index 8ae2c8d..d01e3c3 100644
--- a/repo/tests/phpunit/includes/api/GetClaimsTest.php
+++ b/repo/tests/phpunit/includes/api/GetClaimsTest.php
@@ -177,8 +177,6 @@
* @dataProvider invalidClaimProvider
*/
public function testGetInvalidClaims( $claimGuid ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbgetclaims',
'claim' => $claimGuid
@@ -186,12 +184,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not throw an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException, 'Exception was caught' );
}
public function invalidClaimProvider() {
diff --git a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
index 1f17409..fe59043 100644
--- a/repo/tests/phpunit/includes/api/GetEntitiesTest.php
+++ b/repo/tests/phpunit/includes/api/GetEntitiesTest.php
@@ -296,17 +296,12 @@
* @group API
*/
public function testGetEntitiesByMalformedId( ) {
- try {
- $badid = 'xyz123+++';
- $this->doApiRequest( array(
- 'action' => 'wbgetentities',
- 'ids' => $badid,
- ) );
-
- $this->fail( "Expected a usage exception when providing
a malformed id" );
- } catch ( \UsageException $ex ) {
- $this->assertTrue( true, "make phpunit happy" );
- }
+ $this->setExpectedException( 'UsageException' );
+ $badid = 'xyz123+++';
+ $this->doApiRequest( array(
+ 'action' => 'wbgetentities',
+ 'ids' => $badid,
+ ) );
}
/**
@@ -316,18 +311,12 @@
* @group API
*/
public function testGetEntitiesByBadSite( ) {
- try {
- $this->doApiRequest( array(
- 'action' => 'wbgetentities',
- 'sites' => 'enwiktionary',
- 'titles' => 'Berlin',
- ) );
-
- $this->fail( "expected request to fail" );
- } catch ( \UsageException $ex ) {
- // ok
- $this->assertTrue( true );
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequest( array(
+ 'action' => 'wbgetentities',
+ 'sites' => 'enwiktionary',
+ 'titles' => 'Berlin',
+ ) );
}
/**
diff --git a/repo/tests/phpunit/includes/api/LinkTitlesTest.php
b/repo/tests/phpunit/includes/api/LinkTitlesTest.php
index ade5acc..c2640cf 100644
--- a/repo/tests/phpunit/includes/api/LinkTitlesTest.php
+++ b/repo/tests/phpunit/includes/api/LinkTitlesTest.php
@@ -61,13 +61,9 @@
'totitle' => "testLinkTitlesWithEvenLessToken",
);
- try {
- $this->doApiRequest( $req, null, false,
self::$users['wbeditor']->user );
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequest( $req, null, false,
self::$users['wbeditor']->user );
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
}
public static function provideLinkTitles() {
@@ -145,41 +141,38 @@
'totitle' => $totitle,
) );
- try {
- list( $res,, ) = $this->doApiRequestWithToken( $req,
null, self::$users['wbeditor']->user );
+ if( $expectedFailure ){
+ $this->setExpectedException( 'UsageException' );
+ }
- if ( $expectedFailure ) {
- $this->fail( "Expected failure:
$expectedFailure" );
- }
+ list( $res,, ) = $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
- // check the response -------------------------------
- $this->assertEquals( \Wikibase\Item::ENTITY_TYPE,
$res['entity']['type'] );
- if ( $handle ) {
- $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
- }
- else {
- $this->assertEquals( 2, count(
$res['entity']['sitelinks'] ), "expected exactly two sitelinks structure" );
- }
+ if ( $expectedFailure ) {
+ $this->fail( "Expected failure: $expectedFailure" );
+ }
- $this->assertArrayHasKey( 'lastrevid', $res['entity'] ,
'entity should contain lastrevid key' );
+ // check the response -------------------------------
+ $this->assertEquals( \Wikibase\Item::ENTITY_TYPE,
$res['entity']['type'] );
+ if ( $handle ) {
+ $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
+ }
+ else {
+ $this->assertEquals( 2, count(
$res['entity']['sitelinks'] ), "expected exactly two sitelinks structure" );
+ }
- foreach ( $res['entity']['sitelinks'] as $link ) {
- $this->assertTrue( $fromsite === $link['site']
|| $tosite === $link['site'] );
- $this->assertTrue( $fromtitle ===
$link['title'] || $totitle === $link['title'] );
- }
+ $this->assertArrayHasKey( 'lastrevid', $res['entity'] , 'entity
should contain lastrevid key' );
- // check the item in the database
-------------------------------
- if ( isset( $id ) ) {
- $item = $this->loadEntity( $id );
- $links = self::flattenArray(
$item['sitelinks'], 'site', 'title' );
- $this->assertEquals( $fromtitle,
$links[$fromsite], 'wrong link target' );
- $this->assertEquals( $totitle, $links[$tosite],
'wrong link target' );
- }
+ foreach ( $res['entity']['sitelinks'] as $link ) {
+ $this->assertTrue( $fromsite === $link['site'] ||
$tosite === $link['site'] );
+ $this->assertTrue( $fromtitle === $link['title'] ||
$totitle === $link['title'] );
+ }
- } catch ( \UsageException $e ) {
- if ( !$expectedFailure ) {
- $this->fail( "unexpected exception: $e" );
- }
+ // check the item in the database
-------------------------------
+ if ( isset( $id ) ) {
+ $item = $this->loadEntity( $id );
+ $links = self::flattenArray( $item['sitelinks'],
'site', 'title' );
+ $this->assertEquals( $fromtitle, $links[$fromsite],
'wrong link target' );
+ $this->assertEquals( $totitle, $links[$tosite], 'wrong
link target' );
}
if ( $cleanUp ) {
diff --git a/repo/tests/phpunit/includes/api/RemoveQualifiersTest.php
b/repo/tests/phpunit/includes/api/RemoveQualifiersTest.php
index 66e8ebd..fe5ea6c 100644
--- a/repo/tests/phpunit/includes/api/RemoveQualifiersTest.php
+++ b/repo/tests/phpunit/includes/api/RemoveQualifiersTest.php
@@ -148,7 +148,7 @@
try {
$this->doApiRequest( $params );
- $this->assertFalse( true, 'Invalid request should raise
an exception' );
+ $this->fail( 'Invalid request should raise an
exception' );
}
catch ( \UsageException $e ) {
if ( $expectedError === null ) {
@@ -164,8 +164,6 @@
* @dataProvider invalidGuidProvider
*/
public function testInvalidClaimGuid( $claimGuid, $hash ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbremovequalifiers',
'claim' => $claimGuid,
@@ -175,12 +173,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not throw an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( $e->getCodeString(),
'invalid-guid', 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException );
}
public function invalidGuidProvider() {
diff --git a/repo/tests/phpunit/includes/api/RemoveReferencesTest.php
b/repo/tests/phpunit/includes/api/RemoveReferencesTest.php
index 6f6a6a6..964845b 100644
--- a/repo/tests/phpunit/includes/api/RemoveReferencesTest.php
+++ b/repo/tests/phpunit/includes/api/RemoveReferencesTest.php
@@ -149,7 +149,7 @@
try {
$this->doApiRequest( $params );
- $this->assertFalse( true, 'Invalid request should raise
an exception' );
+ $this->fail( 'Invalid request should raise an
exception' );
}
catch ( \UsageException $e ) {
if ( $expectedError === null ) {
@@ -165,8 +165,6 @@
* @dataProvider invalidGuidProvider
*/
public function testInvalidStatementGuid( $statementGuid, $hash ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbremovereferences',
'statement' => $statementGuid,
@@ -176,12 +174,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not throw an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException );
}
public function invalidGuidProvider() {
diff --git a/repo/tests/phpunit/includes/api/SetAliasesTest.php
b/repo/tests/phpunit/includes/api/SetAliasesTest.php
index 445ea0e..30e7a2b 100644
--- a/repo/tests/phpunit/includes/api/SetAliasesTest.php
+++ b/repo/tests/phpunit/includes/api/SetAliasesTest.php
@@ -166,13 +166,8 @@
'set' => 'foo'
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "Expected a usage exception when providing
a malformed id" );
- } catch ( \UsageException $ex ) {
- $this->assertTrue( true, "make phpunit happy" );
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
/**
diff --git a/repo/tests/phpunit/includes/api/SetClaimValueTest.php
b/repo/tests/phpunit/includes/api/SetClaimValueTest.php
index f92748d..4929a91 100644
--- a/repo/tests/phpunit/includes/api/SetClaimValueTest.php
+++ b/repo/tests/phpunit/includes/api/SetClaimValueTest.php
@@ -136,8 +136,6 @@
* @dataProvider invalidClaimProvider
*/
public function testInvalidClaimGuid( $claimGuid ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbsetclaimvalue',
'claim' => $claimGuid,
@@ -148,12 +146,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not raise an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException, 'Exception was caught' );
}
public function invalidClaimProvider() {
diff --git a/repo/tests/phpunit/includes/api/SetQualifierTest.php
b/repo/tests/phpunit/includes/api/SetQualifierTest.php
index e8ed394..a5dd084 100644
--- a/repo/tests/phpunit/includes/api/SetQualifierTest.php
+++ b/repo/tests/phpunit/includes/api/SetQualifierTest.php
@@ -222,8 +222,6 @@
* @dataProvider invalidClaimProvider
*/
public function testInvalidClaimGuid( $claimGuid ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbsetqualifier',
'claim' => $claimGuid,
@@ -235,12 +233,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not throw an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException, 'Exception was caught' );
}
public function invalidClaimProvider() {
diff --git a/repo/tests/phpunit/includes/api/SetReferenceTest.php
b/repo/tests/phpunit/includes/api/SetReferenceTest.php
index cfc9c4e..ff87038 100644
--- a/repo/tests/phpunit/includes/api/SetReferenceTest.php
+++ b/repo/tests/phpunit/includes/api/SetReferenceTest.php
@@ -150,8 +150,6 @@
* @dataProvider invalidClaimProvider
*/
public function testInvalidClaimGuid( $claimGuid, $snakHash, $refHash,
$expectedError ) {
- $caughtException = false;
-
$params = array(
'action' => 'wbsetreference',
'statement' => $claimGuid,
diff --git a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
index 8db30cb..c00275c 100644
--- a/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/api/SetSiteLinkTest.php
@@ -62,13 +62,8 @@
'linktitle' => "testSetLiteLinkWithNoId",
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
public function testSetLiteLinkWithBadId( ) {
@@ -79,13 +74,8 @@
'linktitle' => "testSetLiteLinkWithNoId",
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
public function testSetLiteLinkWithBadSite( ) {
@@ -97,13 +87,8 @@
'linktitle' => "Berlin",
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
public function testSetLiteLinkWithBadTitle( ) {
@@ -115,13 +100,8 @@
'linktitle' => "testSetLiteLinkWithBadTitle_en",
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
public function testSetLiteLinkWithNoToken( ) {
@@ -137,13 +117,8 @@
'linktitle' => "testSetLiteLinkWithNoToken",
);
- try {
- $this->doApiRequest( $req, null, false,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequest( $req, null, false,
self::$users['wbeditor']->user );
}
public static function provideSetLiteLink() {
@@ -198,46 +173,44 @@
'linktitle' => $linktitle,
) );
- try {
- list( $res,, ) = $this->doApiRequestWithToken( $req,
null, self::$users['wbeditor']->user );
+ if( $expectedFailure ){
+ $this->setExpectedException($expectedFailure );
+ }
- if ( $expectedFailure ) {
- $this->fail( $expectedFailure );
- }
+ list( $res,, ) = $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
- // check the response -------------------------------
- //$this->assertSuccess( $res, 'entity', 'sitelinks', 0
);
- if ( $expectedTitle !== false ) {
- $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
- }
+ if ( $expectedFailure ) {
+ $this->fail( $expectedFailure );
+ }
- $this->assertArrayHasKey( 'lastrevid', $res['entity'] ,
'entity should contain lastrevid key' );
+ // check the response -------------------------------
+ //$this->assertSuccess( $res, 'entity', 'sitelinks', 0 );
+ if ( $expectedTitle !== false ) {
+ $this->assertEquals( 1, count(
$res['entity']['sitelinks'] ), "expected exactly one sitelinks structure" );
+ }
- if ( $expectedTitle !== false ) {
- $link = array_shift(
$res['entity']['sitelinks'] );
- $this->assertEquals( $linksite, $link['site'] );
- } else {
- $link = null;
- }
+ $this->assertArrayHasKey( 'lastrevid', $res['entity'] , 'entity
should contain lastrevid key' );
- if ( $linktitle === '' && $link !== null ) {
- $this->assertArrayHasKey( 'removed', $link );
- }
+ if ( $expectedTitle !== false ) {
+ $link = array_shift( $res['entity']['sitelinks'] );
+ $this->assertEquals( $linksite, $link['site'] );
+ } else {
+ $link = null;
+ }
- if ( $expectedTitle !== false ) {
- $this->assertEquals( $expectedTitle,
$link['title'] );
- }
+ if ( $linktitle === '' && $link !== null ) {
+ $this->assertArrayHasKey( 'removed', $link );
+ }
- if ( $expectedTitle !== false && $linktitle !== '' ) {
- $this->assertArrayHasKey( 'url', $link );
- }
- elseif ( $link !== null ) {
- $this->assertArrayNotHasKey( 'url', $link );
- }
- } catch ( \UsageException $e ) {
- if ( !$expectedFailure ) {
- $this->fail( "unexpected exception: $e" );
- }
+ if ( $expectedTitle !== false ) {
+ $this->assertEquals( $expectedTitle, $link['title'] );
+ }
+
+ if ( $expectedTitle !== false && $linktitle !== '' ) {
+ $this->assertArrayHasKey( 'url', $link );
+ }
+ elseif ( $link !== null ) {
+ $this->assertArrayNotHasKey( 'url', $link );
}
// check the item in the database
-------------------------------
@@ -267,13 +240,8 @@
'linktitle' => "Berlin",
);
- try {
- $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
-
- $this->fail( "request should have failed" );
- } catch ( \UsageException $e ) {
- $this->assertTrue( true ); // ok
- }
+ $this->setExpectedException( 'UsageException' );
+ $this->doApiRequestWithToken( $req, null,
self::$users['wbeditor']->user );
}
}
diff --git a/repo/tests/phpunit/includes/api/SetStatementRankTest.php
b/repo/tests/phpunit/includes/api/SetStatementRankTest.php
index 236112a..c476d72 100644
--- a/repo/tests/phpunit/includes/api/SetStatementRankTest.php
+++ b/repo/tests/phpunit/includes/api/SetStatementRankTest.php
@@ -188,7 +188,7 @@
try {
$this->doApiRequest( $params );
- $this->assertFalse( true, 'Invalid request should raise
an exception' );
+ $this->fail( 'Invalid request should raise an
exception' );
}
catch ( \Exception $e ) {
if ( $e instanceof \UsageException ) {
@@ -212,8 +212,6 @@
* @dataProvider invalidClaimProvider
*/
public function testInvalidClaimGuid( $claimGuid ) {
- $caughtException = false;
-
$ranks = ClaimSerializer::getRanks();
$params = array(
@@ -225,12 +223,10 @@
try {
$this->doApiRequest( $params );
+ $this->fail( 'Invalid claim guid did not throw an
error' );
} catch ( \UsageException $e ) {
$this->assertEquals( 'invalid-guid',
$e->getCodeString(), 'Invalid claim guid raised correct error' );
- $caughtException = true;
}
-
- $this->assertTrue( $caughtException, 'Exception was caught' );
}
public function invalidClaimProvider() {
--
To view, visit https://gerrit.wikimedia.org/r/77700
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic0e5457f7774cfbd9e01f1f692a33eb4d101c1be
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <addshorew...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: Tobias Gritschacher <tobias.gritschac...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
