From e6798aac619562fe4f1f0e382000966fa81f8a69 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 5 Sep 2018 22:14:04 -0400 Subject: [PATCH 1/2] Fixing a bug in the tests where too many deps might be installed This could create a situation where we tell a user to install libraries A, B & C only, but really, we also depend on library D, which was not recommended because we are missing that dependency from our Maker. However, library D may have been installed in the functional test (because we previously installed ALL dependencies, even if it was related to a class we already had). --- src/Maker/MakeCrud.php | 1 + src/Test/MakerTestDetails.php | 11 ++++++++-- src/Test/MakerTestEnvironment.php | 35 ++++++++++++++++++++++++++++++- 3 files changed, 44 insertions(+), 3 deletions(-) diff --git a/src/Maker/MakeCrud.php b/src/Maker/MakeCrud.php index c87228a32..4c9d4da6d 100644 --- a/src/Maker/MakeCrud.php +++ b/src/Maker/MakeCrud.php @@ -13,6 +13,7 @@ use Doctrine\Bundle\DoctrineBundle\DoctrineBundle; use Doctrine\Common\Inflector\Inflector; +use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter; use Symfony\Bundle\FrameworkBundle\Controller\AbstractController; use Symfony\Bundle\MakerBundle\ConsoleStyle; use Symfony\Bundle\MakerBundle\DependencyBuilder; diff --git a/src/Test/MakerTestDetails.php b/src/Test/MakerTestDetails.php index 6669d4427..47daa709d 100644 --- a/src/Test/MakerTestDetails.php +++ b/src/Test/MakerTestDetails.php @@ -326,8 +326,7 @@ public function getAssert() public function getDependencies() { - $depBuilder = new DependencyBuilder(); - $this->maker->configureDependencies($depBuilder); + $depBuilder = $this->getDependencyBuilder(); return array_merge( $depBuilder->getAllRequiredDependencies(), @@ -336,6 +335,14 @@ public function getDependencies() ); } + public function getDependencyBuilder(): DependencyBuilder + { + $depBuilder = new DependencyBuilder(); + $this->maker->configureDependencies($depBuilder); + + return $depBuilder; + } + public function getArgumentsString(): string { return $this->argumentsString; diff --git a/src/Test/MakerTestEnvironment.php b/src/Test/MakerTestEnvironment.php index ebbed0c51..136995e69 100644 --- a/src/Test/MakerTestEnvironment.php +++ b/src/Test/MakerTestEnvironment.php @@ -83,7 +83,8 @@ public function prepare() $this->fs->mirror($this->flexPath, $this->path); // install any missing dependencies - if ($dependencies = $this->testDetails->getDependencies()) { + $dependencies = $this->determineMissingDependencies(); + if ($dependencies) { MakerTestProcess::create(sprintf('composer require %s', implode(' ', $dependencies)), $this->path) ->run(); } @@ -381,4 +382,36 @@ private function processReplacements(array $replacements, $rootDir) file_put_contents($path, str_replace($replacement['find'], $replacement['replace'], $contents)); } } + + /** + * Executes the DependencyBuilder for the Maker command inside the + * actual project, so we know exactly what dependencies we need or + * don't need. + */ + private function determineMissingDependencies(): array + { + $depBuilder = $this->testDetails->getDependencyBuilder(); + file_put_contents($this->path.'/dep_builder', serialize($depBuilder)); + file_put_contents($this->path.'/dep_runner.php', 'getMissingDependencies(), + $depBuilder->getMissingDevDependencies() +); +echo json_encode($missingDependencies); + '); + + $process = MakerTestProcess::create('php dep_runner.php', $this->path)->run(); + $data = json_decode($process->getOutput(), true); + if (null === $data) { + throw new \Exception('Could not determine dependencies'); + } + + unlink($this->path.'/dep_builder'); + unlink($this->path.'/dep_runner.php'); + + return $data; + } } From 1c3002cc1bca04f014582e657fc65731418fd4d2 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 6 Sep 2018 10:23:34 -0400 Subject: [PATCH 2/2] Fixing a bug where we were missing "extra" dependencies Also, one test case has an optional Doctrine dep, but we need it for the test - so, including it. --- src/Test/MakerTestDetails.php | 5 +++++ src/Test/MakerTestEnvironment.php | 2 +- src/Util/YamlSourceManipulator.php | 6 +++--- tests/Maker/FunctionalTest.php | 1 + 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Test/MakerTestDetails.php b/src/Test/MakerTestDetails.php index 47daa709d..466696d1e 100644 --- a/src/Test/MakerTestDetails.php +++ b/src/Test/MakerTestDetails.php @@ -335,6 +335,11 @@ public function getDependencies() ); } + public function getExtraDependencies() + { + return $this->extraDependencies; + } + public function getDependencyBuilder(): DependencyBuilder { $depBuilder = new DependencyBuilder(); diff --git a/src/Test/MakerTestEnvironment.php b/src/Test/MakerTestEnvironment.php index 136995e69..a095fc99a 100644 --- a/src/Test/MakerTestEnvironment.php +++ b/src/Test/MakerTestEnvironment.php @@ -412,6 +412,6 @@ private function determineMissingDependencies(): array unlink($this->path.'/dep_builder'); unlink($this->path.'/dep_runner.php'); - return $data; + return array_merge($data, $this->testDetails->getExtraDependencies()); } } diff --git a/src/Util/YamlSourceManipulator.php b/src/Util/YamlSourceManipulator.php index 65dcae266..2f0585122 100644 --- a/src/Util/YamlSourceManipulator.php +++ b/src/Util/YamlSourceManipulator.php @@ -450,7 +450,7 @@ private function changeValueInYaml($value) $endValuePosition = $this->findEndPositionOfValue($originalVal); $newYamlValue = $this->convertToYaml($value); - if (!is_array($originalVal) && is_array($value)) { + if (!\is_array($originalVal) && \is_array($value)) { // we're converting from a scalar to a (multiline) array // this means we need to break onto the next line @@ -798,7 +798,7 @@ private function advanceCurrentPosition(int $newPosition) * to look for indentation. */ if ($this->isCharLineBreak(substr($this->contents, $originalPosition - 1, 1))) { - $originalPosition--; + --$originalPosition; } // look for empty lines and track the current indentation @@ -890,7 +890,7 @@ private function guessNextArrayTypeAndAdvance(): string // because we are either moving along one line until [ { // or we are finding a line break and stopping: indentation // should not be calculated - $this->currentPosition++; + ++$this->currentPosition; if ($this->isCharLineBreak($nextCharacter)) { return self::ARRAY_FORMAT_MULTILINE; diff --git a/tests/Maker/FunctionalTest.php b/tests/Maker/FunctionalTest.php index c94bc9709..5f8929157 100644 --- a/tests/Maker/FunctionalTest.php +++ b/tests/Maker/FunctionalTest.php @@ -303,6 +303,7 @@ public function getCommandTests() ->addExtraDependencies('doctrine') ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeUserEntityPassword') ->configureDatabase() + ->addExtraDependencies('doctrine') ->setGuardAuthenticator('main', 'App\\Security\\AutomaticAuthenticator') ->setRequiredPhpVersion(70100) ->updateSchemaAfterCommand()