Skip to content

Commit 2250722

Browse files
committed
bug #260 Fixing a bug in the tests where too many deps might be installed (weaverryan)
This PR was squashed before being merged into the 1.0-dev branch (closes #260). Discussion ---------- 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). This would have caught #256. Commits ------- 27787c4 Fixing a bug in the tests where too many deps might be installed
2 parents 70bbe65 + 27787c4 commit 2250722

File tree

5 files changed

+53
-6
lines changed

5 files changed

+53
-6
lines changed

src/Maker/MakeCrud.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Doctrine\Bundle\DoctrineBundle\DoctrineBundle;
1515
use Doctrine\Common\Inflector\Inflector;
16+
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
1617
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
1718
use Symfony\Bundle\MakerBundle\ConsoleStyle;
1819
use Symfony\Bundle\MakerBundle\DependencyBuilder;

src/Test/MakerTestDetails.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,7 @@ public function getAssert()
326326

327327
public function getDependencies()
328328
{
329-
$depBuilder = new DependencyBuilder();
330-
$this->maker->configureDependencies($depBuilder);
329+
$depBuilder = $this->getDependencyBuilder();
331330

332331
return array_merge(
333332
$depBuilder->getAllRequiredDependencies(),
@@ -336,6 +335,19 @@ public function getDependencies()
336335
);
337336
}
338337

338+
public function getExtraDependencies()
339+
{
340+
return $this->extraDependencies;
341+
}
342+
343+
public function getDependencyBuilder(): DependencyBuilder
344+
{
345+
$depBuilder = new DependencyBuilder();
346+
$this->maker->configureDependencies($depBuilder);
347+
348+
return $depBuilder;
349+
}
350+
339351
public function getArgumentsString(): string
340352
{
341353
return $this->argumentsString;

src/Test/MakerTestEnvironment.php

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ public function prepare()
8383
$this->fs->mirror($this->flexPath, $this->path);
8484

8585
// install any missing dependencies
86-
if ($dependencies = $this->testDetails->getDependencies()) {
86+
$dependencies = $this->determineMissingDependencies();
87+
if ($dependencies) {
8788
MakerTestProcess::create(sprintf('composer require %s', implode(' ', $dependencies)), $this->path)
8889
->run();
8990
}
@@ -381,4 +382,36 @@ private function processReplacements(array $replacements, $rootDir)
381382
file_put_contents($path, str_replace($replacement['find'], $replacement['replace'], $contents));
382383
}
383384
}
385+
386+
/**
387+
* Executes the DependencyBuilder for the Maker command inside the
388+
* actual project, so we know exactly what dependencies we need or
389+
* don't need.
390+
*/
391+
private function determineMissingDependencies(): array
392+
{
393+
$depBuilder = $this->testDetails->getDependencyBuilder();
394+
file_put_contents($this->path.'/dep_builder', serialize($depBuilder));
395+
file_put_contents($this->path.'/dep_runner.php', '<?php
396+
397+
require __DIR__."/vendor/autoload.php";
398+
$depBuilder = unserialize(file_get_contents("dep_builder"));
399+
$missingDependencies = array_merge(
400+
$depBuilder->getMissingDependencies(),
401+
$depBuilder->getMissingDevDependencies()
402+
);
403+
echo json_encode($missingDependencies);
404+
');
405+
406+
$process = MakerTestProcess::create('php dep_runner.php', $this->path)->run();
407+
$data = json_decode($process->getOutput(), true);
408+
if (null === $data) {
409+
throw new \Exception('Could not determine dependencies');
410+
}
411+
412+
unlink($this->path.'/dep_builder');
413+
unlink($this->path.'/dep_runner.php');
414+
415+
return array_merge($data, $this->testDetails->getExtraDependencies());
416+
}
384417
}

src/Util/YamlSourceManipulator.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ private function changeValueInYaml($value)
450450
$endValuePosition = $this->findEndPositionOfValue($originalVal);
451451

452452
$newYamlValue = $this->convertToYaml($value);
453-
if (!is_array($originalVal) && is_array($value)) {
453+
if (!\is_array($originalVal) && \is_array($value)) {
454454
// we're converting from a scalar to a (multiline) array
455455
// this means we need to break onto the next line
456456

@@ -798,7 +798,7 @@ private function advanceCurrentPosition(int $newPosition)
798798
* to look for indentation.
799799
*/
800800
if ($this->isCharLineBreak(substr($this->contents, $originalPosition - 1, 1))) {
801-
$originalPosition--;
801+
--$originalPosition;
802802
}
803803

804804
// look for empty lines and track the current indentation
@@ -890,7 +890,7 @@ private function guessNextArrayTypeAndAdvance(): string
890890
// because we are either moving along one line until [ {
891891
// or we are finding a line break and stopping: indentation
892892
// should not be calculated
893-
$this->currentPosition++;
893+
++$this->currentPosition;
894894

895895
if ($this->isCharLineBreak($nextCharacter)) {
896896
return self::ARRAY_FORMAT_MULTILINE;

tests/Maker/FunctionalTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@ public function getCommandTests()
303303
->addExtraDependencies('doctrine')
304304
->setFixtureFilesPath(__DIR__.'/../fixtures/MakeUserEntityPassword')
305305
->configureDatabase()
306+
->addExtraDependencies('doctrine')
306307
->setGuardAuthenticator('main', 'App\\Security\\AutomaticAuthenticator')
307308
->setRequiredPhpVersion(70100)
308309
->updateSchemaAfterCommand()

0 commit comments

Comments
 (0)