Skip to content

Commit 95b5c0d

Browse files
OcramiusOcramius
authored
Merge pull request Roave#76 from Roave/fix/Roave#66-Roave#72-Roave#73-handle-missing-cli-options
Roave#66 Roave#72 Roave#73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all
2 parents 9e95a60 + f3d92aa commit 95b5c0d

6 files changed

+364
-26
lines changed

‎phpunit.xml.dist

+3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
<testsuite name="unit">
1010
<directory>./test/unit</directory>
1111
</testsuite>
12+
<testsuite name="end to end">
13+
<directory>./test/e2e</directory>
14+
</testsuite>
1215
</testsuites>
1316
<filter>
1417
<whitelist processUncoveredFilesFromWhitelist="true">

‎src/Command/AssertBackwardsCompatible.php

+9-4
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ public function execute(InputInterface $input, OutputInterface $output) : int
148148
// @todo fix flaky assumption about the path of the source repo...
149149
$sourceRepo = CheckedOutRepository::fromPath(getcwd());
150150

151-
$fromRevision = $input->hasParameterOption('from')
151+
$fromRevision = $input->getOption('from') !== null
152152
? $this->parseRevisionFromInput($input, $sourceRepo)
153153
: $this->determineFromRevisionFromRepository($sourceRepo, $stdErr);
154154

