From a0d0b99affa690057ff86cc843cd63187349a48d Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 14 Jul 2021 16:59:01 +0200 Subject: [PATCH 1/4] New `logs.badge.matchBranchRegex` config to match multiple branches in reporting By configuring `logs.badge.matchBranchRegex`, it is possible to set multiple branches to report their mutation score to upstream tools: ```json { "source": { "directories": ["src/"] }, "logs": { "badge": { "matchBranchRegex": "/^release-.*$/" } } } ``` Note that `logs.badge.matchBranchRegex` is mutually exclusive with `logs.badge.branch`, which is much more restrictive: the two cannot be used together. Fixes #1536 --- resources/schema.json | 43 ++++++++-- src/Configuration/Entry/Badge.php | 35 ++++++-- .../Schema/SchemaConfigurationFactory.php | 7 +- src/Logger/BadgeLogger.php | 30 +++++-- src/Logger/BadgeLoggerFactory.php | 7 +- .../generate-mutations-closure.php | 3 +- .../Tracing/provide-traces-closure.php | 3 +- .../ConfigurationFactoryTest.php | 4 +- .../Configuration/ConfigurationTest.php | 2 +- .../Configuration/Entry/BadgeAssertions.php | 48 ----------- .../phpunit/Configuration/Entry/BadgeTest.php | 29 ++++++- .../Configuration/Entry/LogsAssertions.php | 5 +- .../phpunit/Configuration/Entry/LogsTest.php | 2 +- .../Schema/SchemaConfigurationFactoryTest.php | 60 ++++++++++++- .../Schema/SchemaConfigurationTest.php | 2 +- .../phpunit/Logger/BadgeLoggerFactoryTest.php | 6 +- tests/phpunit/Logger/BadgeLoggerTest.php | 85 +++++++++++++++++++ .../phpunit/Logger/FileLoggerFactoryTest.php | 2 +- 18 files changed, 278 insertions(+), 95 deletions(-) delete mode 100644 tests/phpunit/Configuration/Entry/BadgeAssertions.php diff --git a/resources/schema.json b/resources/schema.json index 85e65a5ba..ab6119791 100644 --- a/resources/schema.json +++ b/resources/schema.json @@ -64,15 +64,42 @@ "description": "Markdown file which will give a break-down of the effectiveness of each mutator." }, "badge": { - "type": "object", - "additionalProperties": false, - "required": ["branch"], - "properties": { - "branch": { - "type": "string", - "description": "Mutation score badge for your GitHub project." + "description": "Mutation score badge for your GitHub project.", + "oneOf": [ + { + "type": "object", + "additionalProperties": false, + "required": ["branch"], + "properties": { + "branch": { + "type": "string", + "description": "Mutation score will be uploaded to upstream tracker only if your current branch matches this value.", + "examples": [ + "main", + "master", + "develop", + "latest" + ] + } + } + }, + { + "type": "object", + "additionalProperties": false, + "required": ["matchBranchRegex"], + "properties": { + "matchBranchRegex": { + "type": "string", + "description": "Mutation score will be uploaded to upstream tracker only if your current branch matches this regular expression.", + "examples": [ + "/1\\.\\d+/", + "/release-.*/", + "/feature\\/.*/" + ] + } + } } - } + ] }, "github": { "type": "boolean", diff --git a/src/Configuration/Entry/Badge.php b/src/Configuration/Entry/Badge.php index 118d8377a..1168a91f6 100644 --- a/src/Configuration/Entry/Badge.php +++ b/src/Configuration/Entry/Badge.php @@ -35,20 +35,45 @@ namespace Infection\Configuration\Entry; +use InvalidArgumentException; +use Safe\Exceptions\PcreException; +use function Safe\preg_match; +use function Safe\sprintf; + /** * @internal */ final class Badge { - private string $branch; + private ?string $exactBranchMatch; + private ?string $matchBranchRegex; + + public function __construct(?string $exactBranchMatch, ?string $matchBranchRegex) + { + if ($matchBranchRegex !== null) { + try { + // Yes, the `@` is intentional. For some reason, `thecodingmachine/safe` does not suppress the warnings here + @preg_match($matchBranchRegex, ''); + } catch (PcreException $invalidRegex) { + throw new InvalidArgumentException( + sprintf('Provided branchMatchRegex "%s" is not a valid regex', $matchBranchRegex), + 0, + $invalidRegex + ); + } + } + + $this->exactBranchMatch = $exactBranchMatch; + $this->matchBranchRegex = $matchBranchRegex; + } - public function __construct(string $branch) + public function getExactBranchMatch(): ?string { - $this->branch = $branch; + return $this->exactBranchMatch; } - public function getBranch(): string + public function getBranchMatchRegEx(): ?string { - return $this->branch; + return $this->matchBranchRegex; } } diff --git a/src/Configuration/Schema/SchemaConfigurationFactory.php b/src/Configuration/Schema/SchemaConfigurationFactory.php index 4e0d7565d..914653851 100644 --- a/src/Configuration/Schema/SchemaConfigurationFactory.php +++ b/src/Configuration/Schema/SchemaConfigurationFactory.php @@ -93,11 +93,12 @@ private static function createLogs(stdClass $logs): Logs private static function createBadge(?stdClass $badge): ?Badge { - $branch = self::normalizeString($badge->branch ?? null); + $exactBranchMatch = self::normalizeString($badge->branch ?? null); + $matchBranchRegex = self::normalizeString($badge->matchBranchRegex ?? null); - return $branch === null + return $exactBranchMatch === null && $matchBranchRegex === null ? null - : new Badge($branch) + : new Badge($exactBranchMatch, $matchBranchRegex) ; } diff --git a/src/Logger/BadgeLogger.php b/src/Logger/BadgeLogger.php index ee6130618..7176682e3 100644 --- a/src/Logger/BadgeLogger.php +++ b/src/Logger/BadgeLogger.php @@ -43,6 +43,7 @@ use Infection\Logger\Http\StrykerDashboardClient; use Infection\Metrics\MetricsCalculator; use Psr\Log\LoggerInterface; +use function Safe\preg_match; use function Safe\sprintf; /** @@ -54,7 +55,8 @@ final class BadgeLogger implements MutationTestingResultsLogger private StrykerApiKeyResolver $strykerApiKeyResolver; private StrykerDashboardClient $strykerDashboardClient; private MetricsCalculator $metricsCalculator; - private string $branch; + private ?string $exactBranchMatch; + private ?string $matchBranchRegex; private LoggerInterface $logger; public function __construct( @@ -62,14 +64,16 @@ public function __construct( StrykerApiKeyResolver $strykerApiKeyResolver, StrykerDashboardClient $strykerDashboardClient, MetricsCalculator $metricsCalculator, - string $branch, + ?string $exactBranchMatch, + ?string $matchBranchRegex, LoggerInterface $logger ) { $this->buildContextResolver = $buildContextResolver; $this->strykerApiKeyResolver = $strykerApiKeyResolver; $this->strykerDashboardClient = $strykerDashboardClient; $this->metricsCalculator = $metricsCalculator; - $this->branch = $branch; + $this->exactBranchMatch = $exactBranchMatch; + $this->matchBranchRegex = $matchBranchRegex; $this->logger = $logger; } @@ -83,11 +87,23 @@ public function log(): void return; } - if ($buildContext->branch() !== $this->branch) { + $branch = $buildContext->branch(); + + if ($this->exactBranchMatch !== null && $branch !== $this->exactBranchMatch) { $this->logReportWasNotSent(sprintf( 'Expected branch "%s", found "%s"', - $this->branch, - $buildContext->branch() + $this->exactBranchMatch, + $branch + )); + + return; + } + + if ($this->matchBranchRegex !== null && preg_match($this->matchBranchRegex, $branch) !== 1) { + $this->logReportWasNotSent(sprintf( + 'Expected branch to match regex "%s", found "%s"', + $this->matchBranchRegex, + $branch )); return; @@ -106,7 +122,7 @@ public function log(): void $this->strykerDashboardClient->sendReport( 'github.com/' . $buildContext->repositorySlug(), - $buildContext->branch(), + $branch, $apiKey, $this->metricsCalculator->getMutationScoreIndicator() ); diff --git a/src/Logger/BadgeLoggerFactory.php b/src/Logger/BadgeLoggerFactory.php index fc138be0a..e0bd3b24b 100644 --- a/src/Logger/BadgeLoggerFactory.php +++ b/src/Logger/BadgeLoggerFactory.php @@ -66,7 +66,9 @@ public function __construct( public function createFromLogEntries(Logs $logConfig): ?MutationTestingResultsLogger { - if ($logConfig->getBadge() === null) { + $badge = $logConfig->getBadge(); + + if ($badge === null) { return null; } @@ -78,7 +80,8 @@ public function createFromLogEntries(Logs $logConfig): ?MutationTestingResultsLo $this->logger ), $this->metricsCalculator, - $logConfig->getBadge()->getBranch(), + $badge->getExactBranchMatch(), + $badge->getBranchMatchRegEx(), $this->logger ); } diff --git a/tests/benchmark/MutationGenerator/generate-mutations-closure.php b/tests/benchmark/MutationGenerator/generate-mutations-closure.php index 0f13ee00d..e5803b774 100644 --- a/tests/benchmark/MutationGenerator/generate-mutations-closure.php +++ b/tests/benchmark/MutationGenerator/generate-mutations-closure.php @@ -65,7 +65,8 @@ static function (SplFileInfo $fileInfo): Trace { ); $mutators = $container->getMutatorFactory()->create( - $container->getMutatorResolver()->resolve(['@default' => true]) + $container->getMutatorResolver()->resolve(['@default' => true]), + true ); $fileMutationGenerator = $container->getFileMutationGenerator(); diff --git a/tests/benchmark/Tracing/provide-traces-closure.php b/tests/benchmark/Tracing/provide-traces-closure.php index cfe4b7b0c..b6d7ed9de 100644 --- a/tests/benchmark/Tracing/provide-traces-closure.php +++ b/tests/benchmark/Tracing/provide-traces-closure.php @@ -69,7 +69,8 @@ Container::DEFAULT_DRY_RUN, Container::DEFAULT_GIT_DIFF_FILTER, Container::DEFAULT_GIT_DIFF_BASE, - Container::DEFAULT_USE_GITHUB_LOGGER + Container::DEFAULT_USE_GITHUB_LOGGER, + true ); $generateTraces = static function (?int $maxCount) use ($container): iterable { diff --git a/tests/phpunit/Configuration/ConfigurationFactoryTest.php b/tests/phpunit/Configuration/ConfigurationFactoryTest.php index 4b27ea5d2..b2228204f 100644 --- a/tests/phpunit/Configuration/ConfigurationFactoryTest.php +++ b/tests/phpunit/Configuration/ConfigurationFactoryTest.php @@ -732,7 +732,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master') + new Badge('master', null) ), 'config/tmp', new PhpUnit( @@ -785,7 +785,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master') + new Badge('master', null) ), 'none', '/path/to/config/tmp/infection', diff --git a/tests/phpunit/Configuration/ConfigurationTest.php b/tests/phpunit/Configuration/ConfigurationTest.php index 93746ea10..4f9b53482 100644 --- a/tests/phpunit/Configuration/ConfigurationTest.php +++ b/tests/phpunit/Configuration/ConfigurationTest.php @@ -202,7 +202,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master') + new Badge('master', null) ), 'default', 'custom-dir', diff --git a/tests/phpunit/Configuration/Entry/BadgeAssertions.php b/tests/phpunit/Configuration/Entry/BadgeAssertions.php deleted file mode 100644 index 68c49dae7..000000000 --- a/tests/phpunit/Configuration/Entry/BadgeAssertions.php +++ /dev/null @@ -1,48 +0,0 @@ -assertSame($expectedBranch, $badge->getBranch()); - } -} diff --git a/tests/phpunit/Configuration/Entry/BadgeTest.php b/tests/phpunit/Configuration/Entry/BadgeTest.php index 884915a14..19e206392 100644 --- a/tests/phpunit/Configuration/Entry/BadgeTest.php +++ b/tests/phpunit/Configuration/Entry/BadgeTest.php @@ -36,16 +36,37 @@ namespace Infection\Tests\Configuration\Entry; use Infection\Configuration\Entry\Badge; +use InvalidArgumentException; use PHPUnit\Framework\TestCase; final class BadgeTest extends TestCase { - use BadgeAssertions; + public function test_it_can_be_instantiated_with_an_exact_branch_match(): void + { + $badge = new Badge('master', null); + + $this->assertNull($badge->getBranchMatchRegEx()); + $this->assertSame('master', $badge->getExactBranchMatch()); + } + + public function test_it_can_be_instantiated_with_a_regex_branch_match(): void + { + $badge = new Badge(null, '/^foo$/'); + + $this->assertNull($badge->getExactBranchMatch()); + $this->assertSame('/^foo$/', $badge->getBranchMatchRegEx()); + } - public function test_it_can_be_instantiated(): void + public function test_it_rejects_invalid_regex(): void { - $badge = new Badge('master'); + try { + new Badge(null, '/foo#'); - $this->assertBadgeStateIs($badge, 'master'); + $this->fail(); + } catch (InvalidArgumentException $invalid) { + $this->assertSame('Provided branchMatchRegex "/foo#" is not a valid regex', $invalid->getMessage()); + $this->assertSame(0, $invalid->getCode()); + $this->assertNotNull($invalid->getPrevious()); + } } } diff --git a/tests/phpunit/Configuration/Entry/LogsAssertions.php b/tests/phpunit/Configuration/Entry/LogsAssertions.php index bbd779165..b3c918457 100644 --- a/tests/phpunit/Configuration/Entry/LogsAssertions.php +++ b/tests/phpunit/Configuration/Entry/LogsAssertions.php @@ -40,8 +40,6 @@ trait LogsAssertions { - use BadgeAssertions; - private function assertLogsStateIs( Logs $logs, ?string $expectedTextLogFilePath, @@ -65,7 +63,8 @@ private function assertLogsStateIs( $this->assertNull($badge); } else { $this->assertNotNull($badge); - $this->assertBadgeStateIs($badge, $expectedBadge->getBranch()); + + self::assertEquals($expectedBadge, $badge); } } } diff --git a/tests/phpunit/Configuration/Entry/LogsTest.php b/tests/phpunit/Configuration/Entry/LogsTest.php index cd60b882e..b4de797f7 100644 --- a/tests/phpunit/Configuration/Entry/LogsTest.php +++ b/tests/phpunit/Configuration/Entry/LogsTest.php @@ -112,7 +112,7 @@ public function valuesProvider(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master'), + new Badge('master', null), ]; } } diff --git a/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php b/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php index 1c54c17db..8514bd9b3 100644 --- a/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php +++ b/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php @@ -363,7 +363,35 @@ public function provideRawConfig(): iterable null, null, false, - new Badge('master') + new Badge('master', null) + ), + ]), + ]; + + yield '[logs][badge] regex' => [ + <<<'JSON' +{ + "source": { + "directories": ["src"] + }, + "logs": { + "badge": { + "matchBranchRegex": "/^foo$/" + } + } +} +JSON + , + self::createConfig([ + 'source' => new Source(['src'], []), + 'logs' => new Logs( + null, + null, + null, + null, + null, + false, + new Badge(null, '/^foo$/') ), ]), ]; @@ -397,7 +425,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master') + new Badge('master', null) ), ]), ]; @@ -426,6 +454,30 @@ public function provideRawConfig(): iterable ]), ]; + yield '[logs] empty branch match regex' => [ + <<<'JSON' +{ + "source": { + "directories": ["src"] + }, + "logs": { + "text": "", + "summary": "", + "debug": "", + "perMutator": "", + "badge": { + "matchBranchRegex": "" + } + } +} +JSON + , + self::createConfig([ + 'source' => new Source(['src'], []), + 'logs' => Logs::createEmpty(), + ]), + ]; + yield '[logs] empty & untrimmed strings' => [ <<<'JSON' { @@ -455,7 +507,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master') + new Badge('master', null) ), ]), ]; @@ -2325,7 +2377,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master') + new Badge('master', null) ), 'tmpDir' => 'custom-tmp', 'phpunit' => new PhpUnit( diff --git a/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php b/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php index fb72194ee..fa2a428ea 100644 --- a/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php +++ b/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php @@ -126,7 +126,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master') + new Badge('master', null) ), 'path/to/tmp', new PhpUnit('dist/phpunit', 'bin/phpunit'), diff --git a/tests/phpunit/Logger/BadgeLoggerFactoryTest.php b/tests/phpunit/Logger/BadgeLoggerFactoryTest.php index 9c89aeb35..4560b4136 100644 --- a/tests/phpunit/Logger/BadgeLoggerFactoryTest.php +++ b/tests/phpunit/Logger/BadgeLoggerFactoryTest.php @@ -121,7 +121,7 @@ public function test_it_creates_a_badge_logger_on_no_verbosity(): void null, null, false, - new Badge('master') + new Badge('master', null) ) ); @@ -167,7 +167,7 @@ public function logsProvider(): iterable null, null, false, - new Badge('foo') + new Badge('foo', null) ), BadgeLogger::class, ]; @@ -180,7 +180,7 @@ public function logsProvider(): iterable 'debug', 'per_mutator', true, - new Badge('branch') + new Badge('branch', null) ), BadgeLogger::class, ]; diff --git a/tests/phpunit/Logger/BadgeLoggerTest.php b/tests/phpunit/Logger/BadgeLoggerTest.php index a2c9abc07..ca8d8144b 100644 --- a/tests/phpunit/Logger/BadgeLoggerTest.php +++ b/tests/phpunit/Logger/BadgeLoggerTest.php @@ -92,6 +92,7 @@ protected function setUp(): void $this->badgeApiClientMock, $this->metricsCalculatorMock, 'master', + null, $this->logger ); } @@ -235,6 +236,44 @@ public function test_it_skips_logging_when_it_is_branch_not_from_config(): void ); } + public function test_it_skips_logging_when_it_is_branch_not_from_config_regex(): void + { + $this->ciDetectorEnv->setVariables([ + 'TRAVIS' => 'true', + 'TRAVIS_PULL_REQUEST' => 'false', + 'TRAVIS_REPO_SLUG' => 'a/b', + 'TRAVIS_BRANCH' => '1.x-mismatch', + ]); + + $this->badgeApiClientMock + ->expects($this->never()) + ->method('sendReport') + ; + + $badgeLogger = new BadgeLogger( + new BuildContextResolver(CiDetector::fromEnvironment($this->ciDetectorEnv)), + new StrykerApiKeyResolver(), + $this->badgeApiClientMock, + $this->metricsCalculatorMock, + null, + '/^\d+\\.x$/', + $this->logger + ); + + $badgeLogger->log(); + + $this->assertSame( + [ + [ + LogLevel::WARNING, + 'Dashboard report has not been sent: Expected branch to match regex "/^\d+\\.x$/", found "1.x-mismatch"', + [], + ], + ], + $this->logger->getLogs() + ); + } + public function test_it_sends_report_missing_our_api_key(): void { $this->ciDetectorEnv->setVariables([ @@ -302,6 +341,52 @@ public function test_it_sends_report_when_everything_is_ok_with_stryker_key(): v ); } + public function test_it_sends_report_when_everything_is_ok_with_stryker_key_and_matching_branch_regex(): void + { + $this->ciDetectorEnv->setVariables([ + 'TRAVIS' => 'true', + 'TRAVIS_PULL_REQUEST' => 'false', + 'TRAVIS_REPO_SLUG' => 'a/b', + 'TRAVIS_BRANCH' => '7.x', + ]); + + putenv('STRYKER_DASHBOARD_API_KEY=abc'); + + $this->badgeApiClientMock + ->expects($this->once()) + ->method('sendReport') + ->with('github.com/a/b', '7.x', 'abc', 33.3) + ; + + $this->metricsCalculatorMock + ->method('getMutationScoreIndicator') + ->willReturn(33.3) + ; + + $badgeLogger = new BadgeLogger( + new BuildContextResolver(CiDetector::fromEnvironment($this->ciDetectorEnv)), + new StrykerApiKeyResolver(), + $this->badgeApiClientMock, + $this->metricsCalculatorMock, + null, + '/^\d+\\.x$/', + $this->logger + ); + + $badgeLogger->log(); + + $this->assertSame( + [ + [ + LogLevel::WARNING, + 'Sending dashboard report...', + [], + ], + ], + $this->logger->getLogs() + ); + } + public function test_it_sends_report_when_everything_is_ok_with_our_key(): void { $this->ciDetectorEnv->setVariables([ diff --git a/tests/phpunit/Logger/FileLoggerFactoryTest.php b/tests/phpunit/Logger/FileLoggerFactoryTest.php index 28c08029c..a76879068 100644 --- a/tests/phpunit/Logger/FileLoggerFactoryTest.php +++ b/tests/phpunit/Logger/FileLoggerFactoryTest.php @@ -220,7 +220,7 @@ public function logsProvider(): iterable 'debug', 'per_mutator', true, - new Badge('branch') + new Badge('branch', null) ), [ TextFileLogger::class, From 3c1e4dae82f3b018351bc205c60736febc3954c8 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Tue, 20 Jul 2021 12:42:22 +0200 Subject: [PATCH 2/4] Made `logs.badge.branch` capable of accepting a regular expression By configuring `logs.badge.branch` with a `/`-delimited regular expression, we can collect reports about a number of branches. ```json { "source": { "directories": ["src/"] }, "logs": { "badge": { "branch": "/^release-.*$/" } } } ``` Note that `logs.badge.matchBranchRegex`, initially proposed as an alternative, has also been removed. We can safely assume that a `branch` value starting and ending with `/` is a regular expression, because git branches starting or ending with `/` are not valid anyway. Fixes #1536 --- resources/schema.json | 53 +++++++------------ src/Configuration/Entry/Badge.php | 42 +++++++-------- .../Schema/SchemaConfigurationFactory.php | 7 ++- src/Logger/BadgeLogger.php | 26 +++------ src/Logger/BadgeLoggerFactory.php | 3 +- .../ConfigurationFactoryTest.php | 4 +- .../Configuration/ConfigurationTest.php | 2 +- .../phpunit/Configuration/Entry/BadgeTest.php | 24 ++++++--- .../phpunit/Configuration/Entry/LogsTest.php | 2 +- .../Schema/SchemaConfigurationFactoryTest.php | 14 ++--- .../Schema/SchemaConfigurationTest.php | 2 +- .../phpunit/Logger/BadgeLoggerFactoryTest.php | 6 +-- tests/phpunit/Logger/BadgeLoggerTest.php | 14 +++-- .../phpunit/Logger/FileLoggerFactoryTest.php | 2 +- 14 files changed, 86 insertions(+), 115 deletions(-) diff --git a/resources/schema.json b/resources/schema.json index ab6119791..1082128a9 100644 --- a/resources/schema.json +++ b/resources/schema.json @@ -64,42 +64,25 @@ "description": "Markdown file which will give a break-down of the effectiveness of each mutator." }, "badge": { - "description": "Mutation score badge for your GitHub project.", - "oneOf": [ - { - "type": "object", - "additionalProperties": false, - "required": ["branch"], - "properties": { - "branch": { - "type": "string", - "description": "Mutation score will be uploaded to upstream tracker only if your current branch matches this value.", - "examples": [ - "main", - "master", - "develop", - "latest" - ] - } - } - }, - { - "type": "object", - "additionalProperties": false, - "required": ["matchBranchRegex"], - "properties": { - "matchBranchRegex": { - "type": "string", - "description": "Mutation score will be uploaded to upstream tracker only if your current branch matches this regular expression.", - "examples": [ - "/1\\.\\d+/", - "/release-.*/", - "/feature\\/.*/" - ] - } - } + "type": "object", + "description": "Mutation score badge for your GitHub project. If provided, infection will report results for maching branches to an upstream reporting dashboard/collector.", + "additionalProperties": false, + "required": ["branch"], + "properties": { + "branch": { + "type": "string", + "description": "Mutation score badge for your GitHub project. If this value starts and ends with \"/\", it will be considered a regular expression.", + "exaples": [ + "main", + "master", + "develop", + "latest", + "/1\\.\\d+/", + "/release-.*/", + "/feature\\/.*/" + ] } - ] + } }, "github": { "type": "boolean", diff --git a/src/Configuration/Entry/Badge.php b/src/Configuration/Entry/Badge.php index 1168a91f6..2473c2953 100644 --- a/src/Configuration/Entry/Badge.php +++ b/src/Configuration/Entry/Badge.php @@ -36,6 +36,7 @@ namespace Infection\Configuration\Entry; use InvalidArgumentException; +use function preg_quote; use Safe\Exceptions\PcreException; use function Safe\preg_match; use function Safe\sprintf; @@ -45,35 +46,32 @@ */ final class Badge { - private ?string $exactBranchMatch; - private ?string $matchBranchRegex; + private string $branchMatch; - public function __construct(?string $exactBranchMatch, ?string $matchBranchRegex) + public function __construct(string $branch) { - if ($matchBranchRegex !== null) { - try { - // Yes, the `@` is intentional. For some reason, `thecodingmachine/safe` does not suppress the warnings here - @preg_match($matchBranchRegex, ''); - } catch (PcreException $invalidRegex) { - throw new InvalidArgumentException( - sprintf('Provided branchMatchRegex "%s" is not a valid regex', $matchBranchRegex), - 0, - $invalidRegex - ); - } + if (preg_match('#^/.+/$#', $branch) === 0) { + $this->branchMatch = '/^' . preg_quote($branch, '/') . '$/'; + + return; } - $this->exactBranchMatch = $exactBranchMatch; - $this->matchBranchRegex = $matchBranchRegex; - } + try { + // Yes, the `@` is intentional. For some reason, `thecodingmachine/safe` does not suppress the warnings here + @preg_match($branch, ''); + } catch (PcreException $invalidRegex) { + throw new InvalidArgumentException( + sprintf('Provided branchMatchRegex "%s" is not a valid regex', $branch), + 0, + $invalidRegex + ); + } - public function getExactBranchMatch(): ?string - { - return $this->exactBranchMatch; + $this->branchMatch = $branch; } - public function getBranchMatchRegEx(): ?string + public function applicableForBranch(string $branchName): bool { - return $this->matchBranchRegex; + return preg_match($this->branchMatch, $branchName) === 1; } } diff --git a/src/Configuration/Schema/SchemaConfigurationFactory.php b/src/Configuration/Schema/SchemaConfigurationFactory.php index 914653851..4e0d7565d 100644 --- a/src/Configuration/Schema/SchemaConfigurationFactory.php +++ b/src/Configuration/Schema/SchemaConfigurationFactory.php @@ -93,12 +93,11 @@ private static function createLogs(stdClass $logs): Logs private static function createBadge(?stdClass $badge): ?Badge { - $exactBranchMatch = self::normalizeString($badge->branch ?? null); - $matchBranchRegex = self::normalizeString($badge->matchBranchRegex ?? null); + $branch = self::normalizeString($badge->branch ?? null); - return $exactBranchMatch === null && $matchBranchRegex === null + return $branch === null ? null - : new Badge($exactBranchMatch, $matchBranchRegex) + : new Badge($branch) ; } diff --git a/src/Logger/BadgeLogger.php b/src/Logger/BadgeLogger.php index 7176682e3..cff4a95d2 100644 --- a/src/Logger/BadgeLogger.php +++ b/src/Logger/BadgeLogger.php @@ -36,6 +36,7 @@ namespace Infection\Logger; use function getenv; +use Infection\Configuration\Entry\Badge; use Infection\Environment\BuildContextResolver; use Infection\Environment\CouldNotResolveBuildContext; use Infection\Environment\CouldNotResolveStrykerApiKey; @@ -43,7 +44,6 @@ use Infection\Logger\Http\StrykerDashboardClient; use Infection\Metrics\MetricsCalculator; use Psr\Log\LoggerInterface; -use function Safe\preg_match; use function Safe\sprintf; /** @@ -55,8 +55,7 @@ final class BadgeLogger implements MutationTestingResultsLogger private StrykerApiKeyResolver $strykerApiKeyResolver; private StrykerDashboardClient $strykerDashboardClient; private MetricsCalculator $metricsCalculator; - private ?string $exactBranchMatch; - private ?string $matchBranchRegex; + private Badge $badge; private LoggerInterface $logger; public function __construct( @@ -64,16 +63,14 @@ public function __construct( StrykerApiKeyResolver $strykerApiKeyResolver, StrykerDashboardClient $strykerDashboardClient, MetricsCalculator $metricsCalculator, - ?string $exactBranchMatch, - ?string $matchBranchRegex, + Badge $badge, LoggerInterface $logger ) { $this->buildContextResolver = $buildContextResolver; $this->strykerApiKeyResolver = $strykerApiKeyResolver; $this->strykerDashboardClient = $strykerDashboardClient; $this->metricsCalculator = $metricsCalculator; - $this->exactBranchMatch = $exactBranchMatch; - $this->matchBranchRegex = $matchBranchRegex; + $this->badge = $badge; $this->logger = $logger; } @@ -89,20 +86,9 @@ public function log(): void $branch = $buildContext->branch(); - if ($this->exactBranchMatch !== null && $branch !== $this->exactBranchMatch) { + if (!$this->badge->applicableForBranch($branch)) { $this->logReportWasNotSent(sprintf( - 'Expected branch "%s", found "%s"', - $this->exactBranchMatch, - $branch - )); - - return; - } - - if ($this->matchBranchRegex !== null && preg_match($this->matchBranchRegex, $branch) !== 1) { - $this->logReportWasNotSent(sprintf( - 'Expected branch to match regex "%s", found "%s"', - $this->matchBranchRegex, + 'Branch "%s" does not match expected badge configuration', $branch )); diff --git a/src/Logger/BadgeLoggerFactory.php b/src/Logger/BadgeLoggerFactory.php index e0bd3b24b..e8c369670 100644 --- a/src/Logger/BadgeLoggerFactory.php +++ b/src/Logger/BadgeLoggerFactory.php @@ -80,8 +80,7 @@ public function createFromLogEntries(Logs $logConfig): ?MutationTestingResultsLo $this->logger ), $this->metricsCalculator, - $badge->getExactBranchMatch(), - $badge->getBranchMatchRegEx(), + $badge, $this->logger ); } diff --git a/tests/phpunit/Configuration/ConfigurationFactoryTest.php b/tests/phpunit/Configuration/ConfigurationFactoryTest.php index b2228204f..4b27ea5d2 100644 --- a/tests/phpunit/Configuration/ConfigurationFactoryTest.php +++ b/tests/phpunit/Configuration/ConfigurationFactoryTest.php @@ -732,7 +732,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master', null) + new Badge('master') ), 'config/tmp', new PhpUnit( @@ -785,7 +785,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master', null) + new Badge('master') ), 'none', '/path/to/config/tmp/infection', diff --git a/tests/phpunit/Configuration/ConfigurationTest.php b/tests/phpunit/Configuration/ConfigurationTest.php index 4f9b53482..93746ea10 100644 --- a/tests/phpunit/Configuration/ConfigurationTest.php +++ b/tests/phpunit/Configuration/ConfigurationTest.php @@ -202,7 +202,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master', null) + new Badge('master') ), 'default', 'custom-dir', diff --git a/tests/phpunit/Configuration/Entry/BadgeTest.php b/tests/phpunit/Configuration/Entry/BadgeTest.php index 19e206392..8d599287b 100644 --- a/tests/phpunit/Configuration/Entry/BadgeTest.php +++ b/tests/phpunit/Configuration/Entry/BadgeTest.php @@ -43,28 +43,36 @@ final class BadgeTest extends TestCase { public function test_it_can_be_instantiated_with_an_exact_branch_match(): void { - $badge = new Badge('master', null); + $badge = new Badge('master'); - $this->assertNull($badge->getBranchMatchRegEx()); - $this->assertSame('master', $badge->getExactBranchMatch()); + $this->assertTrue($badge->applicableForBranch('master')); + $this->assertFalse($badge->applicableForBranch('mast')); + $this->assertFalse($badge->applicableForBranch('master ')); + $this->assertFalse($badge->applicableForBranch(' master')); + $this->assertFalse($badge->applicableForBranch('master1')); } public function test_it_can_be_instantiated_with_a_regex_branch_match(): void { - $badge = new Badge(null, '/^foo$/'); + $badge = new Badge('/^(foo|bar)$/'); - $this->assertNull($badge->getExactBranchMatch()); - $this->assertSame('/^foo$/', $badge->getBranchMatchRegEx()); + $this->assertTrue($badge->applicableForBranch('foo')); + $this->assertTrue($badge->applicableForBranch('bar')); + $this->assertFalse($badge->applicableForBranch('fo')); + $this->assertFalse($badge->applicableForBranch('ba')); + $this->assertFalse($badge->applicableForBranch('foo ')); + $this->assertFalse($badge->applicableForBranch(' foo')); + $this->assertFalse($badge->applicableForBranch('foo1')); } public function test_it_rejects_invalid_regex(): void { try { - new Badge(null, '/foo#'); + new Badge('/[/'); $this->fail(); } catch (InvalidArgumentException $invalid) { - $this->assertSame('Provided branchMatchRegex "/foo#" is not a valid regex', $invalid->getMessage()); + $this->assertSame('Provided branchMatchRegex "/[/" is not a valid regex', $invalid->getMessage()); $this->assertSame(0, $invalid->getCode()); $this->assertNotNull($invalid->getPrevious()); } diff --git a/tests/phpunit/Configuration/Entry/LogsTest.php b/tests/phpunit/Configuration/Entry/LogsTest.php index b4de797f7..cd60b882e 100644 --- a/tests/phpunit/Configuration/Entry/LogsTest.php +++ b/tests/phpunit/Configuration/Entry/LogsTest.php @@ -112,7 +112,7 @@ public function valuesProvider(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master', null), + new Badge('master'), ]; } } diff --git a/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php b/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php index 8514bd9b3..7e3e05a48 100644 --- a/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php +++ b/tests/phpunit/Configuration/Schema/SchemaConfigurationFactoryTest.php @@ -363,7 +363,7 @@ public function provideRawConfig(): iterable null, null, false, - new Badge('master', null) + new Badge('master') ), ]), ]; @@ -376,7 +376,7 @@ public function provideRawConfig(): iterable }, "logs": { "badge": { - "matchBranchRegex": "/^foo$/" + "branch": "/^foo$/" } } } @@ -391,7 +391,7 @@ public function provideRawConfig(): iterable null, null, false, - new Badge(null, '/^foo$/') + new Badge('/^foo$/') ), ]), ]; @@ -425,7 +425,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master', null) + new Badge('master') ), ]), ]; @@ -466,7 +466,7 @@ public function provideRawConfig(): iterable "debug": "", "perMutator": "", "badge": { - "matchBranchRegex": "" + "branch": "" } } } @@ -507,7 +507,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master', null) + new Badge('master') ), ]), ]; @@ -2377,7 +2377,7 @@ public function provideRawConfig(): iterable 'debug.log', 'perMutator.log', true, - new Badge('master', null) + new Badge('master') ), 'tmpDir' => 'custom-tmp', 'phpunit' => new PhpUnit( diff --git a/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php b/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php index fa2a428ea..fb72194ee 100644 --- a/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php +++ b/tests/phpunit/Configuration/Schema/SchemaConfigurationTest.php @@ -126,7 +126,7 @@ public function valueProvider(): iterable 'debug.log', 'mutator.log', true, - new Badge('master', null) + new Badge('master') ), 'path/to/tmp', new PhpUnit('dist/phpunit', 'bin/phpunit'), diff --git a/tests/phpunit/Logger/BadgeLoggerFactoryTest.php b/tests/phpunit/Logger/BadgeLoggerFactoryTest.php index 4560b4136..9c89aeb35 100644 --- a/tests/phpunit/Logger/BadgeLoggerFactoryTest.php +++ b/tests/phpunit/Logger/BadgeLoggerFactoryTest.php @@ -121,7 +121,7 @@ public function test_it_creates_a_badge_logger_on_no_verbosity(): void null, null, false, - new Badge('master', null) + new Badge('master') ) ); @@ -167,7 +167,7 @@ public function logsProvider(): iterable null, null, false, - new Badge('foo', null) + new Badge('foo') ), BadgeLogger::class, ]; @@ -180,7 +180,7 @@ public function logsProvider(): iterable 'debug', 'per_mutator', true, - new Badge('branch', null) + new Badge('branch') ), BadgeLogger::class, ]; diff --git a/tests/phpunit/Logger/BadgeLoggerTest.php b/tests/phpunit/Logger/BadgeLoggerTest.php index ca8d8144b..52d3a0375 100644 --- a/tests/phpunit/Logger/BadgeLoggerTest.php +++ b/tests/phpunit/Logger/BadgeLoggerTest.php @@ -35,6 +35,7 @@ namespace Infection\Tests\Logger; +use Infection\Configuration\Entry\Badge; use Infection\Environment\BuildContextResolver; use Infection\Environment\StrykerApiKeyResolver; use Infection\Logger\BadgeLogger; @@ -91,8 +92,7 @@ protected function setUp(): void new StrykerApiKeyResolver(), $this->badgeApiClientMock, $this->metricsCalculatorMock, - 'master', - null, + new Badge('master'), $this->logger ); } @@ -228,7 +228,7 @@ public function test_it_skips_logging_when_it_is_branch_not_from_config(): void [ [ LogLevel::WARNING, - 'Dashboard report has not been sent: Expected branch "master", found "foo"', + 'Dashboard report has not been sent: Branch "foo" does not match expected badge configuration', [], ], ], @@ -255,8 +255,7 @@ public function test_it_skips_logging_when_it_is_branch_not_from_config_regex(): new StrykerApiKeyResolver(), $this->badgeApiClientMock, $this->metricsCalculatorMock, - null, - '/^\d+\\.x$/', + new Badge('/^\d+\\.x$/'), $this->logger ); @@ -266,7 +265,7 @@ public function test_it_skips_logging_when_it_is_branch_not_from_config_regex(): [ [ LogLevel::WARNING, - 'Dashboard report has not been sent: Expected branch to match regex "/^\d+\\.x$/", found "1.x-mismatch"', + 'Dashboard report has not been sent: Branch "1.x-mismatch" does not match expected badge configuration', [], ], ], @@ -368,8 +367,7 @@ public function test_it_sends_report_when_everything_is_ok_with_stryker_key_and_ new StrykerApiKeyResolver(), $this->badgeApiClientMock, $this->metricsCalculatorMock, - null, - '/^\d+\\.x$/', + new Badge('/^\d+\\.x$/'), $this->logger ); diff --git a/tests/phpunit/Logger/FileLoggerFactoryTest.php b/tests/phpunit/Logger/FileLoggerFactoryTest.php index a76879068..28c08029c 100644 --- a/tests/phpunit/Logger/FileLoggerFactoryTest.php +++ b/tests/phpunit/Logger/FileLoggerFactoryTest.php @@ -220,7 +220,7 @@ public function logsProvider(): iterable 'debug', 'per_mutator', true, - new Badge('branch', null) + new Badge('branch') ), [ TextFileLogger::class, From fc5634d940602da773da056d4c117480b84e0ea9 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 21 Jul 2021 10:08:29 +0200 Subject: [PATCH 3/4] Documented when `Badge::__construct()` could throw --- src/Configuration/Entry/Badge.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/Configuration/Entry/Badge.php b/src/Configuration/Entry/Badge.php index 2473c2953..76a573f84 100644 --- a/src/Configuration/Entry/Badge.php +++ b/src/Configuration/Entry/Badge.php @@ -48,6 +48,9 @@ final class Badge { private string $branchMatch; + /** + * @throws InvalidArgumentException when the provided $branch looks like a regular expression, but is not a valid one + */ public function __construct(string $branch) { if (preg_match('#^/.+/$#', $branch) === 0) { From 2925fe2237d959b80bf82b4ab7afe3aa430da0c8 Mon Sep 17 00:00:00 2001 From: Marco Pivetta Date: Wed, 21 Jul 2021 10:20:57 +0200 Subject: [PATCH 4/4] Refactored `BadgeTest` so that we do verify branch match semantics through a data provider Previously, we had a single test with multiple assertions. Ref: https://github.com/infection/infection/pull/1538#discussion_r673682313 --- .../phpunit/Configuration/Entry/BadgeTest.php | 45 +++++++++++-------- 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/tests/phpunit/Configuration/Entry/BadgeTest.php b/tests/phpunit/Configuration/Entry/BadgeTest.php index 8d599287b..61f850f72 100644 --- a/tests/phpunit/Configuration/Entry/BadgeTest.php +++ b/tests/phpunit/Configuration/Entry/BadgeTest.php @@ -41,28 +41,37 @@ final class BadgeTest extends TestCase { - public function test_it_can_be_instantiated_with_an_exact_branch_match(): void + /** @dataProvider branch_names_to_be_matched */ + public function test_branch_match(string $branchName, string $branchMatch, bool $willMatch): void { - $badge = new Badge('master'); - - $this->assertTrue($badge->applicableForBranch('master')); - $this->assertFalse($badge->applicableForBranch('mast')); - $this->assertFalse($badge->applicableForBranch('master ')); - $this->assertFalse($badge->applicableForBranch(' master')); - $this->assertFalse($badge->applicableForBranch('master1')); + $this->assertSame( + $willMatch, + (new Badge($branchMatch)) + ->applicableForBranch($branchName) + ); } - public function test_it_can_be_instantiated_with_a_regex_branch_match(): void + /** @return non-empty-list */ + public function branch_names_to_be_matched(): array { - $badge = new Badge('/^(foo|bar)$/'); - - $this->assertTrue($badge->applicableForBranch('foo')); - $this->assertTrue($badge->applicableForBranch('bar')); - $this->assertFalse($badge->applicableForBranch('fo')); - $this->assertFalse($badge->applicableForBranch('ba')); - $this->assertFalse($badge->applicableForBranch('foo ')); - $this->assertFalse($badge->applicableForBranch(' foo')); - $this->assertFalse($badge->applicableForBranch('foo1')); + return [ + ['master', 'master', true], + ['main', 'main', true], + ['main', 'master', false], + ['mast', 'master', false], + ['master ', 'master', false], + [' master', 'master', false], + [' master ', 'master', false], + ['master1', 'master', false], + ['foo', '/^(foo|bar)$/', true], + ['bar', '/^(foo|bar)$/', true], + ['foobar', '/^(foo|bar)$/', false], + ['fo', '/^(foo|bar)$/', false], + ['ba', '/^(foo|bar)$/', false], + ['foo ', '/^(foo|bar)$/', false], + [' foo', '/^(foo|bar)$/', false], + ['foo1', '/^(foo|bar)$/', false], + ]; } public function test_it_rejects_invalid_regex(): void