magento/issue#19481: minor and codestyle fixes

Author: andrewbess

app/code/Magento/SampleData/Console/Command/SampleDataDeployCommand.php

@@ -10,9 +10,12 @@
 use Composer\Console\ApplicationFactory;
 use Exception;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Console\Cli;
 use Magento\Framework\Exception\FileSystemException;
+use Magento\Framework\Exception\InvalidArgumentException;
 use Magento\Framework\Exception\LocalizedException;
 use Magento\Framework\Filesystem;
+use Magento\Framework\Serialize\Serializer\Json;
 use Magento\SampleData\Model\Dependency;
 use Magento\Setup\Model\PackagesAuth;
 use Symfony\Component\Console\Command\Command;
@@ -24,6 +27,8 @@
 
 /**
  * Command for deployment of Sample Data
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class SampleDataDeployCommand extends Command
 {
@@ -50,22 +55,30 @@ class SampleDataDeployCommand extends Command
      */
     private $applicationFactory;
 
+    /**
+     * @var Json
+     */
+    private $serializer;
+
     /**
      * @param Filesystem $filesystem
      * @param Dependency $sampleDataDependency
      * @param ArrayInputFactory $arrayInputFactory
      * @param ApplicationFactory $applicationFactory
+     * @param Json $serializer
      */
     public function __construct(
         Filesystem $filesystem,
         Dependency $sampleDataDependency,
         ArrayInputFactory $arrayInputFactory,
-        ApplicationFactory $applicationFactory
+        ApplicationFactory $applicationFactory,
+        Json $serializer
     ) {
         $this->filesystem = $filesystem;
         $this->sampleDataDependency = $sampleDataDependency;
         $this->arrayInputFactory = $arrayInputFactory;
         $this->applicationFactory = $applicationFactory;
+        $this->serializer = $serializer;
         parent::__construct();
     }
 
@@ -90,20 +103,20 @@ protected function configure()
      *
      * @param InputInterface $input
      * @param OutputInterface $output
