Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made logs.badge.branch capable of accepting a regular expression #1538

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 11 additions & 1 deletion resources/schema.json
Expand Up @@ -65,12 +65,22 @@
},
"badge": {
"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.",
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
"additionalProperties": false,
"required": ["branch"],
"properties": {
"branch": {
"type": "string",
"description": "Mutation score badge for your GitHub project."
"description": "Mutation score badge for your GitHub project. If this value starts and ends with \"/\", it will be considered a regular expression.",
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
"exaples": [
"main",
"master",
"develop",
"latest",
"/1\\.\\d+/",
"/release-.*/",
"/feature\\/.*/"
]
}
}
},
Expand Down
34 changes: 30 additions & 4 deletions src/Configuration/Entry/Badge.php
Expand Up @@ -35,20 +35,46 @@

namespace Infection\Configuration\Entry;

use InvalidArgumentException;
use function preg_quote;
use Safe\Exceptions\PcreException;
use function Safe\preg_match;
use function Safe\sprintf;

/**
* @internal
*/
final class Badge
{
private string $branch;
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)
{
$this->branch = $branch;
if (preg_match('#^/.+/$#', $branch) === 0) {
$this->branchMatch = '/^' . preg_quote($branch, '/') . '$/';

return;
}

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
);
}

$this->branchMatch = $branch;
}

public function getBranch(): string
public function applicableForBranch(string $branchName): bool
{
return $this->branch;
return preg_match($this->branchMatch, $branchName) === 1;
}
}
18 changes: 10 additions & 8 deletions src/Logger/BadgeLogger.php
Expand Up @@ -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;
Expand All @@ -54,22 +55,22 @@ final class BadgeLogger implements MutationTestingResultsLogger
private StrykerApiKeyResolver $strykerApiKeyResolver;
private StrykerDashboardClient $strykerDashboardClient;
private MetricsCalculator $metricsCalculator;
private string $branch;
private Badge $badge;
private LoggerInterface $logger;

public function __construct(
BuildContextResolver $buildContextResolver,
StrykerApiKeyResolver $strykerApiKeyResolver,
StrykerDashboardClient $strykerDashboardClient,
MetricsCalculator $metricsCalculator,
string $branch,
Badge $badge,
LoggerInterface $logger
) {
$this->buildContextResolver = $buildContextResolver;
$this->strykerApiKeyResolver = $strykerApiKeyResolver;
$this->strykerDashboardClient = $strykerDashboardClient;
$this->metricsCalculator = $metricsCalculator;
$this->branch = $branch;
$this->badge = $badge;
$this->logger = $logger;
}

Expand All @@ -83,11 +84,12 @@ public function log(): void
return;
}

if ($buildContext->branch() !== $this->branch) {
$branch = $buildContext->branch();

if (!$this->badge->applicableForBranch($branch)) {
$this->logReportWasNotSent(sprintf(
'Expected branch "%s", found "%s"',
$this->branch,
$buildContext->branch()
'Branch "%s" does not match expected badge configuration',
$branch
));

return;
Expand All @@ -106,7 +108,7 @@ public function log(): void

$this->strykerDashboardClient->sendReport(
'github.com/' . $buildContext->repositorySlug(),
$buildContext->branch(),
$branch,
$apiKey,
$this->metricsCalculator->getMutationScoreIndicator()
);
Expand Down
6 changes: 4 additions & 2 deletions src/Logger/BadgeLoggerFactory.php
Expand Up @@ -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;
}

Expand All @@ -78,7 +80,7 @@ public function createFromLogEntries(Logs $logConfig): ?MutationTestingResultsLo
$this->logger
),
$this->metricsCalculator,
$logConfig->getBadge()->getBranch(),
$badge,
$this->logger
);
}
Expand Down
Expand Up @@ -65,7 +65,8 @@ static function (SplFileInfo $fileInfo): Trace {
);

$mutators = $container->getMutatorFactory()->create(
$container->getMutatorResolver()->resolve(['@default' => true])
$container->getMutatorResolver()->resolve(['@default' => true]),
true
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
);