@@ -226,10 +226,15 @@ private function determineFromRevisionFromRepository(
226226
CheckedOutRepository $repository,
227227
OutputInterface $output
228228
) : Revision {
229-
$versionString = $this->pickFromVersion->forVersions(
230-
$this->getVersions->fromRepository($repository)
231-
)->getVersionString();
229+
$versions = $this->getVersions->fromRepository($repository);
230+
231+
Assert::that($versions->count())
232+
->greaterThan(0, 'Could not detect any released versions for the given repository');
233+
234+
$versionString = $this->pickFromVersion->forVersions($versions)->getVersionString();
235+
232236
$output->writeln(sprintf('Detected last minor version: %s', $versionString));
237+
233238
return $this->parseRevision->fromStringForRepository(
234239
$versionString,
235240
$repository

‎src/Git/PickLastMinorVersionFromCollection.php

+21-13
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,15 @@
44

55
namespace Roave\BackwardCompatibility\Git;
66

7+
use Assert\Assert;
78
use Symfony\Component\Process\Exception\LogicException;
89
use Symfony\Component\Process\Exception\RuntimeException;
10+
use Version\Constraint\ComparisonConstraint;
11+
use Version\Constraint\CompositeConstraint;
912
use Version\Version;
1013
use Version\VersionsCollection;
14+
use function array_values;
1115
use function iterator_to_array;
12-
use function reset;
1316

1417
final class PickLastMinorVersionFromCollection implements PickVersionFromVersionCollection
1518
{
@@ -20,23 +23,28 @@ final class PickLastMinorVersionFromCollection implements PickVersionFromVersion
2023
*/
2124
public function forVersions(VersionsCollection $versions) : Version
2225
{
26+
Assert::that($versions->count())
27+
->greaterThan(0, 'Cannot determine latest minor version from an empty collection');
28+
2329
$versions->sort(VersionsCollection::SORT_DESC);
2430

25-
/** @var Version[] $versionsAsArray */
26-
$versionsAsArray = iterator_to_array($versions->getIterator());
2731
/** @var Version $lastVersion */
28-
$lastVersion = reset($versionsAsArray);
29-
$previousVersionInIteration = $lastVersion;
32+
$lastVersion = array_values(iterator_to_array($versions))[0];
33+
34+
$matchingMinorVersions = $versions->matching(new CompositeConstraint(
35+
CompositeConstraint::OPERATOR_AND,
36+
new ComparisonConstraint(ComparisonConstraint::OPERATOR_LTE, $lastVersion),
37+
new ComparisonConstraint(
38+
ComparisonConstraint::OPERATOR_GTE,
39+
Version::fromString($lastVersion->getMajor() . '.' . $lastVersion->getMinor() . '.0')
40+
)
41+
));
3042

31-
/** @var Version $version */
32-
foreach ($versions as $version) {
33-
if ($lastVersion->getMinor() !== $version->getMinor()) {
34-
return $previousVersionInIteration;
35-
}
43+
$matchingMinorVersions->sort(VersionsCollection::SORT_ASC);
3644

37-
$previousVersionInIteration = $version;
38-
}
45+
/** @var Version[] $matchingMinorVersionsAsArray */
46+
$matchingMinorVersionsAsArray = array_values(iterator_to_array($matchingMinorVersions));
3947

40-
return $previousVersionInIteration;
48+
return $matchingMinorVersionsAsArray[0];
4149
}
4250
}

‎test/e2e/Command/AssertBackwardsCompatibleTest.php

+276
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,276 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace RoaveE2ETest\BackwardCompatibility\Command;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use Symfony\Component\Process\Process;
9+
10+
/**
11+
* @coversNothing
12+
*/
13+
final class AssertBackwardsCompatibleTest extends TestCase
14+
{
15+
private const COMPOSER_MANIFEST = <<<'JSON'
16+
{
17+
"autoload": {
18+
"psr-4": {
19+
"TestArtifact\\": "src/"
20+
}
21+
},
22+
"repositories": [
23+
{
24+
"packagist.org": false
25+
}
26+
]
27+
}
28+
29+
JSON;
30+
31+
private const CLASS_VERSIONS = [
32+
<<<'PHP'
33+
<?php
34+
35+
namespace TestArtifact;
36+
37+
final class TheClass
38+
{
39+
public function method(A $a)
40+
{
41+
}
42+
}
43+
44+
PHP
45+
,
46+
<<<'PHP'
47+
<?php
48+
49+
namespace TestArtifact;
50+
51+
final class TheClass
52+
{
53+
public function method(B $a)
54+
{
55+
}
56+
}
57+
58+
PHP
59+
,
60+
<<<'PHP'
61+
<?php
62+
63+
namespace TestArtifact;
64+
65+
final class TheClass
66+
{
67+
public function method(C $a)
68+
{
69+
}
70+
}
71+
72+
PHP
73+
,
74+
// The last version resets the class to its initial state
75+
<<<'PHP'
76+
<?php
77+
78+
namespace TestArtifact;
79+
80+
final class TheClass
81+
{
82+
public function method(A $a)
83+
{
84+
}
85+
}
86+
87+
PHP
88+
];
89+
90+
/** @var string path to the sources that should be checked */
91+
private $sourcesRepository;
92+
93+
/** @var string[] sha1 of the source versions */
94+
private $versions = [];
95+
96+
protected function setUp() : void
97+
{
98+
parent::setUp();
99+
100+
$this->sourcesRepository = tempnam(sys_get_temp_dir(), 'roave-backward-compatibility-e2e-test');
101+
102+
self::assertInternalType('string', $this->sourcesRepository);
103+
self::assertNotEmpty($this->sourcesRepository);
104+
self::assertFileExists($this->sourcesRepository);
105+
106+
unlink($this->sourcesRepository);
107+
mkdir($this->sourcesRepository);
108+
mkdir($this->sourcesRepository . '/src');
109+
110+
self::assertDirectoryExists($this->sourcesRepository);
111+
self::assertDirectoryExists($this->sourcesRepository . '/src');
112+
113+
(new Process('git init', $this->sourcesRepository))->mustRun();
114+
115+
file_put_contents($this->sourcesRepository . '/composer.json', self::COMPOSER_MANIFEST);
116+
117+
(new Process('git add -A', $this->sourcesRepository))->mustRun();
118+
(new Process('git commit -am "Initial commit with composer manifest"', $this->sourcesRepository))->mustRun();
119+
120+
foreach (self::CLASS_VERSIONS as $key => $classCode) {
121+
file_put_contents($this->sourcesRepository . '/src/TheClass.php', $classCode);
122+
123+
(new Process('git add -A', $this->sourcesRepository))->mustRun();
124+
(new Process(sprintf('git commit -am "Class sources v%d"', $key + 1), $this->sourcesRepository))->mustRun();
125+
$this->versions[$key] = trim((new Process('git rev-parse HEAD', $this->sourcesRepository))->mustRun()
126+
->getOutput());
127+
}
128+
}
129+
130+
protected function tearDown() : void
131+
{
132+
self::assertInternalType('string', $this->sourcesRepository);
133+
self::assertNotEmpty($this->sourcesRepository);
134+
self::assertDirectoryExists($this->sourcesRepository);
135+
136+
// Need to be extremely careful with this stuff - skipping it for now
137+
(new Process(['rm', '-r', $this->sourcesRepository]))->mustRun();
138+
139+
parent::tearDown();
140+
}
141+
142+
public function testWillAllowSpecifyingGitRevision() : void
143+
{
144+
$check = new Process(
145+
[
146+
__DIR__ . '/../../../bin/roave-backward-compatibility-check',
147+
'--from=' . $this->versions[0],
148+
'--to=' . $this->versions[1],
149+
],
150+
$this->sourcesRepository
151+
);
152+
153+
self::assertSame(3, $check->run());
154+
self::assertStringEndsWith(
155+
<<<'EXPECTED'
156+
[BC] CHANGED: The parameter $a of TestArtifact\TheClass#method() changed from TestArtifact\A to a non-contravariant TestArtifact\B
157+
1 backwards-incompatible changes detected
158+
159+
EXPECTED
160+
,
161+
$check->getErrorOutput() // @TODO https://github.com/Roave/BackwardCompatibilityCheck/issues/79 this looks like a symfony bug - we shouldn't check STDERR, but STDOUT
162+
);
163+
}
164+
165+
public function testWillNotRunWithoutTagsNorSpecifiedVersions() : void
166+
{
167+
$check = new Process(
168+
__DIR__ . '/../../../bin/roave-backward-compatibility-check',
169+
$this->sourcesRepository
170+
);
171+
172+
self::assertSame(212, $check->run());
173+
self::assertContains(
174+
'Could not detect any released versions for the given repository',
175+
$check->getErrorOutput()
176+
);
177+
}
178+
179+
public function testWillRunSuccessfullyOnNoBcBreaks() : void
180+
{
181+
$check = new Process(
182+
[
183+
__DIR__ . '/../../../bin/roave-backward-compatibility-check',
184+
'--from=' . $this->versions[0],
185+
'--to=' . $this->versions[3],
186+
],
187+
$this->sourcesRepository
188+
);
189+
190+
self::assertSame(0, $check->run());
191+
self::assertContains(
192+
'No backwards-incompatible changes detected',
193+
$check->getErrorOutput()
194+
);
195+
}
196+
197+
public function testWillPickTaggedVersionOnNoGivenFrom() : void
198+
{
199+
$this->tagOnVersion('1.2.3', 1);
200+
201+
$check = new Process(
202+
[
203+
__DIR__ . '/../../../bin/roave-backward-compatibility-check',
204+
'--to=' . $this->versions[2],
205+
],
206+
$this->sourcesRepository
207+
);
208+
209+
self::assertSame(3, $check->run());
210+
211+
$errorOutput = $check->getErrorOutput();
212+
213+
self::assertContains('Detected last minor version: 1.2.3', $errorOutput);
214+
self::assertStringEndsWith(
215+
<<<'EXPECTED'
216+
[BC] CHANGED: The parameter $a of TestArtifact\TheClass#method() changed from TestArtifact\B to a non-contravariant TestArtifact\C
217+
1 backwards-incompatible changes detected
218+
219+
EXPECTED
220+
,
221+
$errorOutput // @TODO https://github.com/Roave/BackwardCompatibilityCheck/issues/79 this looks like a symfony bug - we shouldn't check STDERR, but STDOUT
222+
);
223+
}
224+
225+
public function testWillPickLatestTaggedVersionOnNoGivenFrom() : void
226+
{
227+
$this->tagOnVersion('2.2.3', 1);
228+
$this->tagOnVersion('1.2.3', 3);
229+
230+
$check = new Process(
231+
[
232+
__DIR__ . '/../../../bin/roave-backward-compatibility-check',
233+
'--to=' . $this->versions[2],
234+
],
235+
$this->sourcesRepository
236+
);
237+
238+
self::assertSame(3, $check->run());
239+
240+
$errorOutput = $check->getErrorOutput();
241+
242+
self::assertContains('Detected last minor version: 2.2.3', $errorOutput);
243+
self::assertStringEndsWith(
244+
<<<'EXPECTED'
245+
[BC] CHANGED: The parameter $a of TestArtifact\TheClass#method() changed from TestArtifact\B to a non-contravariant TestArtifact\C
246+
1 backwards-incompatible changes detected
247+
248+
EXPECTED
249+
,
250+
$errorOutput // @TODO https://github.com/Roave/BackwardCompatibilityCheck/issues/79 this looks like a symfony bug - we shouldn't check STDERR, but STDOUT
251+
);
252+
}
253+
254+
private function tagOnVersion(string $tagName, int $version) : void
255+
{
256+
(new Process(
257+
[
258+
'git',
259+
'checkout',
260+
$this->versions[$version],
261+
],
262+
$this->sourcesRepository
263+
))->mustRun();
264+
265+
(new Process(
266+
[
267+
'git',
268+
'tag',
269+
$tagName,
270+
'-m',
271+
'A tag for version ' . $version,
272+
],
273+
$this->sourcesRepository
274+
))->mustRun();
275+
}
276+
}

0 commit comments

Comments
 (0)