From 2b3a5c9110a51f58bdbe052841767e684a00bd10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9o=20FIDRY?= Date: Wed, 11 Oct 2023 15:41:30 +0200 Subject: [PATCH] feat: Simplify the diff command comparison 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. --- src/Console/Command/Diff.php | 48 ++++++++++---------- tests/Console/Command/DiffTest.php | 73 ++++++++++-------------------- 2 files changed, 48 insertions(+), 73 deletions(-) diff --git a/src/Console/Command/Diff.php b/src/Console/Command/Diff.php index 0c4a0d031..6bec5a0bf 100644 --- a/src/Console/Command/Diff.php +++ b/src/Console/Command/Diff.php @@ -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('Comparing the two archives...'); + + $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 + * @return array{non-empty-string, non-empty-string} */ private static function getPaths(IO $io): array { @@ -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('Comparing the two archives... (do not check the signatures)'); - $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, @@ -168,8 +170,6 @@ private function compareArchives(PharDiff $diff, IO $io): int $pharInfoB, $io, ); - - return ExitCode::FAILURE; } private static function getDiffMode(IO $io): DiffMode @@ -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('Comparing the two archives contents...'); $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( @@ -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; } /** diff --git a/tests/Console/Command/DiffTest.php b/tests/Console/Command/DiffTest.php index 167bca291..e79d37ac2 100644 --- a/tests/Console/Command/DiffTest.php +++ b/tests/Console/Command/DiffTest.php @@ -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... - ⚠️ Using the option "list-diff" is deprecated. Use "--diff=list" instead. - [OK] The contents are identical + // Comparing the two archives... + + [OK] The two archives are identical. OUTPUT; @@ -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... - ⚠️ Using the option "list-diff" is deprecated. Use "--diff=list" instead. - [OK] The contents are identical + // Comparing the two archives... + + [OK] The two archives are identical. OUTPUT; @@ -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... - ⚠️ Using the option "gnu-diff" is deprecated. Use "--diff=gnu" instead. - [OK] The contents are identical + // Comparing the two archives... + + [OK] The two archives are identical. OUTPUT; @@ -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; @@ -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, @@ -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 @@ -402,7 +379,7 @@ private static function commonDiffPharsProvider(): iterable OUTPUT, - 1, + ExitCode::FAILURE, ]; } @@ -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 @@ -446,7 +423,7 @@ private static function listDiffPharsProvider(): iterable OUTPUT, - 2, + ExitCode::FAILURE, ]; yield 'same files different content' => [ @@ -454,7 +431,7 @@ private static function listDiffPharsProvider(): 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 @@ -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 @@ -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 @@ -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 @@ -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' => [ @@ -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 @@ -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 @@ -644,7 +621,7 @@ public static function GNUDiffPharsProvider(): iterable > echo 'Hello world!'; OUTPUT, - 2, + ExitCode::FAILURE, ]; } }