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

[Performance] Improve Infection performance executed against slow test suites #1539

Merged
merged 8 commits into from Jul 25, 2021
11 changes: 10 additions & 1 deletion src/Command/RunCommand.php
Expand Up @@ -116,6 +116,8 @@ final class RunCommand extends BaseCommand

private const OPTION_USE_NOOP_MUTATORS = 'noop';

private const OPTION_EXECUTE_ONLY_COVERING_TEST_CASES = 'only-covering-test-cases';

/** @var string */
private const OPTION_MIN_MSI = 'min-msi';

Expand Down Expand Up @@ -257,6 +259,12 @@ protected function configure(): void
InputOption::VALUE_NONE,
'Use noop mutators that do not change AST. For debugging purposes.',
)
->addOption(
self::OPTION_EXECUTE_ONLY_COVERING_TEST_CASES,
null,
InputOption::VALUE_NONE,
'Execute only those test cases that cover mutated line, not the whole file with covering test cases. Can dramatically speed up Mutation Testing for slow test suites. For PHPUnit / Pest it uses `--filter` option',
)
->addOption(
self::OPTION_MIN_MSI,
null,
Expand Down Expand Up @@ -444,7 +452,8 @@ private function createContainer(IO $io, LoggerInterface $logger): Container
$gitDiffFilter,
$gitDiffBase,
(bool) $input->getOption(self::OPTION_LOGGER_GITHUB),
(bool) $input->getOption(self::OPTION_USE_NOOP_MUTATORS)
(bool) $input->getOption(self::OPTION_USE_NOOP_MUTATORS),
(bool) $input->getOption(self::OPTION_EXECUTE_ONLY_COVERING_TEST_CASES)
);
}

Expand Down
10 changes: 9 additions & 1 deletion src/Configuration/Configuration.php
Expand Up @@ -87,6 +87,7 @@ class Configuration
private bool $dryRun;
/** @var array<string, array<int, string>> */
private array $ignoreSourceCodeMutatorsMap;
private bool $executeOnlyCoveringTestCases;

/**
* @param string[] $sourceDirectories
Expand Down Expand Up @@ -123,7 +124,8 @@ public function __construct(
int $msiPrecision,
int $threadCount,
bool $dryRun,
array $ignoreSourceCodeMutatorsMap
array $ignoreSourceCodeMutatorsMap,
bool $executeOnlyCoveringTestCases
) {
Assert::nullOrGreaterThanEq($timeout, 0);
Assert::allString($sourceDirectories);
Expand Down Expand Up @@ -161,6 +163,7 @@ public function __construct(
$this->threadCount = $threadCount;
$this->dryRun = $dryRun;
$this->ignoreSourceCodeMutatorsMap = $ignoreSourceCodeMutatorsMap;
$this->executeOnlyCoveringTestCases = $executeOnlyCoveringTestCases;
}

public function getProcessTimeout(): float
Expand Down Expand Up @@ -317,4 +320,9 @@ public function getIgnoreSourceCodeMutatorsMap(): array
{
return $this->ignoreSourceCodeMutatorsMap;
}

public function getExecuteOnlyCoveringTestCases(): bool
{
return $this->executeOnlyCoveringTestCases;
}
}
6 changes: 4 additions & 2 deletions src/Configuration/ConfigurationFactory.php
Expand Up @@ -118,7 +118,8 @@ public function create(
?string $gitDiffFilter,
?string $gitDiffBase,
bool $useGitHubLogger,
bool $useNoopMutators
bool $useNoopMutators,
bool $executeOnlyCoveringTestCases
): Configuration {
$configDir = dirname($schema->getFile());

Expand Down Expand Up @@ -170,7 +171,8 @@ public function create(
$msiPrecision,
$threadCount,
$dryRun,
$ignoreSourceCodeMutatorsMap
$ignoreSourceCodeMutatorsMap,
$executeOnlyCoveringTestCases
);
}

Expand Down
13 changes: 9 additions & 4 deletions src/Container.php
Expand Up @@ -166,6 +166,7 @@ final class Container
public const DEFAULT_GIT_DIFF_BASE = null;
public const DEFAULT_USE_GITHUB_LOGGER = false;
public const DEFAULT_USE_NOOP_MUTATORS = false;
public const DEFAULT_EXECUTE_ONLY_COVERING_TEST_CASES = false;
public const DEFAULT_NO_PROGRESS = false;
public const DEFAULT_FORCE_PROGRESS = false;
public const DEFAULT_EXISTING_COVERAGE_PATH = null;
Expand Down Expand Up @@ -684,7 +685,8 @@ public static function create(): self
self::DEFAULT_GIT_DIFF_FILTER,
self::DEFAULT_GIT_DIFF_BASE,
self::DEFAULT_USE_GITHUB_LOGGER,
self::DEFAULT_USE_NOOP_MUTATORS
self::DEFAULT_USE_NOOP_MUTATORS,
self::DEFAULT_EXECUTE_ONLY_COVERING_TEST_CASES
);
}

Expand Down Expand Up @@ -715,7 +717,8 @@ public function withValues(
?string $gitDiffFilter,
?string $gitDiffBase,
bool $useGitHubLogger,
bool $useNoopMutators
bool $useNoopMutators,
bool $executeOnlyCoveringTestCases
): self {
$clone = clone $this;

Expand Down Expand Up @@ -790,7 +793,8 @@ static function (self $container) use (
$gitDiffFilter,
$gitDiffBase,
$useGitHubLogger,
$useNoopMutators
$useNoopMutators,
$executeOnlyCoveringTestCases
): Configuration {
return $container->getConfigurationFactory()->create(
$container->getSchemaConfiguration(),
Expand All @@ -815,7 +819,8 @@ static function (self $container) use (
$gitDiffFilter,
$gitDiffBase,
$useGitHubLogger,
$useNoopMutators
$useNoopMutators,
$executeOnlyCoveringTestCases
);
}
);
Expand Down
34 changes: 19 additions & 15 deletions src/TestFramework/AbstractTestFrameworkAdapter.php
Expand Up @@ -91,7 +91,10 @@ public function getInitialTestRunCommandLine(
array $phpExtraArgs,
bool $skipCoverage
): array {
return $this->getCommandLine($this->buildInitialConfigFile(), $extraOptions, $phpExtraArgs);
return $this->getCommandLine(
$phpExtraArgs,
$this->argumentsAndOptionsBuilder->buildForInitialTestsRun($this->buildInitialConfigFile(), $extraOptions)
);
}

/**
Expand All @@ -109,14 +112,17 @@ public function getMutantCommandLine(
string $extraOptions
): array {
return $this->getCommandLine(
$this->buildMutationConfigFile(
$tests,
$mutantFilePath,
$mutationHash,
$mutationOriginalFilePath
),
$extraOptions,
[]
[],
$this->argumentsAndOptionsBuilder->buildForMutant(
$this->buildMutationConfigFile(
$tests,
$mutantFilePath,
$mutationHash,
$mutationOriginalFilePath
),
$extraOptions,
$tests
)
);
}

Expand Down Expand Up @@ -154,20 +160,18 @@ protected function buildMutationConfigFile(

/**
* @param string[] $phpExtraArgs
* @param string[] $testFrameworkArgs
*
* @return string[]
*/
private function getCommandLine(
string $configPath,
string $extraOptions,
array $phpExtraArgs
array $phpExtraArgs,
array $testFrameworkArgs
): array {
$frameworkArgs = $this->argumentsAndOptionsBuilder->build($configPath, $extraOptions);

return $this->commandLineBuilder->build(
$this->testFrameworkExecutable,
$phpExtraArgs,
$frameworkArgs
$testFrameworkArgs
);
}

