jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/399619 )
Change subject: Simplify the microtime() mocking in CachingResultsBuilderTest
......................................................................
Simplify the microtime() mocking in CachingResultsBuilderTest
This is a direct follow up for all comments I wrote in I9f114fd, and
should be considered part of T182106.
The most relevant change is the still fragile microtime mocking (hence
the title of this commit). I'm also reintroducing the "makeKey" name,
and use trivial test implementations instead of mocks when possible.
Lucas, feel free to pick this patch up and continue working on it as you
like. If you disagree with certain changes I propose here, feel free to
revert them.
Bug: T182106
Change-Id: I456e6552ccbd1398f5e8fa978a20b0bb2be1109e
---
M src/ConstraintCheck/Api/CachingResultsBuilder.php
M tests/phpunit/Api/CachingResultsBuilderTest.php
2 files changed, 24 insertions(+), 32 deletions(-)
Approvals:
Lucas Werkmeister (WMDE): Looks good to me, approved
jenkins-bot: Verified
Thiemo Kreuz (WMDE): Looks good to me, but someone else must approve
diff --git a/src/ConstraintCheck/Api/CachingResultsBuilder.php
b/src/ConstraintCheck/Api/CachingResultsBuilder.php
index 06bbd58..f37249f 100644
--- a/src/ConstraintCheck/Api/CachingResultsBuilder.php
+++ b/src/ConstraintCheck/Api/CachingResultsBuilder.php
@@ -132,7 +132,7 @@
* @param EntityId $entityId
* @return string cache key
*/
- public function getKey( EntityId $entityId ) {
+ public function makeKey( EntityId $entityId ) {
return $this->cache->makeKey(
'WikibaseQualityConstraints', // extension
'checkConstraints', // action
@@ -156,7 +156,7 @@
if ( $this->canStoreResults( $entityIds, $claimIds,
$constraintIds ) ) {
foreach ( $entityIds as $entityId ) {
- $key = $this->getKey( $entityId );
+ $key = $this->makeKey( $entityId );
$value = [
'results' =>
$results->getArray()[$entityId->getSerialization()],
'latestRevisionIds' =>
$this->getLatestRevisionIds(
@@ -196,7 +196,7 @@
public function getStoredResults(
EntityId $entityId
) {
- $key = $this->getKey( $entityId );
+ $key = $this->makeKey( $entityId );
$value = $this->cache->get( $key, $curTTL, [], $asOf );
$now = call_user_func( $this->microtime, true );
@@ -204,7 +204,7 @@
return null;
}
- $ageInSeconds = (integer)ceil( $now - $asOf );
+ $ageInSeconds = (int)ceil( $now - $asOf );
$dependedEntityIds = array_map(
[ $this->entityIdParser, "parse" ],
diff --git a/tests/phpunit/Api/CachingResultsBuilderTest.php
b/tests/phpunit/Api/CachingResultsBuilderTest.php
index f2bfdde..74b2cbe 100644
--- a/tests/phpunit/Api/CachingResultsBuilderTest.php
+++ b/tests/phpunit/Api/CachingResultsBuilderTest.php
@@ -6,7 +6,6 @@
use TimeAdjustableWANObjectCache;
use WANObjectCache;
use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Entity\EntityIdParser;
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\ItemIdParser;
use Wikibase\DataModel\Entity\PropertyId;
@@ -42,7 +41,7 @@
$resultsBuilder,
WANObjectCache::newEmpty(),
$this->getMock( EntityRevisionLookup::class ),
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
@@ -71,7 +70,7 @@
$resultsBuilder,
$cache,
$lookup,
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
@@ -99,7 +98,7 @@
$resultsBuilder,
$cache,
$lookup,
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
@@ -123,12 +122,12 @@
$resultsBuilder,
$cache,
$lookup,
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
$cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], null
);
- $cachedResults = $cache->get( $cachingResultsBuilder->getKey(
$q100 ) );
+ $cachedResults = $cache->get( $cachingResultsBuilder->makeKey(
$q100 ) );
$this->assertNotNull( $cachedResults );
$this->assertArrayHasKey( 'results', $cachedResults );
@@ -170,12 +169,12 @@
$resultsBuilder,
$cache,
$lookup,
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
$cachingResultsBuilder->getAndStoreResults( [ $q100 ], [], null
);
- $cachedResults = $cache->get( $cachingResultsBuilder->getKey(
$q100 ) );
+ $cachedResults = $cache->get( $cachingResultsBuilder->makeKey(
$q100 ) );
$this->assertNotNull( $cachedResults );
$this->assertArrayHasKey( 'latestRevisionIds', $cachedResults );
@@ -187,7 +186,7 @@
$this->getMock( ResultsBuilder::class ),
WANObjectCache::newEmpty(),
$this->getMock( EntityRevisionLookup::class ),
- $this->getMock( EntityIdParser::class ),
+ new ItemIdParser(),
86400
);
@@ -216,7 +215,7 @@
86400
);
$q5 = new ItemId( 'Q5' );
- $key = $cachingResultsBuilder->getKey( $q5 );
+ $key = $cachingResultsBuilder->makeKey( $q5 );
$value = [
'results' => 'garbage data, should not matter',
'latestRevisionIds' => [
@@ -242,11 +241,9 @@
return 99;
}
} );
+
$cache = new TimeAdjustableWANObjectCache( [ 'cache' => new
HashBagOStuff() ] );
- $now = microtime( true );
- if ( ( $now - 1337 ) + 1337 !== $now ) {
- $this->markTestSkipped( 'Unix time outside float
accuracy range?!' );
- }
+ $now = 9001;
$cache->setTime( $now - 1337 );
$cachingResultsBuilder = new CachingResultsBuilder(
$this->getMock( ResultsBuilder::class ),
@@ -255,12 +252,12 @@
new ItemIdParser(),
86400
);
- $cachingResultsBuilder->setMicrotimeFunction( function (
$get_as_float ) use ( $now ) {
- $this->assertTrue( $get_as_float );
+ $cachingResultsBuilder->setMicrotimeFunction( function () use (
$now ) {
return $now;
} );
+
$q5 = new ItemId( 'Q5' );
- $key = $cachingResultsBuilder->getKey( $q5 );
+ $key = $cachingResultsBuilder->makeKey( $q5 );
$expectedResults = 'garbage data, should not matter';
$value = [
'results' => $expectedResults,
@@ -274,24 +271,23 @@
$response = $cachingResultsBuilder->getStoredResults( $q5 );
$this->assertNotNull( $response );
- $actualResults = $response->getArray();
- $this->assertSame( [ 'Q5' => $expectedResults ], $actualResults
);
+ $this->assertSame( [ 'Q5' => $expectedResults ],
$response->getArray() );
$cachingMetadata =
$response->getMetadata()->getCachingMetadata();
$this->assertTrue( $cachingMetadata->isCached() );
- $maxAgeInSeconds = $cachingMetadata->getMaximumAgeInSeconds();
- $this->assertSame( 1337, $maxAgeInSeconds );
+ $this->assertSame( 1337,
$cachingMetadata->getMaximumAgeInSeconds() );
}
public function testGetResults_EmptyCache() {
+ $entityIds = [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ];
+
$cachingResultsBuilder = $this->getMockBuilder(
CachingResultsBuilder::class )
->disableOriginalConstructor()
->setMethods( [ 'getStoredResults',
'getAndStoreResults' ] )
->getMock();
$cachingResultsBuilder->method( 'getStoredResults'
)->willReturn( null );
$cachingResultsBuilder->method( 'getAndStoreResults' )
+ ->with( $entityIds, [], null )
->willReturnCallback( function( $entityIds, $claimIds,
$constraintIds ) {
- $this->assertSame( [], $claimIds );
- $this->assertNull( $constraintIds );
$results = [];
foreach ( $entityIds as $entityId ) {
$serialization =
$entityId->getSerialization();
@@ -301,11 +297,7 @@
} );
/** @var CachingResultsBuilder $cachingResultsBuilder */
- $results = $cachingResultsBuilder->getResults(
- [ new ItemId( 'Q5' ), new ItemId( 'Q10' ) ],
- [],
- null
- );
+ $results = $cachingResultsBuilder->getResults( $entityIds, [],
null );
$expected = [ 'Q5' => 'garbage of Q5', 'Q10' => 'garbage of
Q10' ];
$actual = $results->getArray();
--
To view, visit https://gerrit.wikimedia.org/r/399619
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I456e6552ccbd1398f5e8fa978a20b0bb2be1109e
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/WikibaseQualityConstraints
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <[email protected]>
Gerrit-Reviewer: Lucas Werkmeister (WMDE) <[email protected]>
Gerrit-Reviewer: Thiemo Kreuz (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
