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

ignoreSourceCodeByRegex: show ignored mutants on progress and summary #1607

Closed
Slamdunk opened this issue Dec 3, 2021 · 5 comments
Closed
Labels
DX Developer Experience Feature Request
Milestone

Comments

@Slamdunk
Copy link
Contributor

Slamdunk commented Dec 3, 2021

Currently the ignored mutants are not shown on output, and it's weird to me to have a counter that doesn't reach its end.
It also gives a false sense of confidence that 100% score is perfection: I like and want to have 100% score in this case, but it should be clear how perfect score was achieved.
Here's a diff of one of my output, red is actual and green is expected:

 Processing source code files: 7/7
-.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped
+.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped, I: ignored
 
 ..................................................   ( 50 / 544)
 ..................................................   (100 / 544)
 ..................................................   (150 / 544)
 ..............T...................................   (200 / 544)
 ............T.....................................   (250 / 544)
-...................TT
+...................TTIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (300 / 544)
+IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (350 / 544)
+IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (400 / 544)
+IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (450 / 544)
+IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII   (500 / 544)
+IIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII         (544 / 544)
 
 271 mutations were generated:
      267 mutants were killed
+     273 mutants were configured to be ignored
        0 mutants were not covered by tests
        0 covered mutants were not detected
        0 errors were encountered
        0 syntax errors were encountered
        4 time outs were encountered
        0 mutants required more time than configured

 Metrics:
          Mutation Score Indicator (MSI): 100%
          Mutation Code Coverage: 100%
          Covered Code MSI: 100%
@maks-rafalko
Copy link
Member

maks-rafalko commented Dec 11, 2021

it's weird to me to have a counter that doesn't reach its end.

Agree, this is absolutely weird, didn't see it previously.

I'm ok with adding I for ignored mutants, that will give more clarity for the end user.

Feel free to contribute, it's quite easy (though need some time to updated all tests). We have an example of adding Syntax Error to the output. This is a very similar change.

@maks-rafalko maks-rafalko added Feature Request DX Developer Experience labels Dec 11, 2021
@sidz
Copy link
Member

sidz commented Dec 12, 2021

I started to implement this feature request due to some free time and faced with a fact that we probably need to revise how ignoreSourceCodeByRegex and global-ignoreSourceCodeByRegex work and instead of adding I option we shouldn't generate mutants instead.

Why?

We have an ignore option which can be used to ignore some mutations as well as ignoreSourceCodeByRegex. But the main thing is that everything what is under ignore won't be covered by I see https://github.com/infection/infection/blob/0.25.4/src/Mutator/IgnoreMutator.php#L98.

But currently this mutator does not handle ignoreSourceCodeByRegex and we generate mutations which will be skipped later in https://github.com/infection/infection/blob/0.25.4/src/Process/Runner/MutationTestingRunner.php#L107-L121

IMO we need to work with regex in the same way like we do for typical ignore and in this case:

  • we won't have an issue with counter
  • we won't generate useless mutations (which btw can take time and memory)

Also I have a strong feeling that we can/should combine ignoreSourceCodeByRegex with ignore.

Thoughts?

@maks-rafalko
Copy link
Member

maks-rafalko commented Dec 12, 2021

IMO we need to work with regex in the same way like we do for typical ignore and in this case:

in this case, IgnoreMutator decorator should be aware of diff, or it should generate a source code string based on Node $node, to be able to apply regular expression.

In pseudo code it would be

# IgnoreMutator
public function canMutate(Node $node): bool
{

    if (/* ignore regex is in the config */) {
        $sourceCodeLine = // generate from $node;

        if ($this->diffSourceCodeMatcher->matches($sourceCodeLine, $sourceCodeRegex)) {
            return false;
        }
    }
}

what I want to say, that it's not for free, we would need to generate source code line. And to be honest I don't know how to do it and even if this is possible.

Consider the following case:

$a = str_replace('a', foo(c('test' . 'bar')));

and ignore regex:

/foo(.*)/

to ignore lines where foo() function is used.

  1. In this case, how would we ignore a mutation for str_replace deletion?
- $a = str_replace('a', foo(c('test')));
+ $a = foo(c('test'));
  1. how would we ignore a mutation for concatenation deletion?
- $a = str_replace('a', foo(c('test'. 'bar')));
+ $a = str_replace('a', foo(c('test')));

in this case generating the code from Node $node, which is concatenation of strings does not have parent c() and foo() calls, which makes it very hard to understand what to do here.

That' s why in the current implementation of ignoreSourceCodeByRegex we are working with diff, where we already have a line of code instead of just 1 node in the hierarchy.

If you will find the way to solve the issues above, I'm +1 for your suggestion about moving it to IgnoreMutator.

Also I have a strong feeling that we can/should combine ignoreSourceCodeByRegex with ignore.

Regarding this: I'm looking into possible syntax of ignore and ignoreSourceCodeByRegex here (https://infection.github.io/guide/how-to.html#Disable-in-particular-class-or-method-or-line) and can't understand how we can combine these approaches, because at least

  • regex can contain *: Assert::.*
  • glob patter can contain *: App\Api\*::productList

and * here has different behavior/syntax.

@sidz
Copy link
Member

sidz commented Dec 13, 2021

Implemented in #1612

@sidz sidz closed this as completed Dec 13, 2021
@maks-rafalko maks-rafalko added this to the next milestone Dec 13, 2021
@maks-rafalko
Copy link
Member

Released: https://github.com/infection/infection/releases/tag/0.25.5

@maks-rafalko maks-rafalko modified the milestones: next, 0.26.0 Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Developer Experience Feature Request
Projects
None yet
Development

No branches or pull requests

3 participants