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

Use parent array for coverage #655

Merged
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
14 changes: 13 additions & 1 deletion src/Mutation.php
Expand Up @@ -93,6 +93,11 @@ final class Mutation implements MutationInterface
*/
private $mutationByMutatorIndex;

/**
* @var array|int[]
*/
private $lineRange;

public function __construct(
string $originalFilePath,
array $originalFileAst,
Expand All @@ -102,7 +107,8 @@ public function __construct(
bool $isOnFunctionSignature,
bool $isCoveredByTest,
$mutatedNode,
int $mutationByMutatorIndex
int $mutationByMutatorIndex,
array $lineRange
) {
$this->originalFilePath = $originalFilePath;
$this->originalFileAst = $originalFileAst;
Expand All @@ -113,6 +119,7 @@ public function __construct(
$this->isCoveredByTest = $isCoveredByTest;
$this->mutatedNode = $mutatedNode;
$this->mutationByMutatorIndex = $mutationByMutatorIndex;
$this->lineRange = $lineRange;
}

public function getMutator(): Mutator
Expand Down Expand Up @@ -183,4 +190,9 @@ public function getMutatedNode()
{
return $this->mutatedNode;
}

public function getLineRange(): array
{
return $this->lineRange;
}
}
2 changes: 2 additions & 0 deletions src/MutationInterface.php
Expand Up @@ -65,4 +65,6 @@ public function isCoveredByTest(): bool;
* @return Node|Node[] Node, array of Nodes
*/
public function getMutatedNode();

public function getLineRange(): array;
}
18 changes: 2 additions & 16 deletions src/TestFramework/Coverage/CodeCoverageData.php
Expand Up @@ -147,7 +147,7 @@ private function getTestsForMutationOnFunctionSignature(MutationInterface $mutat
{
$filePath = $mutation->getOriginalFilePath();

foreach ($this->linesForMutation($mutation) as $line) {
foreach ($mutation->getLineRange() as $line) {
if ($this->hasExecutedMethodOnLine($filePath, $line)) {
yield from $this->getTestsForExecutedMethodOnLine($filePath, $line);
}
Expand All @@ -158,27 +158,13 @@ private function getTestsForMutation(MutationInterface $mutation): \Generator
{
$filePath = $mutation->getOriginalFilePath();

foreach ($this->linesForMutation($mutation) as $line) {
foreach ($mutation->getLineRange() as $line) {
if ($this->hasTestsOnLine($filePath, $line)) {
yield from $this->getCoverage()[$filePath]['byLine'][$line];
}
}
}

private function linesForMutation(MutationInterface $mutation): \Generator
{
/*
* If we only look for tests that cover only the very first line of our
* mutation, we may naturally miss other tests that may actually also
* cover this mutation, this all leading to false positives.
*
* Therefore we have to consider all lines of a mutation.
*/
for ($line = $mutation->getAttributes()['startLine']; $line <= $mutation->getAttributes()['endLine']; ++$line) {
yield $line;
}
}

/**
* coverage[$sourceFilePath] = [
* 'byMethod' => [
Expand Down
52 changes: 39 additions & 13 deletions src/Visitor/MutationsCollectorVisitor.php
Expand Up @@ -109,7 +109,24 @@ public function leaveNode(Node $node): void
continue;
}

$isCoveredByTest = $this->isCoveredByTest($isOnFunctionSignature, $node);
if ($isOnFunctionSignature) {
// hasExecutedMethodOnLine checks for all lines of a given method,
// therefore it isn't making any sense to check any other line but first
$isCoveredByTest = $this->codeCoverageData->hasExecutedMethodOnLine($this->filePath, $node->getLine());
$linerange = $this->getNodeRange($node);
} else {
$outerMostArrayNode = $this->getOuterMostArrayNode($node);
$isCoveredByTest = false;
$linerange = $this->getNodeRange($outerMostArrayNode);

foreach ($linerange as $line) {
if ($this->codeCoverageData->hasTestsOnLine($this->filePath, $line)) {
$isCoveredByTest = true;

break;
}
}
}

if ($this->onlyCovered && !$isCoveredByTest) {
continue;
Expand All @@ -129,7 +146,8 @@ public function leaveNode(Node $node): void
$isOnFunctionSignature,
$isCoveredByTest,
$mutatedNode,
$mutationByMutatorIndex
$mutationByMutatorIndex,
$linerange
);
}
}
Expand All @@ -143,20 +161,28 @@ public function getMutations(): array
return $this->mutations;
}

private function isCoveredByTest(bool $isOnFunctionSignature, Node $node): bool
/**
* If the node is part of an array, this will find the outermost array.
* Otherwise this will return the node itself
*/
private function getOuterMostArrayNode(Node $node): Node
{
if ($isOnFunctionSignature) {
// hasExecutedMethodOnLine checks for all lines of a given method,
// therefore it isn't making any sense to check any other line but first
return $this->codeCoverageData->hasExecutedMethodOnLine($this->filePath, $node->getLine());
}
$outerMostArrayParent = $node;

for ($line = $node->getStartLine(); $line <= $node->getEndLine(); ++$line) {
if ($this->codeCoverageData->hasTestsOnLine($this->filePath, $line)) {
return true;
do {
if ($node instanceof Node\Expr\Array_) {
$outerMostArrayParent = $node;
}
}
} while ($node = $node->getAttribute(ParentConnectorVisitor::PARENT_KEY));

return false;
return $outerMostArrayParent;
}

/**
* @return array|int[]
*/
private function getNodeRange(Node $node): array
{
return range($node->getStartLine(), $node->getEndLine());
}
}
38 changes: 35 additions & 3 deletions tests/MutationTest.php
Expand Up @@ -67,7 +67,8 @@ public function test_it_correctly_generates_hash(): void
false,
true,
new Node\Scalar\LNumber(1),
0
0,
[1, 2, 3]
);

$this->assertSame('2930c05082a35248987760a81b9f9a08', $mutation->getHash());
Expand All @@ -94,7 +95,8 @@ public function test_it_correctly_sets_is_on_function_signature(): void
false,
true,
new Node\Scalar\LNumber(1),
0
0,
[1, 2, 3]
);

$this->assertFalse($mutation->isOnFunctionSignature());
Expand Down Expand Up @@ -122,9 +124,39 @@ public function test_it_correctly_sets_original_file_ast(): void
false,
true,
new Node\Scalar\LNumber(1),
0
0,
[1, 2, 3]
);

$this->assertSame($fileAst, $mutation->getOriginalFileAst());
}

public function test_it_correctly_sets_line_range(): void
{
$mutator = new Plus(new MutatorConfig([]));
$attributes = [
'startLine' => 3,
'endLine' => 5,
'startTokenPos' => 21,
'endTokenPos' => 31,
'startFilePos' => 43,
'endFilePos' => 53,
];
$range = [21, 22, 23, 24];

$mutation = new Mutation(
'/abc.php',
['file' => 'ast'],
$mutator,
$attributes,
'Interface_',
false,
true,
new Node\Scalar\LNumber(1),
0,
$range
);

$this->assertSame($range, $mutation->getLineRange());
}
}
12 changes: 8 additions & 4 deletions tests/TestFramework/Coverage/CodeCoverageDataTest.php
Expand Up @@ -149,7 +149,8 @@ public function test_it_returns_zero_tests_for_not_covered_function_body_mutator
false,
true,
new LNumber(1),
0
0,
[1]
);

