33309: Code review/refactoring, fixing failed tests

Author: bohdan-harniuk

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);
+    }
 }

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;