Lucas Werkmeister (WMDE) has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/374566 )
Change subject: Improve DelegatingConstraintCheckerTest
......................................................................
Improve DelegatingConstraintCheckerTest
Instead of a JsonFileEntityLookup, we use an InMemoryEntityLookup and
create the items in the tests that use them, which makes the tests much
more readable and lets us get rid of some bloated JSON files. The tests
also use checkAgainstConstraintsOnEntityId instead of
checkAgainstConstraints, because I plan to remove the latter method
(it’s almost entirely redundant with checkAgainstConstraintsOnEntityId).
Change-Id: I4c6528fb38de33562770455b794dea6620638907
---
M tests/phpunit/DelegatingConstraintCheckerTest.php
D tests/phpunit/P1.json
D tests/phpunit/Q1.json
D tests/phpunit/Q2.json
D tests/phpunit/Q3.json
D tests/phpunit/Q4.json
D tests/phpunit/Q5.json
D tests/phpunit/Q6.json
8 files changed, 105 insertions(+), 392 deletions(-)
Approvals:
Jonas Kress (WMDE): Looks good to me, approved
jenkins-bot: Verified
diff --git a/tests/phpunit/DelegatingConstraintCheckerTest.php
b/tests/phpunit/DelegatingConstraintCheckerTest.php
index 27e9d21..faf91ae 100644
--- a/tests/phpunit/DelegatingConstraintCheckerTest.php
+++ b/tests/phpunit/DelegatingConstraintCheckerTest.php
@@ -4,8 +4,10 @@
use Wikibase\DataModel\Entity\DispatchingEntityIdParser;
use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\Property;
use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\DataModel\Services\Lookup\EntityRetrievingDataTypeLookup;
+use Wikibase\DataModel\Services\Lookup\InMemoryEntityLookup;
use Wikibase\DataModel\Services\Statement\StatementGuidParser;
use Wikibase\Rdf\RdfVocabulary;
use Wikibase\Repo\Tests\NewItem;
@@ -16,7 +18,6 @@
use WikibaseQuality\ConstraintReport\Tests\ConstraintParameters;
use WikibaseQuality\ConstraintReport\Tests\ResultAssertions;
use WikibaseQuality\ConstraintReport\Tests\TitleParserMock;
-use WikibaseQuality\Tests\Helper\JsonFileEntityLookup;
use Wikimedia\Rdbms\DBUnexpectedError;
/**
@@ -59,13 +60,13 @@
private $constraintChecker;
/**
- * @var JsonFileEntityLookup
+ * @var InMemoryEntityLookup
*/
private $lookup;
protected function setUp() {
parent::setUp();
- $this->lookup = $this->createEntityLookup();
+ $this->lookup = new InMemoryEntityLookup();
$entityIdParser = new DispatchingEntityIdParser( [
'/^Q/' => function( $serialization ) {
return new ItemId( $serialization );
@@ -314,59 +315,97 @@
);
}
- public function testCheckAgainstConstraints() {
- $entity = $this->lookup->getEntity( new ItemId( 'Q1' ) );
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
- $this->assertEquals( 18, count( $result ), 'Every constraint
should be represented by one result' );
+ public function testCheckOnEntityId() {
+ $entity = NewItem::withId( 'Q1' )
+ ->andStatement(
+ NewStatement::forProperty( 'P1' )
+ ->withValue( 'foo' )
+ )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
+ $this->assertCount( 18, $result, 'Every constraint should be
represented by one result' );
foreach ( $result as $checkResult ) {
$this->assertNotSame( 'todo',
$checkResult->getStatus(), 'Constraints should not be unimplemented' );
}
}
- public function testCheckAgainstConstraintsWithoutEntity() {
- $result = $this->constraintChecker->checkAgainstConstraints(
null );
- $this->assertEquals( null, $result, 'Should return null' );
+ public function testCheckOnEntityIdEmptyResult() {
+ $entity = NewItem::withId( 'Q2' )
+ ->andStatement(
+ NewStatement::forProperty( 'P2' )
+ ->withValue( 'foo' )
+ )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
+ $this->assertCount( 0, $result, 'Should be empty' );
}
- public function
testCheckAgainstConstraintsDoesNotCrashWhenResultIsEmpty() {
- $entity = $this->lookup->getEntity( new ItemId( 'Q2' ) );
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
- $this->assertEquals( 0, count( $result ), 'Should be empty' );
- }
+ public function testCheckOnEntityIdUnknownConstraint() {
+ $entity = NewItem::withId( 'Q3' )
+ ->andStatement(
+ NewStatement::forProperty( 'P3' )
+ ->withValue( 'foo' )
+ )
+ ->build();
+ $this->lookup->addEntity( $entity );
- public function
testCheckAgainstConstraintsWithConstraintThatDoesNotBelongToCheckedConstraints()
{
- $entity = $this->lookup->getEntity( new ItemId( 'Q3' ) );
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
- $this->assertEquals( 1, count( $result ), 'Should be one
result' );
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
+ $this->assertCount( 1, $result, 'Should be one result' );
$this->assertEquals( 'todo', $result[ 0 ]->getStatus(), 'Should
be marked as a todo' );
}
- public function
testCheckAgainstConstraintsDoesNotCrashWhenStatementHasNovalue() {
- $entity = $this->lookup->getEntity( new ItemId( 'Q4' ) );
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
- $this->assertEquals( 0, count( $result ), 'Should be empty' );
+ public function testCheckOnEntityIdNoValue() {
+ $entity = NewItem::withId( 'Q4' )
+ ->andStatement(
+ NewStatement::noValueFor( 'P4' )
+ )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
+ $this->assertCount( 0, $result, 'Should be empty' );
}
- public function testCheckAgainstConstraintsWithKnownException() {
- $entity = $this->lookup->getEntity( new ItemId( 'Q5' ) );
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
+ public function testCheckOnEntityIdKnownException() {
+ $entity = NewItem::withId( 'Q5' )
+ ->andStatement(
+ NewStatement::forProperty( 'P10' )
+ ->withValue( 'foo' )
+ )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
$this->assertEquals( 'exception', $result[ 0 ]->getStatus(),
'Should be an exception' );
}
- public function testCheckAgainstConstraintsWithBrokenException() {
+ public function testCheckOnEntityIdBrokenException() {
$entity = NewItem::withId( 'Q5' )
->andStatement( NewStatement::noValueFor( 'P11' ) )
->build();
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
$this->assertEquals( 'bad-parameters', $result[ 0
]->getStatus(), 'Should be a bad parameter but not throw an exception' );
}
- public function testCheckAgainstConstraintsWithMandatoryConstraint() {
+ public function testCheckOnEntityIdMandatoryConstraint() {
$entity = NewItem::withId( 'Q6' )
->andStatement( NewStatement::noValueFor( 'P6' ) )
->build();
+ $this->lookup->addEntity( $entity );
- $results = $this->constraintChecker->checkAgainstConstraints(
$entity );
+ $results =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
$this->assertCount( 1, $results );
$result = $results[0];
@@ -375,29 +414,52 @@
$this->assertSame( [ 'mandatory' ], $result->getParameters()[
'constraint_status' ] );
}
- public function testCheckAgainstConstraintsWithNonMandatoryConstraint()
{
+ public function testCheckOnEntityIdNonMandatoryConstraint() {
$entity = NewItem::withId( 'Q7' )
->andStatement( NewStatement::noValueFor( 'P7' ) )
->build();
- $result = $this->constraintChecker->checkAgainstConstraints(
$entity );
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkAgainstConstraintsOnEntityId( $entity->getId() );
+
$this->assertEquals( 'warning', $result[ 0 ]->getStatus(),
'Should be a warning' );
}
- public function testCheckAgainstConstraints_ByClaims() {
+ public function testCheckOnClaimId() {
+ $statement = NewStatement::forProperty( 'P1' )
+ ->withValue( 'foo' )
+ ->withGuid( 'Q1$c0f25a6f-9e33-41c8-be34-c86a730ff30b' )
+ ->build();
+ $entity = NewItem::withId( 'Q1' )
+ ->andStatement( $statement )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
$result =
$this->constraintChecker->checkAgainstConstraintsOnClaimId(
- 'Q1$c0f25a6f-9e33-41c8-be34-c86a730ff30b' );
+ $statement->getGuid()
+ );
$this->assertCount( 18, $result, 'Every constraint should be
represented by one result' );
}
- public function
testCheckAgainstConstraintsDoesNotCrashWhenResultIsEmpty_ByClaims() {
+ public function testCheckOnClaimIdEmptyResult() {
+ $statement = NewStatement::forProperty( 'P2' )
+ ->withValue( 'foo' )
+ ->withGuid( 'Q2$1d1fd258-91ca-4db5-91e4-26219c5aae7a' )
+ ->build();
+ $entity = NewItem::withId( 'Q2' )
+ ->andStatement( $statement )
+ ->build();
+ $this->lookup->addEntity( $entity );
+
$result =
$this->constraintChecker->checkAgainstConstraintsOnClaimId(
- 'Q2$c0f25a6f-9e33-41c8-be34-c86a730ff30b' );
+ $statement->getGuid()
+ );
$this->assertCount( 0, $result, 'Should be empty' );
}
- public function
testCheckAgainstConstraintsDoesNotCrashWhenClaimDoesNotExist() {
+ public function testCheckOnClaimIdUnknownClaimId() {
$result =
$this->constraintChecker->checkAgainstConstraintsOnClaimId(
'Q99$does-not-exist' );
@@ -405,7 +467,12 @@
}
public function testCheckConstraintParametersOnPropertyId() {
- $result =
$this->constraintChecker->checkConstraintParametersOnPropertyId( new
PropertyId( 'P1' ) );
+ $entity = new Property( new PropertyId( 'P1' ), null, 'time' );
+ $this->lookup->addEntity( $entity );
+
+ $result =
$this->constraintChecker->checkConstraintParametersOnPropertyId(
+ $entity->getId()
+ );
$this->assertCount( 18, $result, 'Every constraint should be
represented by one result' );
foreach ( $result as $constraintGuid => $constraintResult ) {
@@ -455,13 +522,6 @@
$this->assertCount( 2, $result, 'The constraint should have two
exceptions' );
$this->assertInstanceOf( ConstraintParameterException::class,
$result[0] );
$this->assertInstanceOf( ConstraintParameterException::class,
$result[1] );
- }
-
- /**
- * @return JsonFileEntityLookup
- */
- private function createEntityLookup() {
- return new JsonFileEntityLookup( __DIR__ );
}
}
diff --git a/tests/phpunit/P1.json b/tests/phpunit/P1.json
deleted file mode 100644
index c5c32ad..0000000
--- a/tests/phpunit/P1.json
+++ /dev/null
@@ -1,5 +0,0 @@
-{
- "id": "P1",
- "type": "property",
- "datatype": "time"
-}
diff --git a/tests/phpunit/Q1.json b/tests/phpunit/Q1.json
deleted file mode 100644
index 8a4f762..0000000
--- a/tests/phpunit/Q1.json
+++ /dev/null
@@ -1,50 +0,0 @@
-{
- "id": "Q1",
- "type": "item",
- "aliases": {
- "en": [
- {
- "language": "en",
- "value": "foo"
- },
- {
- "language": "en",
- "value": "bar"
- }
- ],
- "de": [
- {
- "language": "de",
- "value": "foobar"
- }
- ]
- },
- "labels": {
- "en": {
- "language": "en",
- "value": "foobar"
- },
- "de": {
- "language": "de",
- "value": "Fubar"
- }
- },
- "claims": {
- "P1": [
- {
- "id": "Q1$c0f25a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P1",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
\ No newline at end of file
diff --git a/tests/phpunit/Q2.json b/tests/phpunit/Q2.json
deleted file mode 100644
index 5546a18..0000000
--- a/tests/phpunit/Q2.json
+++ /dev/null
@@ -1,50 +0,0 @@
-{
- "id": "Q2",
- "type": "item",
- "aliases": {
- "en": [
- {
- "language": "en",
- "value": "foo"
- },
- {
- "language": "en",
- "value": "bar"
- }
- ],
- "de": [
- {
- "language": "de",
- "value": "foobar"
- }
- ]
- },
- "labels": {
- "en": {
- "language": "en",
- "value": "foobar"
- },
- "de": {
- "language": "de",
- "value": "Fubar"
- }
- },
- "claims": {
- "P2": [
- {
- "id": "Q2$c0f25a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P2",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
\ No newline at end of file
diff --git a/tests/phpunit/Q3.json b/tests/phpunit/Q3.json
deleted file mode 100644
index 6978ce2..0000000
--- a/tests/phpunit/Q3.json
+++ /dev/null
@@ -1,50 +0,0 @@
-{
- "id": "Q3",
- "type": "item",
- "aliases": {
- "en": [
- {
- "language": "en",
- "value": "foo"
- },
- {
- "language": "en",
- "value": "bar"
- }
- ],
- "de": [
- {
- "language": "de",
- "value": "foobar"
- }
- ]
- },
- "labels": {
- "en": {
- "language": "en",
- "value": "foobar"
- },
- "de": {
- "language": "de",
- "value": "Fubar"
- }
- },
- "claims": {
- "P3": [
- {
- "id": "Q3$c0f25a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P3",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
\ No newline at end of file
diff --git a/tests/phpunit/Q4.json b/tests/phpunit/Q4.json
deleted file mode 100644
index 43b61e0..0000000
--- a/tests/phpunit/Q4.json
+++ /dev/null
@@ -1,45 +0,0 @@
-{
- "id": "Q4",
- "type": "item",
- "aliases": {
- "en": [
- {
- "language": "en",
- "value": "foo"
- },
- {
- "language": "en",
- "value": "bar"
- }
- ],
- "de": [
- {
- "language": "de",
- "value": "foobar"
- }
- ]
- },
- "labels": {
- "en": {
- "language": "en",
- "value": "foobar"
- },
- "de": {
- "language": "de",
- "value": "Fubar"
- }
- },
- "claims": {
- "P4": [
- {
- "id": "Q4$c0f25a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "novalue",
- "property": "P4"
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
\ No newline at end of file
diff --git a/tests/phpunit/Q5.json b/tests/phpunit/Q5.json
deleted file mode 100644
index 5070fb7..0000000
--- a/tests/phpunit/Q5.json
+++ /dev/null
@@ -1,50 +0,0 @@
-{
- "id": "Q5",
- "type": "item",
- "aliases": {
- "en": [
- {
- "language": "en",
- "value": "foo"
- },
- {
- "language": "en",
- "value": "bar"
- }
- ],
- "de": [
- {
- "language": "de",
- "value": "foobar"
- }
- ]
- },
- "labels": {
- "en": {
- "language": "en",
- "value": "foobar"
- },
- "de": {
- "language": "de",
- "value": "Fubar"
- }
- },
- "claims": {
- "P10": [
- {
- "id": "Q5$c0f25a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P10",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
\ No newline at end of file
diff --git a/tests/phpunit/Q6.json b/tests/phpunit/Q6.json
deleted file mode 100644
index 7e9acf8..0000000
--- a/tests/phpunit/Q6.json
+++ /dev/null
@@ -1,97 +0,0 @@
-{
- "id": "Q6",
- "type": "item",
- "aliases": {},
- "labels": {},
- "claims": {
- "P1": [
- {
- "id": "Q6$01015a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P1",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- },
- {
- "id": "Q6$01025a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P1",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ],
- "P2": [
- {
- "id": "Q6$02015a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P2",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ],
- "P3": [
- {
- "id": "Q6$03015a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P3",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ],
- "P4": [
- {
- "id": "Q6$04015a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "novalue",
- "property": "P4"
- },
- "type": "statement",
- "rank": "normal"
- }
- ],
- "P10": [
- {
- "id": "Q6$10015a6f-9e33-41c8-be34-c86a730ff30b",
- "mainsnak": {
- "snaktype": "value",
- "property": "P10",
- "datatype": "string",
- "datavalue": {
- "value": "foo",
- "type": "string"
- }
- },
- "type": "statement",
- "rank": "normal"
- }
- ]
- }
-}
--
To view, visit https://gerrit.wikimedia.org/r/374566
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c6528fb38de33562770455b794dea6620638907
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Jonas Kress (WMDE) <[email protected]>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
