From 7f69845211b9188675d82f4048c51c025a9dd83e Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 8 Jun 2022 07:52:09 +0200 Subject: [PATCH 1/5] GitDiffFile: consume configured directories --- src/Configuration/ConfigurationFactory.php | 11 +++++++---- src/Logger/GitHub/GitDiffFileProvider.php | 12 +++++++++--- .../Logger/GitHub/GitDiffFileProviderTest.php | 12 ++++++------ 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/src/Configuration/ConfigurationFactory.php b/src/Configuration/ConfigurationFactory.php index 71cdf62c8..57e59058d 100644 --- a/src/Configuration/ConfigurationFactory.php +++ b/src/Configuration/ConfigurationFactory.php @@ -151,7 +151,7 @@ public function create( $schema->getSource()->getDirectories(), $schema->getSource()->getExcludes() ), - $this->retrieveFilter($filter, $gitDiffFilter, $isForGitDiffLines, $gitDiffBase), + $this->retrieveFilter($filter, $gitDiffFilter, $isForGitDiffLines, $gitDiffBase, $schema->getSource()->getDirectories()), $schema->getSource()->getExcludes(), $this->retrieveLogs($schema->getLogs(), $useGitHubLogger, $htmlLogFilePath), $logVerbosity, @@ -303,7 +303,10 @@ private function retrieveIgnoreSourceCodeMutatorsMap(array $resolvedMutatorsMap) return $map; } - private function retrieveFilter(string $filter, ?string $gitDiffFilter, bool $isForGitDiffLines, ?string $gitDiffBase): string + /** + * @param string[] $sourceDirectories + */ + private function retrieveFilter(string $filter, ?string $gitDiffFilter, bool $isForGitDiffLines, ?string $gitDiffBase, array $sourceDirectories): string { if ($gitDiffFilter === null && !$isForGitDiffLines) { return $filter; @@ -312,10 +315,10 @@ private function retrieveFilter(string $filter, ?string $gitDiffFilter, bool $is $baseBranch = $gitDiffBase ?? GitDiffFileProvider::DEFAULT_BASE; if ($isForGitDiffLines) { - return $this->gitDiffFileProvider->provide('AM', $baseBranch); + return $this->gitDiffFileProvider->provide('AM', $baseBranch, $sourceDirectories); } - return $this->gitDiffFileProvider->provide($gitDiffFilter, $baseBranch); + return $this->gitDiffFileProvider->provide($gitDiffFilter, $baseBranch, $sourceDirectories); } private function retrieveLogs(Logs $logs, ?bool $useGitHubLogger, ?string $htmlLogFilePath): Logs diff --git a/src/Logger/GitHub/GitDiffFileProvider.php b/src/Logger/GitHub/GitDiffFileProvider.php index 608e0d961..a9abcb869 100644 --- a/src/Logger/GitHub/GitDiffFileProvider.php +++ b/src/Logger/GitHub/GitDiffFileProvider.php @@ -54,14 +54,20 @@ public function __construct(private ShellCommandLineExecutor $shellCommandLineEx { } - public function provide(string $gitDiffFilter, string $gitDiffBase): string + /** + * @param string[] $sourceDirectories + */ + public function provide(string $gitDiffFilter, string $gitDiffBase, array $sourceDirectories): string { $referenceCommit = $this->findReferenceCommit($gitDiffBase); $filter = $this->shellCommandLineExecutor->execute(sprintf( - 'git diff %s --diff-filter=%s --name-only | grep src/ | paste -s -d "," -', + 'git diff %s --diff-filter=%s --name-only | grep %s | paste -s -d "," -', escapeshellarg($referenceCommit), - escapeshellarg($gitDiffFilter) + escapeshellarg($gitDiffFilter), + implode(' ', array_map(static function (string $directory): string { + return '-e '.escapeshellarg($directory); + }, $sourceDirectories)) )); if ($filter === '') { diff --git a/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php b/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php index e993e3ee7..3eb550310 100644 --- a/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php +++ b/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php @@ -53,17 +53,17 @@ public function test_it_throws_no_code_to_mutate_exception_when_diff_is_empty(): $this->expectException(NoFilesInDiffToMutate::class); $diffProvider = new GitDiffFileProvider($shellCommandLineExecutor); - $diffProvider->provide('AM', 'master'); + $diffProvider->provide('AM', 'master', ['src/']); } public function test_it_executes_diff_and_returns_filter_as_a_string(): void { $expectedMergeBaseCommandLine = 'git merge-base \'master\' HEAD'; - $expectedDiffCommandLine = 'git diff \'0ABCMERGE_BASE_342\' --diff-filter=\'AM\' --name-only | grep src/ | paste -s -d "," -'; + $expectedDiffCommandLine = 'git diff \'0ABCMERGE_BASE_342\' --diff-filter=\'AM\' --name-only | grep -e \'app/\' -e \'my lib/\' | paste -s -d "," -'; if (PHP_OS_FAMILY === 'Windows') { $expectedMergeBaseCommandLine = 'git merge-base "master" HEAD'; - $expectedDiffCommandLine = 'git diff "0ABCMERGE_BASE_342" --diff-filter="AM" --name-only | grep src/ | paste -s -d "," -'; + $expectedDiffCommandLine = 'git diff "0ABCMERGE_BASE_342" --diff-filter="AM" --name-only | grep -e "app/" -e "my lib/" | paste -s -d "," -'; } $shellCommandLineExecutor = $this->createMock(ShellCommandLineExecutor::class); @@ -75,16 +75,16 @@ public function test_it_executes_diff_and_returns_filter_as_a_string(): void case $expectedMergeBaseCommandLine: return "0ABCMERGE_BASE_342\n"; case $expectedDiffCommandLine: - return 'src/A.php,src/B.php'; + return 'app/A.php,my lib/B.php'; default: $this->fail("Unexpected shell command: $command"); } }); $diffProvider = new GitDiffFileProvider($shellCommandLineExecutor); - $filter = $diffProvider->provide('AM', 'master'); + $filter = $diffProvider->provide('AM', 'master', ['app/', 'my lib/']); - $this->assertSame('src/A.php,src/B.php', $filter); + $this->assertSame('app/A.php,my lib/B.php', $filter); } public function test_it_provides_lines_filter_as_a_string(): void From 325fd1f0d415a50e8f691ade47d5647b1324e30d Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 8 Jun 2022 07:59:39 +0200 Subject: [PATCH 2/5] CS Fix --- src/Logger/GitHub/GitDiffFileProvider.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Logger/GitHub/GitDiffFileProvider.php b/src/Logger/GitHub/GitDiffFileProvider.php index a9abcb869..feb467693 100644 --- a/src/Logger/GitHub/GitDiffFileProvider.php +++ b/src/Logger/GitHub/GitDiffFileProvider.php @@ -66,7 +66,7 @@ public function provide(string $gitDiffFilter, string $gitDiffBase, array $sourc escapeshellarg($referenceCommit), escapeshellarg($gitDiffFilter), implode(' ', array_map(static function (string $directory): string { - return '-e '.escapeshellarg($directory); + return '-e ' . escapeshellarg($directory); }, $sourceDirectories)) )); From 183b6413c2d77e321213969b5af230d64e11e620 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Wed, 8 Jun 2022 08:01:08 +0200 Subject: [PATCH 3/5] CS Fix 2 --- src/Logger/GitHub/GitDiffFileProvider.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Logger/GitHub/GitDiffFileProvider.php b/src/Logger/GitHub/GitDiffFileProvider.php index feb467693..5f9041095 100644 --- a/src/Logger/GitHub/GitDiffFileProvider.php +++ b/src/Logger/GitHub/GitDiffFileProvider.php @@ -35,7 +35,9 @@ namespace Infection\Logger\GitHub; +use function array_map; use function escapeshellarg; +use function implode; use Infection\Process\ShellCommandLineExecutor; use function Safe\sprintf; use Symfony\Component\Process\Exception\ProcessFailedException; From e3b4566b2937174974f98ce80db250240425b6e3 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Thu, 9 Jun 2022 08:31:08 +0200 Subject: [PATCH 4/5] Prefer native git listing over grep --- src/Logger/GitHub/GitDiffFileProvider.php | 6 ++---- tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/Logger/GitHub/GitDiffFileProvider.php b/src/Logger/GitHub/GitDiffFileProvider.php index 5f9041095..ab8a92aac 100644 --- a/src/Logger/GitHub/GitDiffFileProvider.php +++ b/src/Logger/GitHub/GitDiffFileProvider.php @@ -64,12 +64,10 @@ public function provide(string $gitDiffFilter, string $gitDiffBase, array $sourc $referenceCommit = $this->findReferenceCommit($gitDiffBase); $filter = $this->shellCommandLineExecutor->execute(sprintf( - 'git diff %s --diff-filter=%s --name-only | grep %s | paste -s -d "," -', + 'git diff %s --diff-filter=%s --name-only -- %s | paste -s -d "," -', escapeshellarg($referenceCommit), escapeshellarg($gitDiffFilter), - implode(' ', array_map(static function (string $directory): string { - return '-e ' . escapeshellarg($directory); - }, $sourceDirectories)) + implode(' ', array_map('\\escapeshellarg', $sourceDirectories)) )); if ($filter === '') { diff --git a/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php b/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php index 3eb550310..3e7890fe4 100644 --- a/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php +++ b/tests/phpunit/Logger/GitHub/GitDiffFileProviderTest.php @@ -59,11 +59,11 @@ public function test_it_throws_no_code_to_mutate_exception_when_diff_is_empty(): public function test_it_executes_diff_and_returns_filter_as_a_string(): void { $expectedMergeBaseCommandLine = 'git merge-base \'master\' HEAD'; - $expectedDiffCommandLine = 'git diff \'0ABCMERGE_BASE_342\' --diff-filter=\'AM\' --name-only | grep -e \'app/\' -e \'my lib/\' | paste -s -d "," -'; + $expectedDiffCommandLine = 'git diff \'0ABCMERGE_BASE_342\' --diff-filter=\'AM\' --name-only -- \'app/\' \'my lib/\' | paste -s -d "," -'; if (PHP_OS_FAMILY === 'Windows') { $expectedMergeBaseCommandLine = 'git merge-base "master" HEAD'; - $expectedDiffCommandLine = 'git diff "0ABCMERGE_BASE_342" --diff-filter="AM" --name-only | grep -e "app/" -e "my lib/" | paste -s -d "," -'; + $expectedDiffCommandLine = 'git diff "0ABCMERGE_BASE_342" --diff-filter="AM" --name-only -- "app/" "my lib/" | paste -s -d "," -'; } $shellCommandLineExecutor = $this->createMock(ShellCommandLineExecutor::class); From 951f85b5df7064c406afe713835748c08b0081a2 Mon Sep 17 00:00:00 2001 From: Filippo Tessarotto Date: Thu, 9 Jun 2022 10:23:23 +0200 Subject: [PATCH 5/5] Lower overall MIN_MSI as requested --- .github/workflows/mt.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/mt.yaml b/.github/workflows/mt.yaml index a3c1726ff..a5d988456 100644 --- a/.github/workflows/mt.yaml +++ b/.github/workflows/mt.yaml @@ -11,7 +11,7 @@ on: - master env: - MIN_MSI: 71.39 + MIN_MSI: 71.35 MIN_COVERED_MSI: 86.78 jobs: