Skip to content

Commit

Permalink
feat: Simplify the diff command comparison (#1053)
Browse files Browse the repository at this point in the history 
Now that the equality check works as expected (see #1049), we can
simplify the way the diff command works by checking the equality once
and skip the "archive" and "content" comparisons if the archives are
equal.
  • Loading branch information
@theofidry
theofidry authored Oct 11, 2023
1 parent b8065e6 commit d733d96
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 73 deletions.
48 changes: 23 additions & 25 deletions src/Console/Command/Diff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,28 @@ public function getConfiguration(): Configuration

public function execute(IO $io): int
{
$paths = self::getPaths($io);
$diff = new PharDiff(...self::getPaths($io));
$diffMode = self::getDiffMode($io);

$io->comment('<info>Comparing the two archives...</info>');

$pharInfoA = $diff->getPharInfoA();
$pharInfoB = $diff->getPharInfoB();

if ($pharInfoA->equals($pharInfoB)) {
$io->success('The two archives are identical.');

$diff = new PharDiff(...$paths);
return ExitCode::SUCCESS;
}

$result1 = $this->compareArchives($diff, $io);
$result2 = $this->compareContents($diff, $io);
$this->compareArchives($diff, $io);
$this->compareContents($diff, $diffMode, $io);

return $result1 + $result2;
return ExitCode::FAILURE;
}

/**
* @return list<non-empty-string>
* @return array{non-empty-string, non-empty-string}
*/
private static function getPaths(IO $io): array
{
Expand All @@ -142,19 +152,11 @@ private static function getPaths(IO $io): array
);
}

private function compareArchives(PharDiff $diff, IO $io): int
private function compareArchives(PharDiff $diff, IO $io): void
{
$io->comment('<info>Comparing the two archives... (do not check the signatures)</info>');

$pharInfoA = $diff->getPharInfoA();
$pharInfoB = $diff->getPharInfoB();

if ($pharInfoA->equals($pharInfoB)) {
$io->success('The two archives are identical');

return ExitCode::SUCCESS;
}

self::renderArchive(
$diff->getPharInfoA()->getFileName(),
$pharInfoA,
Expand All @@ -168,8 +170,6 @@ private function compareArchives(PharDiff $diff, IO $io): int
$pharInfoB,
$io,
);

return ExitCode::FAILURE;
}

private static function getDiffMode(IO $io): DiffMode
Expand Down Expand Up @@ -216,31 +216,31 @@ private static function getDiffMode(IO $io): DiffMode
return DiffMode::from($io->getOption(self::DIFF_OPTION)->asNonEmptyString());
}

private function compareContents(PharDiff $diff, IO $io): int
private function compareContents(PharDiff $diff, DiffMode $diffMode, IO $io): void
{
$io->comment('<info>Comparing the two archives contents...</info>');

$checkSumAlgorithm = $io->getOption(self::CHECK_OPTION)->asNullableNonEmptyString() ?? self::DEFAULT_CHECKSUM_ALGO;

if ($io->hasOption('-c') || $io->hasOption('--check')) {
return $diff->listChecksums($checkSumAlgorithm);
}
$diff->listChecksums($checkSumAlgorithm);

$diffMode = self::getDiffMode($io);
return;
}

$diffResult = $diff->diff($diffMode);

if (null === $diffResult || [[], []] === $diffResult) {
$io->success('The contents are identical');

return ExitCode::SUCCESS;
return;
}

if (is_string($diffResult)) {
// Git or GNU diff: we don't have much control on the format
$io->writeln($diffResult);

return ExitCode::FAILURE;
return;
}

$io->writeln(sprintf(
Expand All @@ -263,8 +263,6 @@ private function compareContents(PharDiff $diff, IO $io): int
'%d file(s) difference',
count($diffResult[0]) + count($diffResult[1]),
));

return ExitCode::FAILURE;
}