-     * @return int|void
+     * @return int
      * @throws FileSystemException
      * @throws LocalizedException
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        $rootJson = json_decode(
+        $rootJson = $this->serializer->unserialize(
             $this->filesystem->getDirectoryRead(
                 DirectoryList::ROOT
             )->readFile("composer.json")
         );
-        if (!isset($rootJson->version)) {
+        if (!isset($rootJson['version'])) {
             $magentoProductPackage = array_filter(
-                (array) $rootJson->require,
+                $rootJson['require'],
                 function ($package) {
                     return false !== strpos($package, 'magento/product-');
                 },
@@ -156,9 +169,15 @@ function ($package) {
                     . '</info>'
                 );
                 $application->resetComposer();
+
+                return Cli::RETURN_FAILURE;
             }
+
+            return Cli::RETURN_SUCCESS;
         } else {
             $output->writeln('<info>' . 'There is no sample data for current set of modules.' . '</info>');
+
+            return Cli::RETURN_FAILURE;
         }
     }
 
@@ -189,15 +208,30 @@ private function createAuthFile()
     /**
      * Updates PHP memory limit
      *
+     * @throws InvalidArgumentException
      * @return void
      */
     private function updateMemoryLimit()
     {
         if (function_exists('ini_set')) {
-            @ini_set('display_errors', 1);
+            $result = ini_alter('display_errors', 1);
+            if ($result === false) {
+                $error = error_get_last();
+                throw new InvalidArgumentException(__(
+                    'Failed to set ini option display_errors to value 1. %1',
+                    $error['message']
+                ));
+            }
             $memoryLimit = trim(ini_get('memory_limit'));
             if ($memoryLimit != -1 && $this->getMemoryInBytes($memoryLimit) < 756 * 1024 * 1024) {
-                @ini_set('memory_limit', '756M');
+                $result = ini_alter('memory_limit', '756M');
+                if ($result === false) {
+                    $error = error_get_last();
+                    throw new InvalidArgumentException(__(
+                        'Failed to set ini option memory_limit to 756M. %1',
+                        $error['message']
+                    ));
+                }
             }
         }
     }

app/code/Magento/SampleData/Test/Unit/Console/Command/AbstractSampleDataCommandTest.php

@@ -26,6 +26,16 @@
  */
 abstract class AbstractSampleDataCommandTest extends TestCase
 {
+    /*
+     * Expected arguments for `composer config` to set missing field "version"
+     */
+    private const STUB_EXPECTED_COMPOSER_CONFIG = [
+        'command' => 'config',
+        'setting-key' => 'version',
+        'setting-value' => ['0.0.1'],
+        '--quiet' => 1
+    ];
+
     /**
      * @var ReadInterface|MockObject
      */
@@ -121,7 +131,7 @@ protected function setupMocks(
                 ->withConsecutive(
                     [
                         'input' => new ArrayInput(
-                            $this->expectedComposerArgumentsCommandConfig()
+                            self::STUB_EXPECTED_COMPOSER_CONFIG
                         ),
                         'output' => $this->anything()
                     ],

app/code/Magento/SampleData/Test/Unit/Console/Command/SampleDataDeployCommandTest.php

@@ -9,15 +9,46 @@
 
 use Exception;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Serialize\Serializer\Json;
 use Magento\SampleData\Console\Command\SampleDataDeployCommand;
 use Magento\Setup\Model\PackagesAuth;
+use PHPUnit\Framework\MockObject\MockObject;
 use Symfony\Component\Console\Tester\CommandTester;
 
 /**
  * Tests for command `sampledata:deploy`
  */
 class SampleDataDeployCommandTest extends AbstractSampleDataCommandTest
 {
+    /**
+     * @var Json|MockObject
+     */
+    private $serializerMock;
+
+    /**
+     * Creates mocks
+     *
+     * @return void
+     */
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->serializerMock = $this->createMock(Json::class);
+    }
+
+    /**
+     * Sets mock for unserialization composer content
+     * @param array $composerJsonContent
+     * @return void
+     */
+    protected function setupMockForSerializer(array $composerJsonContent): void
+    {
+        $this->serializerMock->expects($this->any())
+            ->method('unserialize')
+            ->will($this->returnValue($composerJsonContent));
+    }
+
     /**
      * Sets mocks for auth file
      *
@@ -63,6 +94,7 @@ public function testExecute(
             $composerJsonContent
         );
         $this->setupMocksForAuthFile($authExist);
+        $this->setupMockForSerializer($composerJsonContent);
         $commandTester = $this->createCommandTester();
         $commandTester->execute([]);
 
@@ -94,6 +126,7 @@ public function testExecuteWithNoUpdate(
             ['--no-update' => 1]
         );
         $this->setupMocksForAuthFile($authExist);
+        $this->setupMockForSerializer($composerJsonContent);
         $commandInput = ['--no-update' => 1];
 
         $commandTester = $this->createCommandTester();
@@ -172,6 +205,12 @@ public function testExecuteWithException()
             ->method('readFile')
             ->with('composer.json')
             ->willReturn('{"require": {"magento/product-community-edition": "0.0.1"}, "version": "0.0.1"}');
+        $this->serializerMock->expects($this->any())
+            ->method('unserialize')
+            ->will($this->returnValue([
+                'require' => ["magento/product-community-edition" => "0.0.1"],
+                'version' => '0.0.1'
+            ]));
         $this->filesystemMock->expects($this->once())
             ->method('getDirectoryRead')
             ->with(DirectoryList::ROOT)
@@ -208,26 +247,12 @@ private function createCommandTester(): CommandTester
                 $this->filesystemMock,
                 $this->sampleDataDependencyMock,
                 $this->arrayInputFactoryMock,
-                $this->applicationFactoryMock
+                $this->applicationFactoryMock,
+                $this->serializerMock
             )
         );
     }
 
-    /**
-     * Expected arguments for `composer config` to set missing field "version"
-     *
-     * @return array
-     */
-    protected function expectedComposerArgumentsCommandConfig(): array
-    {
-        return [
-            'command' => 'config',
-            'setting-key' => 'version',
-            'setting-value' => ['0.0.1'],
-            '--quiet' => 1
-        ];
-    }
-
     /**
      * Returns expected arguments for command `composer require`
      *

lib/internal/Magento/Framework/Exception/InvalidArgumentException.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Framework\Exception;
+
+/**
+ * An exception to be thrown when arguments are invalidated
+ */
+class InvalidArgumentException extends LocalizedException
+{
+}