Expand Down
11 changes: 10 additions & 1 deletion src/TestFramework/CommandLineArgumentsAndOptionsBuilder.php
Expand Up @@ -35,6 +35,8 @@

namespace Infection\TestFramework;

use Infection\AbstractTestFramework\Coverage\TestLocation;

/**
* @internal
*/
Expand All @@ -43,5 +45,12 @@ interface CommandLineArgumentsAndOptionsBuilder
/**
* @return string[]
*/
public function build(string $configPath, string $extraOptions): array;
public function buildForInitialTestsRun(string $configPath, string $extraOptions): array;

/**
* @param TestLocation[] $tests
*
* @return string[]
*/
public function buildForMutant(string $configPath, string $extraOptions, array $tests): array;
}
6 changes: 4 additions & 2 deletions src/TestFramework/Factory.php
Expand Up @@ -102,7 +102,8 @@ public function create(string $adapterName, bool $skipCoverage): TestFrameworkAd
$this->jUnitFilePath,
$this->projectDir,
$this->infectionConfig->getSourceDirectories(),
$skipCoverage
$skipCoverage,
$this->infectionConfig->getExecuteOnlyCoveringTestCases()
);
}

Expand All @@ -120,7 +121,8 @@ public function create(string $adapterName, bool $skipCoverage): TestFrameworkAd
$this->jUnitFilePath,
$this->projectDir,
$this->infectionConfig->getSourceDirectories(),
$skipCoverage
$skipCoverage,
$this->infectionConfig->getExecuteOnlyCoveringTestCases()
);
}