$this->assertCount(0, $codeCoverageData->getAllTestsFor($mutation));
Expand All @@ -169,7 +170,8 @@ public function test_it_returns_tests_for_covered_function_body_mutator(): void
false,
true,
new LNumber(1),
0
0,
[26]
);

$this->assertCount(2, $codeCoverageData->getAllTestsFor($mutation));
Expand All @@ -189,7 +191,8 @@ public function test_it_returns_zero_tests_for_not_covered_function_signature_mu
true,
true,
new LNumber(1),
0
0,
[1]
);

$this->assertCount(0, $codeCoverageData->getAllTestsFor($mutation));
Expand All @@ -209,7 +212,8 @@ public function test_it_returns_tests_for_covered_function_signature_mutator():
true,
true,
new LNumber(1),
0
0,
[24]
);

$this->assertCount(6, $codeCoverageData->getAllTestsFor($mutation));
Expand Down
9 changes: 6 additions & 3 deletions tests/Visitor/MutatorVisitorTest.php
Expand Up @@ -121,7 +121,8 @@ public function hello() : string
true,
true,
new Nop(),
0
0,
range(29, 48)
),
];

Expand Down Expand Up @@ -170,7 +171,8 @@ public function bye() : string
true,
true,
new Nop(),
0
0,
range(29, 50)
),
];
$badLexer = new Lexer\Emulative([
Expand Down Expand Up @@ -226,7 +228,8 @@ public function bye() : string
true,
true,
new Nop(),
0
0,
range(29, 48)
),
];
}
Expand Down