From 172a1efc1b3e0c911be11c25c438f5f46c26c476 Mon Sep 17 00:00:00 2001 From: "andrii.zinkevych" Date: Tue, 6 Jul 2021 12:05:40 +0300 Subject: [PATCH 1/3] 33309: Eliminated AspectMock usage from TestGeneratorTest.php --- .../Util/TestGeneratorTest.php | 86 +++++++++++-------- .../Util/Filesystem/CestFileCreatorUtil.php | 66 ++++++++++++++ .../Util/TestGenerator.php | 13 +-- 3 files changed, 121 insertions(+), 44 deletions(-) create mode 100644 src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php index 8817d89c7..3bc06de59 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php @@ -3,16 +3,18 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); namespace tests\unit\Magento\FunctionalTestFramework\Util; -use AspectMock\Test as AspectMock; - +use Exception; +use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException; use Magento\FunctionalTestingFramework\Filter\FilterList; -use Magento\FunctionalTestingFramework\Test\Handlers\TestObjectHandler; use Magento\FunctionalTestingFramework\Test\Objects\ActionObject; use Magento\FunctionalTestingFramework\Test\Objects\TestHookObject; use Magento\FunctionalTestingFramework\Test\Objects\TestObject; +use Magento\FunctionalTestingFramework\Util\Filesystem\CestFileCreatorUtil; +use ReflectionProperty; use tests\unit\Util\MagentoTestCase; use Magento\FunctionalTestingFramework\Util\TestGenerator; use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; @@ -22,7 +24,7 @@ class TestGeneratorTest extends MagentoTestCase { /** - * Before method functionality + * Before method functionality. */ public function setUp(): void { @@ -30,35 +32,29 @@ public function setUp(): void } /** - * After method functionality + * After method functionality. * * @return void */ public function tearDown(): void { - AspectMock::clean(); GenerationErrorHandler::getInstance()->reset(); } /** * Basic test to check exceptions for incorrect entities. * - * @throws \Exception + * @return void + * @throws Exception */ - public function testEntityException() + public function testEntityException(): void { $actionObject = new ActionObject('fakeAction', 'comment', [ 'userInput' => '{{someEntity.entity}}' ]); $testObject = new TestObject("sampleTest", ["merge123" => $actionObject], [], [], "filename"); - - AspectMock::double(TestObjectHandler::class, ['initTestData' => '']); - $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $testObject]); - - AspectMock::double(TestGenerator::class, ['loadAllTestObjects' => ["sampleTest" => $testObject]]); - $testGeneratorObject->createAllTestFiles(null, []); // assert that no exception for createAllTestFiles and generation error is stored in GenerationErrorHandler @@ -69,11 +65,12 @@ public function testEntityException() } /** - * Tests that skipped tests do not have a fully generated body + * Tests that skipped tests do not have a fully generated body. * - * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + * @return void + * @throws TestReferenceException */ - public function testSkippedNoGeneration() + public function testSkippedNoGeneration(): void { $actionInput = 'fakeInput'; $actionObject = new ActionObject('fakeAction', 'comment', [ @@ -91,14 +88,22 @@ public function testSkippedNoGeneration() } /** - * Tests that skipped tests have a fully generated body when --allowSkipped is passed in + * Tests that skipped tests have a fully generated body when --allowSkipped is passed in. * - * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + * @return void + * @throws TestReferenceException */ - public function testAllowSkipped() + public function testAllowSkipped(): void { // Mock allowSkipped for TestGenerator - AspectMock::double(MftfApplicationConfig::class, ['allowSkipped' => true]); + $mockConfig = $this->createMock(MftfApplicationConfig::class); + $mockConfig->expects($this->any()) + ->method('allowSkipped') + ->willReturn(true); + + $property = new ReflectionProperty(MftfApplicationConfig::class, 'MFTF_APPLICATION_CONTEXT'); + $property->setAccessible(true); + $property->setValue($mockConfig); $actionInput = 'fakeInput'; $actionObject = new ActionObject('fakeAction', 'comment', [ @@ -128,17 +133,20 @@ public function testAllowSkipped() } /** - * Tests that TestGenerator createAllTestFiles correctly filters based on severity + * Tests that TestGenerator createAllTestFiles correctly filters based on severity. * - * @throws \Magento\FunctionalTestingFramework\Exceptions\TestReferenceException + * @return void + * @throws TestReferenceException */ - public function testFilter() + public function testFilter(): void { - // Mock filters for TestGenerator - AspectMock::double( - MftfApplicationConfig::class, - ['getFilterList' => new FilterList(['severity' => ["CRITICAL"]])] - ); + $mockConfig = $this->createMock(MftfApplicationConfig::class); + $fileList = new FilterList(['severity' => ["CRITICAL"]]); + $mockConfig->expects($this->once())->method('getFilterList')->willReturn($fileList); + + $property = new ReflectionProperty(MftfApplicationConfig::class, 'MFTF_APPLICATION_CONTEXT'); + $property->setAccessible(true); + $property->setValue($mockConfig); $actionInput = 'fakeInput'; $actionObject = new ActionObject('fakeAction', 'comment', [ @@ -161,16 +169,26 @@ public function testFilter() [], "filename" ); - AspectMock::double(TestGenerator::class, ['loadAllTestObjects' => ["sampleTest" => $test1, "test2" => $test2]]); // Mock createCestFile to return name of tests that testGenerator tried to create $generatedTests = []; - AspectMock::double(TestGenerator::class, ['createCestFile' => function ($arg1, $arg2) use (&$generatedTests) { - $generatedTests[$arg2] = true; - }]); + $cestFileCreatorUtil = $this->createMock(CestFileCreatorUtil::class); + $cestFileCreatorUtil->expects($this->once()) + ->method('create') + ->will( + $this->returnCallback( + function ($filename) use (&$generatedTests) { + $generatedTests[$filename] = true; + } + ) + ); + + $property = new ReflectionProperty(CestFileCreatorUtil::class, 'INSTANCE'); + $property->setAccessible(true); + $property->setValue($cestFileCreatorUtil); $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $test1, "test2" => $test2]); - $testGeneratorObject->createAllTestFiles(null, []); + $testGeneratorObject->createAllTestFiles(); // Ensure Test1 was Generated but not Test 2 $this->assertArrayHasKey('test1Cest', $generatedTests); diff --git a/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php new file mode 100644 index 000000000..a1c408359 --- /dev/null +++ b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php @@ -0,0 +1,66 @@ +exportDirectory); - $exportFilePath = $this->exportDirectory . DIRECTORY_SEPARATOR . $filename . ".php"; - $file = fopen($exportFilePath, 'w'); - - if (!$file) { - throw new TestFrameworkException(sprintf('Could not open test file: "%s"', $exportFilePath)); - } - - fwrite($file, $testPhp); - fclose($file); + CestFileCreatorUtil::getInstance()->create($filename, $this->exportDirectory, $testPhp); } /** From ea1b727d52373f0b514cea3c352e4119127c4d88 Mon Sep 17 00:00:00 2001 From: "andrii.zinkevych" Date: Tue, 6 Jul 2021 12:17:45 +0300 Subject: [PATCH 2/3] 33309: Fixed static-tests --- .../Util/Filesystem/CestFileCreatorUtil.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php index a1c408359..b3c896ccb 100644 --- a/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php +++ b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php @@ -21,7 +21,9 @@ class CestFileCreatorUtil /** * CestFileCreatorUtil constructor. */ - private function __construct(){} + private function __construct() + { + } /** * Get CestFileCreatorUtil instance. @@ -41,9 +43,9 @@ public static function getInstance() * Create a single PHP file containing the $cestPhp using the $filename. * If the _generated directory doesn't exist it will be created. * + * @param string $filename * @param string $exportDirectory * @param string $testPhp - * @param string $filename * * @return void * @throws TestFrameworkException From 43f943603db2128cbc90c9d2f61c852faaa6afec Mon Sep 17 00:00:00 2001 From: bohdan-harniuk Date: Thu, 22 Jul 2021 18:32:32 +0300 Subject: [PATCH 3/3] 33309: Code review/refactoring, fixing failed tests --- .../Util/TestGeneratorTest.php | 74 ++++++++++++------- .../Util/Filesystem/CestFileCreatorUtil.php | 6 +- 2 files changed, 50 insertions(+), 30 deletions(-) diff --git a/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php b/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php index 3bc06de59..730493191 100644 --- a/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php +++ b/dev/tests/unit/Magento/FunctionalTestFramework/Util/TestGeneratorTest.php @@ -8,25 +8,27 @@ namespace tests\unit\Magento\FunctionalTestFramework\Util; use Exception; +use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; use Magento\FunctionalTestingFramework\Exceptions\TestReferenceException; use Magento\FunctionalTestingFramework\Filter\FilterList; use Magento\FunctionalTestingFramework\Test\Objects\ActionObject; use Magento\FunctionalTestingFramework\Test\Objects\TestHookObject; use Magento\FunctionalTestingFramework\Test\Objects\TestObject; use Magento\FunctionalTestingFramework\Util\Filesystem\CestFileCreatorUtil; +use Magento\FunctionalTestingFramework\Util\GenerationErrorHandler; +use Magento\FunctionalTestingFramework\Util\TestGenerator; use ReflectionProperty; use tests\unit\Util\MagentoTestCase; -use Magento\FunctionalTestingFramework\Util\TestGenerator; -use Magento\FunctionalTestingFramework\Config\MftfApplicationConfig; use tests\unit\Util\TestLoggingUtil; -use Magento\FunctionalTestingFramework\Util\GenerationErrorHandler; class TestGeneratorTest extends MagentoTestCase { /** * Before method functionality. + * + * @return void */ - public function setUp(): void + protected function setUp(): void { TestLoggingUtil::getInstance()->setMockLoggingUtil(); } @@ -36,7 +38,7 @@ public function setUp(): void * * @return void */ - public function tearDown(): void + protected function tearDown(): void { GenerationErrorHandler::getInstance()->reset(); } @@ -53,12 +55,12 @@ public function testEntityException(): void 'userInput' => '{{someEntity.entity}}' ]); - $testObject = new TestObject("sampleTest", ["merge123" => $actionObject], [], [], "filename"); - $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $testObject]); + $testObject = new TestObject('sampleTest', ['merge123' => $actionObject], [], [], 'filename'); + $testGeneratorObject = TestGenerator::getInstance('', ['sampleTest' => $testObject]); $testGeneratorObject->createAllTestFiles(null, []); // assert that no exception for createAllTestFiles and generation error is stored in GenerationErrorHandler - $errorMessage = '/' . preg_quote("Removed invalid test object sampleTest") . '/'; + $errorMessage = '/' . preg_quote('Removed invalid test object sampleTest') . '/'; TestLoggingUtil::getInstance()->validateMockLogStatmentRegex('error', $errorMessage, []); $testErrors = GenerationErrorHandler::getInstance()->getErrorsByType('test'); $this->assertArrayHasKey('sampleTest', $testErrors); @@ -78,9 +80,9 @@ public function testSkippedNoGeneration(): void ]); $annotations = ['skip' => ['issue']]; - $testObject = new TestObject("sampleTest", ["merge123" => $actionObject], $annotations, [], "filename"); + $testObject = new TestObject('sampleTest', ['merge123' => $actionObject], $annotations, [], 'filename'); - $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $testObject]); + $testGeneratorObject = TestGenerator::getInstance('', ['sampleTest' => $testObject]); $output = $testGeneratorObject->assembleTestPhp($testObject); $this->assertStringContainsString('This test is skipped', $output); @@ -97,7 +99,7 @@ public function testAllowSkipped(): void { // Mock allowSkipped for TestGenerator $mockConfig = $this->createMock(MftfApplicationConfig::class); - $mockConfig->expects($this->any()) + $mockConfig ->method('allowSkipped') ->willReturn(true); @@ -115,16 +117,16 @@ public function testAllowSkipped(): void ]); $annotations = ['skip' => ['issue']]; - $beforeHook = new TestHookObject("before", "sampleTest", ['beforeAction' => $beforeActionObject]); + $beforeHook = new TestHookObject('before', 'sampleTest', ['beforeAction' => $beforeActionObject]); $testObject = new TestObject( - "sampleTest", - ["fakeAction" => $actionObject], + 'sampleTest', + ['fakeAction' => $actionObject], $annotations, - ["before" => $beforeHook], - "filename" + ['before' => $beforeHook], + 'filename' ); - $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $testObject]); + $testGeneratorObject = TestGenerator::getInstance('', ['sampleTest' => $testObject]); $output = $testGeneratorObject->assembleTestPhp($testObject); $this->assertStringNotContainsString('This test is skipped', $output); @@ -141,8 +143,10 @@ public function testAllowSkipped(): void public function testFilter(): void { $mockConfig = $this->createMock(MftfApplicationConfig::class); - $fileList = new FilterList(['severity' => ["CRITICAL"]]); - $mockConfig->expects($this->once())->method('getFilterList')->willReturn($fileList); + $fileList = new FilterList(['severity' => ['CRITICAL']]); + $mockConfig + ->method('getFilterList') + ->willReturn($fileList); $property = new ReflectionProperty(MftfApplicationConfig::class, 'MFTF_APPLICATION_CONTEXT'); $property->setAccessible(true); @@ -156,24 +160,24 @@ public function testFilter(): void $annotation1 = ['severity' => ['CRITICAL']]; $annotation2 = ['severity' => ['MINOR']]; $test1 = new TestObject( - "test1", - ["fakeAction" => $actionObject], + 'test1', + ['fakeAction' => $actionObject], $annotation1, [], - "filename" + 'filename' ); $test2 = new TestObject( - "test2", - ["fakeAction" => $actionObject], + 'test2', + ['fakeAction' => $actionObject], $annotation2, [], - "filename" + 'filename' ); // Mock createCestFile to return name of tests that testGenerator tried to create $generatedTests = []; $cestFileCreatorUtil = $this->createMock(CestFileCreatorUtil::class); - $cestFileCreatorUtil->expects($this->once()) + $cestFileCreatorUtil ->method('create') ->will( $this->returnCallback( @@ -187,11 +191,27 @@ function ($filename) use (&$generatedTests) { $property->setAccessible(true); $property->setValue($cestFileCreatorUtil); - $testGeneratorObject = TestGenerator::getInstance("", ["sampleTest" => $test1, "test2" => $test2]); + $testGeneratorObject = TestGenerator::getInstance('', ['sampleTest' => $test1, 'test2' => $test2]); $testGeneratorObject->createAllTestFiles(); // Ensure Test1 was Generated but not Test 2 $this->assertArrayHasKey('test1Cest', $generatedTests); $this->assertArrayNotHasKey('test2Cest', $generatedTests); } + + /** + * @inheritDoc + */ + public static function tearDownAfterClass(): void + { + parent::tearDownAfterClass(); + + $cestFileCreatorUtilInstance = new ReflectionProperty(CestFileCreatorUtil::class, 'INSTANCE'); + $cestFileCreatorUtilInstance->setAccessible(true); + $cestFileCreatorUtilInstance->setValue(null); + + $mftfAppConfigInstance = new ReflectionProperty(MftfApplicationConfig::class, 'MFTF_APPLICATION_CONTEXT'); + $mftfAppConfigInstance->setAccessible(true); + $mftfAppConfigInstance->setValue(null); + } } diff --git a/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php index b3c896ccb..9fe205151 100644 --- a/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php +++ b/src/Magento/FunctionalTestingFramework/Util/Filesystem/CestFileCreatorUtil.php @@ -30,10 +30,10 @@ private function __construct() * * @return CestFileCreatorUtil */ - public static function getInstance() + public static function getInstance(): CestFileCreatorUtil { - if (self::$INSTANCE === null) { - return new self(); + if (!self::$INSTANCE) { + self::$INSTANCE = new CestFileCreatorUtil(); } return self::$INSTANCE;