/**
Expand Down
73 changes: 25 additions & 48 deletions tests/Console/Command/DiffTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,11 @@ public function test_it_can_display_the_list_diff_of_two_phar_files(): void
);

$expectedOutput = <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
[OK] The two archives are identical
// Comparing the two archives contents...
⚠️ <warning>Using the option "list-diff" is deprecated. Use "--diff=list" instead.</warning>
[OK] The contents are identical
// Comparing the two archives...
[OK] The two archives are identical.


OUTPUT;
Expand Down Expand Up @@ -138,16 +133,11 @@ public function test_it_can_display_the_git_diff_of_two_phar_files(): void
);

$expectedOutput = <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
[OK] The two archives are identical
// Comparing the two archives contents...
⚠️ <warning>Using the option "list-diff" is deprecated. Use "--diff=list" instead.</warning>
[OK] The contents are identical
// Comparing the two archives...
[OK] The two archives are identical.


OUTPUT;
Expand All @@ -172,16 +162,11 @@ public function test_it_can_display_the_gnu_diff_of_two_phar_files(): void
);

$expectedOutput = <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
[OK] The two archives are identical
// Comparing the two archives contents...
⚠️ <warning>Using the option "gnu-diff" is deprecated. Use "--diff=gnu" instead.</warning>
[OK] The contents are identical
// Comparing the two archives...
[OK] The two archives are identical.


OUTPUT;
Expand Down Expand Up @@ -288,13 +273,9 @@ public function test_it_can_compare_phar_files_without_the_phar_extension(): voi

$expected = <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
[OK] The two archives are identical
// Comparing the two archives...
// Comparing the two archives contents...
[OK] The contents are identical
[OK] The two archives are identical.


OUTPUT;
Expand Down Expand Up @@ -360,13 +341,9 @@ private static function commonDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-foo.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
[OK] The two archives are identical
// Comparing the two archives contents...
// Comparing the two archives...
[OK] The contents are identical
[OK] The two archives are identical.


OUTPUT,
Expand All @@ -378,7 +355,7 @@ private static function commonDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-bar-compressed.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-bar.phar
Archive Compression: None
Expand All @@ -402,7 +379,7 @@ private static function commonDiffPharsProvider(): iterable


OUTPUT,
1,
ExitCode::FAILURE,
];
}

Expand All @@ -415,7 +392,7 @@ private static function listDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-bar.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-foo.phar
Archive Compression: None
Expand Down Expand Up @@ -446,15 +423,15 @@ private static function listDiffPharsProvider(): iterable


OUTPUT,
2,
ExitCode::FAILURE,
];

yield 'same files different content' => [
self::FIXTURES_DIR.'/simple-phar-bar.phar',
self::FIXTURES_DIR.'/simple-phar-baz.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-bar.phar
Archive Compression: None
Expand Down Expand Up @@ -491,7 +468,7 @@ public static function gitDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-bar.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-foo.phar
Archive Compression: None
Expand Down Expand Up @@ -525,7 +502,7 @@ public static function gitDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-baz.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-bar.phar
Archive Compression: None
Expand Down Expand Up @@ -569,7 +546,7 @@ public static function GNUDiffPharsProvider(): iterable
self::FIXTURES_DIR.'/simple-phar-bar.phar',
<<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-foo.phar
Archive Compression: None
Expand All @@ -593,7 +570,7 @@ public static function GNUDiffPharsProvider(): iterable
Only in simple-phar-foo.phar: foo.php

OUTPUT,
2,
ExitCode::FAILURE,
];

yield 'same files different content' => [
Expand All @@ -602,7 +579,7 @@ public static function GNUDiffPharsProvider(): iterable
Platform::isOSX()
? <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
Archive: simple-phar-bar.phar
Archive Compression: None
Expand Down Expand Up @@ -631,7 +608,7 @@ public static function GNUDiffPharsProvider(): iterable
OUTPUT
: <<<'OUTPUT'
// Comparing the two archives... (do not check the signatures)
// Comparing the two archives...
[OK] The two archives are identical
Expand All @@ -644,7 +621,7 @@ public static function GNUDiffPharsProvider(): iterable
> echo 'Hello world!';

OUTPUT,
2,
ExitCode::FAILURE,
];
}
}

0 comments on commit d733d96

Please sign in to comment.