Skip to content

Commit

Permalink
11608 Fix Intermittent PHPUnit Fails (#3863)
Browse files Browse the repository at this point in the history
  • Loading branch information
paul-m committed Nov 2, 2022
1 parent c98455b commit 7da8269
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ core_version_requirement: ^9.1
package: DKAN
dependencies:
- common
- datastore
- metastore
2 changes: 0 additions & 2 deletions modules/datastore/src/Service/Factory/Import.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@

/**
* Create an importer object for a given resource.
*
* @codeCoverageIgnore
*/
class Import implements ImportFactoryInterface {

Expand Down
11 changes: 8 additions & 3 deletions modules/datastore/src/Service/ResourcePurger.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use Drupal\common\LoggerTrait;
use Drupal\common\DataResource;
use Drupal\Core\Entity\EntityPublishedInterface;
use Drupal\datastore\Service;
use Drupal\metastore\ReferenceLookupInterface;
use Drupal\metastore\Storage\DataFactory;
Expand Down Expand Up @@ -207,7 +208,7 @@ private function purge(int $vid, string $uuid, bool $prior) {

foreach (array_diff($purge, $keep) as $idAndVersion) {
// $idAndVersion is a json encoded array with resource's id and version.
list($id, $version) = json_decode($idAndVersion);
[$id, $version] = json_decode($idAndVersion);
$this->delete($id, $version);
}
}
Expand All @@ -230,7 +231,7 @@ private function getResourcesToPurge(int $initialVid, NodeInterface $node, bool
$purge = [];

foreach ($this->getOlderRevisionIds($initialVid, $node) as $vid) {
list($published, $resource) = $this->getRevisionData($vid);
[$published, $resource] = $this->getRevisionData($vid);
$purge = array_merge($purge, $resource);
$publishedCount = $published ? $publishedCount + 1 : $publishedCount;
if (!$prior && $publishedCount >= 2) {
Expand Down Expand Up @@ -315,8 +316,12 @@ private function getResourcesToKeep(string $uuid) : array {
*/
private function getRevisionData(string $vid) : array {
$revision = $this->storage->getEntityStorage()->loadRevision($vid);
$published = FALSE;
if ($revision instanceof EntityPublishedInterface) {
$published = $revision->isPublished();
}
return [
$revision->status->value ?? FALSE,
$published,
$this->getResources($revision),
];
}
Expand Down
21 changes: 19 additions & 2 deletions modules/datastore/tests/src/Functional/DictionaryEnforcerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

use Drupal\datastore\Controller\ImportController;
use Drupal\metastore\DataDictionary\DataDictionaryDiscovery;
use Drupal\Tests\common\Traits\CleanUp;
use Drupal\Tests\common\Traits\GetDataTrait;
use Drupal\Tests\metastore\Unit\ServiceTest;

Expand All @@ -20,7 +21,7 @@
*/
class DictionaryEnforcerTest extends ExistingSiteBase {

use GetDataTrait;
use GetDataTrait, CleanUp;

/**
* Uploaded resource file destination.
Expand Down Expand Up @@ -85,6 +86,13 @@ class DictionaryEnforcerTest extends ExistingSiteBase {
*/
protected $webServiceApi;

/**
* External URL for the fixture CSV file.
*
* @var string
*/
protected $resourceUrl;

/**
* {@inheritdoc}
*/
Expand All @@ -100,16 +108,22 @@ public function setUp(): void {
$this->datasetStorage = \Drupal::service('dkan.metastore.storage')
->getInstance('dataset');
// Copy resource file to uploads directory.
/** @var \Drupal\Core\File\FileSystemInterface $file_system */
$file_system = \Drupal::service('file_system');
$upload_path = $file_system->realpath(self::UPLOAD_LOCATION);
$file_system->prepareDirectory($upload_path, FileSystemInterface::CREATE_DIRECTORY);
$file_system->copy(self::TEST_DATA_PATH . self::RESOURCE_FILE, $upload_path);
$file_system->copy(self::TEST_DATA_PATH . self::RESOURCE_FILE, $upload_path, FileSystemInterface::EXISTS_REPLACE);
// Create resource URL.
$this->resourceUrl = \Drupal::service('stream_wrapper_manager')
->getViaUri(self::UPLOAD_LOCATION . self::RESOURCE_FILE)
->getExternalUrl();
}

public function tearDown() {
parent::tearDown();
$this->removeAllMappedFiles();
}

/**
* Test dictionary enforcement.
*/
Expand Down Expand Up @@ -194,6 +208,9 @@ public function testDictionaryEnforcement(): void {
$response = $this->webServiceApi->summary($dist_id, $request);
$result = json_decode($response->getContent(), TRUE);

// Clean up after ourselves, before performing the assertion.
$this->metastore->delete('dataset', $dataset_id);

// Validate schema.
$this->assertEquals([
'numOfColumns' => 6,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ protected function setUp() {
\Drupal::setContainer($chain->getMock());
}

protected function tearDown(): void {
parent::tearDown();
$this->buffer = NULL;
}

/**
* Helper function to compare output of streaming vs normal query controller.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,11 @@ protected function setUp(): void {
$this->assertTrue($this->database instanceof DatabaseTableInterface);
}

protected function tearDown() {
parent::tearDown();
$this->database = NULL;
}

/**
*
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
use Symfony\Component\DependencyInjection\ContainerInterface;

/**
*
* @coversDefaultClass Drupal\frontend\Controller\Page
*/
class ControllerPageTest extends TestCase {

Expand All @@ -17,15 +17,20 @@ class ControllerPageTest extends TestCase {
*
* @var int
*/
private $cacheMaxAge = 0;
private $cacheMaxAge;

protected function setUp(): void {
parent::setUp();
$this->cacheMaxAge = 0;
}

/**
* Getter.
*/
public function getContainer() {

$container = $this->getMockBuilder(ContainerInterface::class)
->setMethods(['get', 'has'])
->onlyMethods(['get', 'has'])
->disableOriginalConstructor()
->getMockForAbstractClass();

Expand Down Expand Up @@ -53,22 +58,22 @@ public function containerGet($input) {
case 'frontend.page':
$pageBuilder = $this->getMockBuilder(Page::class)
->disableOriginalConstructor()
->setMethods(['build'])
->onlyMethods(['build'])
->getMock();
$pageBuilder->method('build')->willReturn("<h1>Hello World!!!</h1>\n");
return $pageBuilder;

break;
break;
case 'config.factory':
$immutableConfig = $this->getMockBuilder(ImmutableConfig::class)
->disableOriginalConstructor()
->setMethods(["get"])
->onlyMethods(["get"])
->getMock();
$immutableConfig->method('get')->willReturn($this->cacheMaxAge);

$configFactory = $this->getMockBuilder(ConfigFactory::class)
->disableOriginalConstructor()
->setMethods(["get"])
->onlyMethods(["get"])
->getMock();
$configFactory->method('get')->willReturn($immutableConfig);
return $configFactory;
Expand Down
9 changes: 4 additions & 5 deletions modules/metastore/src/Storage/Data.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Drupal\common\LoggerTrait;
use Drupal\Core\Entity\ContentEntityInterface;
use Drupal\Core\Entity\EntityPublishedInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Entity\Query\QueryInterface;
use Drupal\Core\Entity\RevisionLogInterface;
Expand Down Expand Up @@ -243,12 +244,10 @@ public function getEntityPublishedRevision(string $uuid): ?ContentEntityInterfac
}

$entity = $this->entityStorage->load($entity_id);
$published = $entity->status->value ?? FALSE;
if (!$published) {
return NULL;
if ($entity instanceof EntityPublishedInterface) {
return $entity->isPublished() ? $entity : NULL;
}

return $entity;
return NULL;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions modules/metastore/tests/src/Unit/ServiceTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
use RootedData\RootedJsonData;

/**
*
* @coversDefaultClass Drupal\metastore\Service
*/
class ServiceTest extends TestCase {

Expand Down Expand Up @@ -296,7 +296,7 @@ public function testPublish() {
$result = $service->publish('dataset', 1);
$this->assertTrue($result);
}

/**
*
*/
Expand Down
14 changes: 13 additions & 1 deletion tests/src/Functional/DatasetTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,22 @@ public function setUp(): void {
$this->removeDatastoreTables();
$this->setDefaultModerationState();
$this->changeDatasetsResourceOutputPerspective();

$this->validMetadataFactory = ServiceTest::getValidMetadataFactory($this);
}

public function tearDown() {
parent::tearDown();
$this->removeHarvests();
$this->removeAllNodes();
$this->removeAllMappedFiles();
$this->removeAllFileFetchingJobs();
$this->flushQueues();
$this->removeFiles();
$this->removeDatastoreTables();
$this->setDefaultModerationState();
$this->changeDatasetsResourceOutputPerspective();
}

public function testChangingDatasetResourcePerspectiveOnOutput() {
$this->datastoreImportAndQuery();

Expand Down

0 comments on commit 7da8269

Please sign in to comment.