Expand Down
5 changes: 3 additions & 2 deletions src/TestFramework/PhpUnit/Adapter/PestAdapterFactory.php
Expand Up @@ -64,7 +64,8 @@ public static function create(
string $jUnitFilePath,
string $projectDir,
array $sourceDirectories,
bool $skipCoverage
bool $skipCoverage,
bool $executeOnlyCoveringTestCases = false
): TestFrameworkAdapter {
Assert::string($testFrameworkConfigDir, 'Config dir is not allowed to be `null` for the Pest adapter');

Expand Down Expand Up @@ -97,7 +98,7 @@ public static function create(
$projectDir,
new JUnitTestCaseSorter()
),
new ArgumentsAndOptionsBuilder(),
new ArgumentsAndOptionsBuilder($executeOnlyCoveringTestCases),
new VersionParser(),
new CommandLineBuilder()
);
Expand Down
5 changes: 3 additions & 2 deletions src/TestFramework/PhpUnit/Adapter/PhpUnitAdapterFactory.php
Expand Up @@ -67,7 +67,8 @@ public static function create(
string $jUnitFilePath,
string $projectDir,
array $sourceDirectories,
bool $skipCoverage
bool $skipCoverage,
bool $executeOnlyCoveringTestCases = false
): TestFrameworkAdapter {
Assert::string($testFrameworkConfigDir, 'Config dir is not allowed to be `null` for the Pest adapter');

Expand Down Expand Up @@ -100,7 +101,7 @@ public static function create(
$projectDir,
new JUnitTestCaseSorter()
),
new ArgumentsAndOptionsBuilder(),
new ArgumentsAndOptionsBuilder($executeOnlyCoveringTestCases),
new VersionParser(),
new CommandLineBuilder()
);
Expand Down
Expand Up @@ -35,18 +35,30 @@

namespace Infection\TestFramework\PhpUnit\CommandLine;

use function array_key_exists;
use function array_map;
use function array_merge;
use function count;
use function explode;
use Infection\AbstractTestFramework\Coverage\TestLocation;
use Infection\TestFramework\CommandLineArgumentsAndOptionsBuilder;
use function ltrim;
use function preg_quote;
use function rtrim;

/**
* @internal
*/
final class ArgumentsAndOptionsBuilder implements CommandLineArgumentsAndOptionsBuilder
{
public function build(string $configPath, string $extraOptions): array
private bool $executeOnlyCoveringTestCases;

public function __construct(bool $executeOnlyCoveringTestCases)
{
$this->executeOnlyCoveringTestCases = $executeOnlyCoveringTestCases;
}

public function buildForInitialTestsRun(string $configPath, string $extraOptions): array
{
$options = [
'--configuration',
Expand All @@ -64,4 +76,36 @@ public function build(string $configPath, string $extraOptions): array

return $options;
}

/**
* @param TestLocation[] $tests
*/
public function buildForMutant(string $configPath, string $extraOptions, array $tests): array
{
$options = $this->buildForInitialTestsRun($configPath, $extraOptions);

if ($this->executeOnlyCoveringTestCases && count($tests) > 0) {
$filterString = '/';
$usedTestCases = [];

foreach ($tests as $testLocation) {
$testCaseString = $testLocation->getMethod();

if (array_key_exists($testCaseString, $usedTestCases)) {
continue;
}

$usedTestCases[$testCaseString] = true;

$filterString .= preg_quote($testCaseString, '/') . '|';
}

$filterString = rtrim($filterString, '|') . '/';
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure we absolutely need this computation? I mean, is there a chance we won't run this mutant ever? And what about mutants on the same line of the same file?

Copy link
Member Author

Choose a reason for hiding this comment

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

didn't get you, could you please clarify what do you mean?

This code just builds '/App\\\\Test::test_case1|App\\\\Test::test_case2/' string, meaning to run only 2 test cases of X. Those tests cases - is 100% of all the test cases that cover particular mutated line (Mutant)


$options[] = '--filter';
$options[] = $filterString;
}

return $options;
}
}
Expand Up @@ -10,7 +10,7 @@
[2.0, 4.0, 0.5]
]);

test('tests division with shared dataset', function (float $a, float $b, float $expectedResult) {
test('tests division with shared dataset, with special | char', function (float $a, float $b, float $expectedResult) {
$sourceClass = new ForPestWithDataProvider();

expect($sourceClass->div($a, $b))->toBe($expectedResult);
Expand Down
4 changes: 3 additions & 1 deletion tests/phpunit/Configuration/ConfigurationAssertions.php
Expand Up @@ -83,7 +83,8 @@ private function assertConfigurationStateIs(
int $expectedMsiPrecision,
int $expectedThreadCount,
bool $expectedDryRyn,
array $expectedIgnoreSourceCodeMutatorsMap
array $expectedIgnoreSourceCodeMutatorsMap,
bool $expectedExecuteOnlyCoveringTestCases
): void {
$this->assertSame($expectedTimeout, $configuration->getProcessTimeout());
$this->assertSame($expectedSourceDirectories, $configuration->getSourceDirectories());
Expand Down Expand Up @@ -132,6 +133,7 @@ private function assertConfigurationStateIs(
$this->assertSame($expectedThreadCount, $configuration->getThreadCount());
$this->assertSame($expectedDryRyn, $configuration->isDryRun());
$this->assertSame($expectedIgnoreSourceCodeMutatorsMap, $configuration->getIgnoreSourceCodeMutatorsMap());
$this->assertSame($expectedExecuteOnlyCoveringTestCases, $configuration->getExecuteOnlyCoveringTestCases());
}

/**
Expand Down