#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all #76

Ocramius · 2018-05-28T09:53:29Z

Fixes #66
Fixes #72
Fixes #73

…OR* is picked Previously, only the *MINOR* version was being compared, leading to previous major versions being considered in the lookup for the lowest available stable release.

…itories The reasoning behind this is that `symfony/console` has a very squishy API, and that can lead to a lot of headaches in upgrades or implicit API changes. Having an E2E test will ensure that the implementation details of `symfony/console` are also considered, since the interfaces really just lie.

…le-missing-cli-options Close #66

…se from empty `git tag` list

… comparisons on no valid detected versions

…d in the selected stable minor releases

Ocramius · 2018-05-28T11:10:56Z

@bendavies I integrated your commits.

@ciaranmcnulty any suggestion for scenarios that I may not have covered yet?

Ocramius · 2018-05-28T11:11:17Z

test/e2e/Command/AssertBackwardsCompatibleTest.php

+            ],
+            $this->sourcesRepository
+        ))->mustRun();
+        (new Process(


These operations should likely be isolated in a private method

Ocramius · 2018-05-28T11:11:53Z

test/e2e/Command/AssertBackwardsCompatibleTest.php

+
+EXPECTED
+            ,
+            $errorOutput // @TODO this looks like a symfony bug - we shouldn't check STDERR, but STDOUT


This is a problem. For some reason, everything is going to STDERR in my tests (tried manually via bash, so the bug is with symfony/console or with how we use it)

symfony/symfony#13730 ?

Looked it up as well - unsure if that's the actual bug. I'll probably need to dump the streams and see if both point to STDERR by accident, because the output sure only is on STDERR.

Umm, we are manually splitting output between stderr/stdout ourselves.

If you specify --format=markdown, stdout is formatted markdown: stderr contains the human-readable output, so this is the expectation. Not a symfony bug.

See these for stderr being used:

BackwardCompatibilityCheck/src/Command/AssertBackwardsCompatible.php

Line 146 in 9e95a60

$stdErr = $output->getErrorOutput();

BackwardCompatibilityCheck/src/Command/AssertBackwardsCompatible.php

Lines 151 to 153 in 9e95a60

$fromRevision = $input->hasParameterOption('from')

? $this->parseRevisionFromInput($input, $sourceRepo)

: $this->determineFromRevisionFromRepository($sourceRepo, $stdErr);

BackwardCompatibilityCheck/src/Command/AssertBackwardsCompatible.php

Line 185 in 9e95a60

(new SymfonyConsoleTextFormatter($stdErr))->write($changes);

And then, markdown formatting if required:

BackwardCompatibilityCheck/src/Command/AssertBackwardsCompatible.php

Lines 190 to 192 in 9e95a60

if (ArrayHelpers::stringArrayContainsString('markdown', $outputFormats)) {

(new MarkdownPipedToSymfonyConsoleFormatter($output))->write($changes);

}

This was done specifically so you can pipe output to a file - see PR #35 , specifically #35 (comment)

Yes, but in the test (also manual ones), redirecting output doesn't seem to have any effect

Moved to #79

Ocramius · 2018-05-28T11:12:12Z

test/e2e/Command/AssertBackwardsCompatibleTest.php

+
+JSON;
+
+    private const CLASS_V1 = <<<'PHP'


These constants should be moved to an array

…ted in #79

…rray const`

Ref: #76 (comment)

Ocramius · 2018-06-04T16:05:05Z

🚢

Ref: Roave/BackwardCompatibilityCheck#76 (comment)

bendavies and others added 8 commits May 27, 2018 12:37

fix when no from option is given

8abf857

update tests

7b6e1ce

#66 #72 #73 ensure that the latest minor version for the current *MAJ…

c30e153

…OR* is picked Previously, only the *MINOR* version was being compared, leading to previous major versions being considered in the lookup for the lowest available stable release.

Merge remote-tracking branch 'origin/pr/66' into fix/#66-#72-#73-hand…

062e718

…le-missing-cli-options Close #66

#66 #72 #73 removed leftover var_dump() in tests (I know, I know...)

c0a863f

#66 #72 #73 AssertBackardsCompatible no longer fetches latest relea…

a07f172

…se from empty `git tag` list

#66 #72 #73 ensuring that AssertBackardsCompatible will not attempt…

a5ae097

… comparisons on no valid detected versions

Ocramius added the bug label May 28, 2018

Ocramius added this to the 1.0.3 milestone May 28, 2018

Ocramius self-assigned this May 28, 2018

Ocramius requested a review from asgrim May 28, 2018 09:53

Ocramius added 2 commits May 28, 2018 11:59

#66 #72 #73 ensuring that development/beta/alpha tags are not include…

8e23685

…d in the selected stable minor releases

#66 #72 #73 automated CS check fixes

6c797c5

Ocramius commented May 28, 2018

View reviewed changes

This was referenced May 30, 2018

Throw exception if repository does not contain any tagged version #78

Closed

STDERR and STDOUT do not seem to be properly separated in the symfony console output #79

Open

Ocramius added 3 commits June 4, 2018 17:42

#66 #72 #73 documented TODO about STDERR/STDOUT mixing bug as documen…

5768cb1

…ted in #79

#66 #72 #73 segregating class versions in e2e tests into a `private a…

a609052

…rray const`

#66 #72 #73 isolated tag creation to private method

f3d92aa

Ref: #76 (comment)

Ocramius merged commit 95b5c0d into master Jun 4, 2018

Ocramius deleted the fix/#66-#72-#73-handle-missing-cli-options branch June 4, 2018 16:05

uzibhalepu added a commit to uzibhalepu/BackwardCompatibilityCheck that referenced this pull request Aug 6, 2024

#66 #72 #73 isolated tag creation to private method

e9ac4d3

Ref: Roave/BackwardCompatibilityCheck#76 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all #76

#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all #76

Uh oh!

Ocramius commented May 28, 2018

Uh oh!

Ocramius commented May 28, 2018

Uh oh!

Ocramius May 28, 2018

Uh oh!

Ocramius May 28, 2018

Uh oh!

bendavies May 28, 2018

Uh oh!

Ocramius May 28, 2018

Uh oh!

asgrim May 29, 2018

Uh oh!

Ocramius May 29, 2018

Uh oh!

Ocramius Jun 4, 2018

Uh oh!

Ocramius May 28, 2018

Uh oh!

Ocramius commented Jun 4, 2018

Uh oh!

Uh oh!

	$fromRevision = $input->hasParameterOption('from')
	? $this->parseRevisionFromInput($input, $sourceRepo)
	: $this->determineFromRevisionFromRepository($sourceRepo, $stdErr);

	if (ArrayHelpers::stringArrayContainsString('markdown', $outputFormats)) {
	(new MarkdownPipedToSymfonyConsoleFormatter($output))->write($changes);
	}

#66 #72 #73 fixed missing --from handling, minor version detection and scenario with no detected versions at all #76

#66 #72 #73 fixed missing --from handling, minor version detection and scenario with no detected versions at all #76

Uh oh!

Conversation

Ocramius commented May 28, 2018

Uh oh!

Ocramius commented May 28, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ocramius commented Jun 4, 2018

Uh oh!

Uh oh!

#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all #76

#66 #72 #73 fixed missing `--from` handling, minor version detection and scenario with no detected versions at all #76