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

cannot use avoidCallsTo to protect *calls* to certain methods from mutation #404

Open
fmobus opened this issue Sep 26, 2017 · 10 comments
Open

Comments

@fmobus
Copy link

fmobus commented Sep 26, 2017

I am facing a situation with pitest that is generating false positives in cases of logging messages. It is easier to clarify it with an example. Let's say I have the following code:

public class MyClass {
  public void myBusinessMethod() {
    // some lines of complicated business thing
    logIntermediateResult(value)
    // some more lines of complicated business thing
    logFinalResult(result);
    return result;
  }
  public void logIntermediateResult(Object value) {
    log.info(
      append(value.bla()).and(
      append(value.other())),
      "some very complicated message that could span several lines"
    );
  }
  public void logFinalResult(Object value) {
    log.info(
      append(value.somethingElse()).and(
      append(value.evenMoreDetailed().and(
      append(value.detailsdetailsdetails())),
      "some very complicated message that could span several lines"
    );
  }

What is bugging me is that a mutation is generated for each of the calls to the logging methods, and it is clear that those mutations are not necessary. If, however, I inline the methods, the logging library avoidance defaults seem to kick in, and skip the mutations. The problem is that I'd rather not inline those methods; the logging lines itself are a bit verbose, and keeping them tucked away in methods is much cleaner.

How should I go about avoiding mutations to calls to specific methods? Here's what I already tried:

  • adding "log*" to the avoidCallsTo configurations (it seems to only takes packages)
  • adding excludedMethods to the configuration (it seems to only consider the inside of the method, not calls to it)
  • adding a @DoNotMutate (introduced in 1.2.2) annotation to the log* methods of my class (it seems to behave like excludedMethods

Nothing seems to work. Is this something that is not currently supported? Please advise.

@hollesse
Copy link

hollesse commented Dec 4, 2017

same issue here!

@rnveach
Copy link
Contributor

rnveach commented Mar 29, 2018

+1

There should be a an option to exclude a specific method call from mutations.
Pitest knows the name of the method and the class it is located in, so we should be able to create an expression to match against it.
removed call to com/my/company/ClassName::methodName → SURVIVED

@nicwaters96
Copy link

nicwaters96 commented Mar 26, 2019

+1

since moving from Java8 to Java11 I have a situation where I'm getting surviving mutations from lines not in the source, but in the bytecode. For example:

cats.stream().map(catAdapter::convertCatToDog).collect(Collectors.toList())

becomes

Stream var10000 = cats.stream(); CatAdapter var10001 = this.catAdapter; Objects.requireNonNull(var10001); return (List)var10000.map(var10001).collect(Collectors.toList());

this results in surviving mutations on 'Objects.requireNonNull'. Note- only an issue with method references, without the method reference the compiler doesn't add 'requiresNonNull'

It would be really useful to be able to use 'avoidCallsTo' specifically for that method. I obviously don't want to avoid calls to the whole of Objects.class.
actually am a little unsure that avoidCallsTo works other than at package level - when I added 'java.util.Objects' pitest seemed to avoid calls to the entire of java.util

(I think the issue would have originally become apparent with JDK9, my project just happens to have gone from 8-11)

@svalchinov
Copy link

@nicwaters96 hit the same issue as well, any workarounds or potential fixes?

@nicwaters96
Copy link

I haven't found a workaround - we just make engineers aware of mutations from bytecode being an issue when they're reviewing reports. It's a bit tedious because we use a Pitest plugin with Sonar and I'm always marking these as false positives:)

@svalchinov
Copy link

Thanks! For now I'll be avoiding method references when this occurs

@nicwaters96
Copy link

it's a bit of a no-win because one either avoids false positive mutations or avoids the IDE highlighting places where a method reference should/could be used:)

@svalchinov
Copy link

I agree, it's a hacky workaround, the other alternative is ignoring calls to java.util.Objects or removing that particular mutator completely.

@hcoles
Copy link
Owner

hcoles commented Aug 29, 2019

@nicwaters96 @svalchinov Not had chance to look at the more general issue of filtering specific calls, but the next release will filter out mutations to calls to Objects.requireNonNull added by the compiler for method references.

@nicwaters96
Copy link

@hcoles great! I'll upgrade asap. it'll save me some time not marking 'requireNonNull's survivals as false positives:)
thanks for the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants