Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

magento/magento2#28579:DependencyTest does not analyze GraphQL schema… #28747

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions app/code/Magento/SwatchesGraphQl/composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
"php": "~7.3.0||~7.4.0",
"magento/framework": "*",
"magento/module-swatches": "*",
"magento/module-catalog": "*"
},
"suggest": {
"magento/module-catalog": "*",
"magento/module-catalog-graph-ql": "*"
},
"license": [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,11 @@ protected function setUp(): void
$fileResolverMock = $this->getMockBuilder(
\Magento\Framework\Config\FileResolverInterface::class
)->disableOriginalConstructor()->getMock();
$filePath1 = __DIR__ . '/../_files/schemaA.graphqls';
$filePath2 = __DIR__ . '/../_files/schemaB.graphqls';
$fileList = [
file_get_contents(__DIR__ . '/../_files/schemaA.graphqls'),
file_get_contents(__DIR__ . '/../_files/schemaB.graphqls')
$filePath1 => file_get_contents($filePath1),
$filePath2 => file_get_contents($filePath2)
];
$fileResolverMock->expects($this->any())->method('get')->willReturn($fileList);
$graphQlReader = $this->objectManager->create(
Expand Down Expand Up @@ -219,31 +221,25 @@ function ($a, $b) {
}
//Checks to make sure that the given description exists in the expectedOutput array
$this->assertArrayHasKey(

array_search(
'Comment for empty PhysicalProductInterface',
array_column($expectedOutput, 'description')
),
$expectedOutput

array_search(
'Comment for empty PhysicalProductInterface',
array_column($expectedOutput, 'description')
),
$expectedOutput
);
$this->assertArrayHasKey(

array_search(
'Comment for empty Enum',
array_column($expectedOutput, 'description')
),
$expectedOutput

array_search(
'Comment for empty Enum',
array_column($expectedOutput, 'description')
),
$expectedOutput
);
$this->assertArrayHasKey(

array_search(
'Comment for SearchResultPageInfo',
array_column($expectedOutput, 'description')
),
$expectedOutput

array_search(
'Comment for SearchResultPageInfo',
array_column($expectedOutput, 'description')
),
$expectedOutput
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,24 @@ protected function setUp(): void
$fileResolverMock = $this->getMockBuilder(
\Magento\Framework\Config\FileResolverInterface::class
)->disableOriginalConstructor()->getMock();
$filePath1 = __DIR__ . '/_files/schemaC.graphqls';
$filePath2 = __DIR__ . '/_files/schemaD.graphqls';
$fileList = [
file_get_contents(__DIR__ . '/_files/schemaC.graphqls'),
file_get_contents(__DIR__ . '/_files/schemaD.graphqls')
$filePath1 => file_get_contents($filePath1),
$filePath2 => file_get_contents($filePath2)
];
$fileResolverMock->expects($this->any())->method('get')->willReturn($fileList);
$graphQlReader = $objectManager->create(
\Magento\Framework\GraphQlSchemaStitching\GraphQlReader::class,
['fileResolver' => $fileResolverMock]
);
$reader = $objectManager->create(
// phpstan:ignore
\Magento\Framework\GraphQlSchemaStitching\Reader::class,
['readers' => ['graphql_reader' => $graphQlReader]]
);
$data = $objectManager->create(
// phpstan:ignore
\Magento\Framework\GraphQl\Config\Data ::class,
['reader' => $reader]
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,20 @@

namespace Magento\Test\Integrity;

use Magento\Test\Integrity\Dependency\DeclarativeSchemaDependencyProvider;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\Utility\AggregateInvoker;
use Magento\Framework\App\Utility\Files;
use Magento\Framework\Component\ComponentRegistrar;
use Magento\Framework\Exception\LocalizedException;
use Magento\Test\Integrity\Dependency\DeclarativeSchemaDependencyProvider;
use Magento\TestFramework\Inspection\Exception as InspectionException;
use PHPUnit\Framework\TestCase;

/**
* Class DeclarativeDependencyTest
* Test for undeclared dependencies in declarative schema
*/
class DeclarativeDependencyTest extends \PHPUnit\Framework\TestCase
class DeclarativeDependencyTest extends TestCase
{
/**
* @var DeclarativeSchemaDependencyProvider
Expand All @@ -25,7 +31,7 @@ class DeclarativeDependencyTest extends \PHPUnit\Framework\TestCase
/**
* Sets up data
*
* @throws \Exception
* @throws InspectionException
*/
protected function setUp(): void
{
Expand All @@ -37,15 +43,16 @@ protected function setUp(): void
'MAGETWO-43654: The build is running from vendor/magento. DependencyTest is skipped.'
);
}
$this->dependencyProvider = new DeclarativeSchemaDependencyProvider();
$objectManager = ObjectManager::getInstance();
$this->dependencyProvider = $objectManager->create(DeclarativeSchemaDependencyProvider::class);
}

/**
* @throws \Exception
* @throws LocalizedException
*/
public function testUndeclaredDependencies()
{
$invoker = new \Magento\Framework\App\Utility\AggregateInvoker($this);
$invoker = new AggregateInvoker($this);
$invoker(
/**
* Check undeclared modules dependencies for specified file
Expand Down Expand Up @@ -107,7 +114,7 @@ private function prepareFiles(array $files): array
*/
private function getErrorMessage(string $id): string
{
$decodedId = $this->dependencyProvider->decodeDependencyId($id);
$decodedId = DeclarativeSchemaDependencyProvider::decodeDependencyId($id);
$entityType = $decodedId['entityType'];
if ($entityType === DeclarativeSchemaDependencyProvider::SCHEMA_ENTITY_TABLE) {
$message = sprintf(
Expand All @@ -131,14 +138,13 @@ private function getErrorMessage(string $id): string
*
* @param string $file
* @return mixed
* @throws \Exception
* @throws InspectionException
*/
private function readJsonFile(string $file, bool $asArray = false)
{
$decodedJson = json_decode(file_get_contents($file), $asArray);
if (null == $decodedJson) {
//phpcs:ignore Magento2.Exceptions.DirectThrow
throw new \Exception("Invalid Json: $file");
throw new InspectionException("Invalid Json: $file");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed this, we usually do a localized exception that's translatable and any parameters like file go into a "%1" or "%s" if using sprintf.
This is probably fine because it's a test. But "Invalid Json: $file" would still be a risky practice and sprintf would be preferred.

}

return $decodedJson;
Expand Down
Loading