$fileMutationGenerator = $container->getFileMutationGenerator();
Expand Down
3 changes: 2 additions & 1 deletion tests/benchmark/Tracing/provide-traces-closure.php
Expand Up @@ -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
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
);

$generateTraces = static function (?int $maxCount) use ($container): iterable {
Expand Down
48 changes: 0 additions & 48 deletions tests/phpunit/Configuration/Entry/BadgeAssertions.php

This file was deleted.

46 changes: 42 additions & 4 deletions tests/phpunit/Configuration/Entry/BadgeTest.php
Expand Up @@ -36,16 +36,54 @@
namespace Infection\Tests\Configuration\Entry;

use Infection\Configuration\Entry\Badge;
use InvalidArgumentException;
use PHPUnit\Framework\TestCase;

final class BadgeTest extends TestCase
{
use BadgeAssertions;
/** @dataProvider branch_names_to_be_matched */
public function test_branch_match(string $branchName, string $branchMatch, bool $willMatch): void
{
$this->assertSame(
$willMatch,
(new Badge($branchMatch))
->applicableForBranch($branchName)
);
}

/** @return non-empty-list<array{string, non-empty-string, bool}> */
public function branch_names_to_be_matched(): array
{
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],
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
];
}

public function test_it_can_be_instantiated(): void
public function test_it_rejects_invalid_regex(): void
{
$badge = new Badge('master');
try {
new Badge('/[/');

$this->assertBadgeStateIs($badge, 'master');
$this->fail();
Ocramius marked this conversation as resolved.
Show resolved Hide resolved
} catch (InvalidArgumentException $invalid) {
$this->assertSame('Provided branchMatchRegex "/[/" is not a valid regex', $invalid->getMessage());
$this->assertSame(0, $invalid->getCode());
$this->assertNotNull($invalid->getPrevious());
}
}
}
5 changes: 2 additions & 3 deletions tests/phpunit/Configuration/Entry/LogsAssertions.php
Expand Up @@ -40,8 +40,6 @@

trait LogsAssertions
{
use BadgeAssertions;

private function assertLogsStateIs(
Logs $logs,
?string $expectedTextLogFilePath,
Expand All @@ -65,7 +63,8 @@ private function assertLogsStateIs(
$this->assertNull($badge);
} else {
$this->assertNotNull($badge);
$this->assertBadgeStateIs($badge, $expectedBadge->getBranch());
Copy link
Sponsor Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this: it made the tests much more confusing than they were, and a simpler solution was to do object-to-object equality.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

object-to-object equality is always tricky due to the loose comparison, but if anyone is interested there is https://github.com/webmozarts/strict-phpunit which solves that issue and allows to remove a lot of boilertemplate


self::assertEquals($expectedBadge, $badge);
}
}
}
Expand Up @@ -368,6 +368,34 @@ public function provideRawConfig(): iterable
]),
];

yield '[logs][badge] regex' => [
<<<'JSON'
{
"source": {
"directories": ["src"]
},
"logs": {
"badge": {
"branch": "/^foo$/"
}
}
}
JSON
,
self::createConfig([
'source' => new Source(['src'], []),
'logs' => new Logs(
null,
null,
null,
null,
null,
false,
new Badge('/^foo$/')
),
]),
];

yield '[logs] nominal' => [
<<<'JSON'
{
Expand Down Expand Up @@ -426,6 +454,30 @@ public function provideRawConfig(): iterable
]),
];

yield '[logs] empty branch match regex' => [
<<<'JSON'
{
"source": {
"directories": ["src"]
},
"logs": {
"text": "",
"summary": "",
"debug": "",
"perMutator": "",
"badge": {
"branch": ""
}
}
}
JSON
,
self::createConfig([
'source' => new Source(['src'], []),
'logs' => Logs::createEmpty(),
]),
];

yield '[logs] empty & untrimmed strings' => [
<<<'JSON'
{
Expand Down