pitest icon indicating copy to clipboard operation
pitest copied to clipboard

suggestion : Ignore the generated code

Open sarahBuisson opened this issue 7 years ago • 30 comments

I use lombock, it seems your code coverage run also on the code who is generated by lombok. It should'nt.

sarahBuisson avatar Apr 25 '17 09:04 sarahBuisson

I agree that it would be good to avoid mutating auto generated code, but as far as I'm aware Lombok annotations have source retention. This means there is no easy way for pitest to detect that the bytecode code is autogenerated.

hcoles avatar Apr 25 '17 09:04 hcoles

I see than when a line is generated by an anotation, pitest place the line coverage on the anotation on the report file view. Maybe we can add a list of annotation for which the coverage is not count?

sarahBuisson avatar Apr 25 '17 15:04 sarahBuisson

@sarahBuisson The problem is that pitest has no way of knowing that the bytecode is generated by lombok - the annotations are not visible in the bytecode as they have source retention.

hcoles avatar Apr 25 '17 15:04 hcoles

Well, JaCoCo made it possible a few weeks back. Maybe something can be salvaged from that? https://github.com/jacoco/jacoco/pull/513

kernle32dll avatar May 12 '17 16:05 kernle32dll

@Kernle32DLL thanks for pointing me at that. Although the lombok annotations used to control generation still have source retention, at some point in the last year Lombok has started adding its own Generated annotation with class retention to the generated methods.

This should make it fairly simple to filter them out.

hcoles avatar May 12 '17 18:05 hcoles

I've just pushed a simple change that will prevent mutation of any method annotated with a annotation matching one of three hard coded names

  • Generated
  • DoNotMutate
  • CoverageIgnore

This will only work if the annotations have at least class level retention.

I've not had chance to try this against Lombok yet, but if it is indeed adding a Generated annotation this should prevent mutation of that code.

At the moment line coverage will still be calculated for the annotated methods.

hcoles avatar May 12 '17 21:05 hcoles

@hcoles : Gave it a quick spin, looks good to me. The number of mutations decreased from 263 to 111. It is worth pointing out that to test this feature, a lombok config must be present (https://github.com/jacoco/jacoco/pull/495#issuecomment-291162737). This is apparently not documented at the moment (https://projectlombok.org/features/configuration.html). Mine reads as:

config.stopBubbling = true
lombok.addLombokGeneratedAnnotation = true

kernle32dll avatar May 16 '17 12:05 kernle32dll

@Kernle32DLL Thanks.

Pitest not is yet checking for the annotation at the class level so, depending on what lombok is doing, some autogenerated code may still be mutated. I'll add that in when I get chance.

hcoles avatar May 16 '17 12:05 hcoles

Hello, I've been working on it, and I've found a solution to this issue. (I search the presence of the annotation in the java file). I've forked your project, implement the solution ( who work !) but I've an issue with the integration tests, (PitMojoIT.java). the project (pit-lombock) run and generate pit-test report normaly when it's launched alone, but doesn't seems to run when it launched by the PitMojoIT Test. Can you help me to debug this?

I've fork your project here ( branch 'annotation'): https://github.com/sarahBuisson/pitest/tree/annotation

Thank-you very much !

sarahBuisson avatar Jun 06 '17 11:06 sarahBuisson

@sarahBuisson

Thanks for your work on this, however parsing the source would represent a major change for pitest and would not be something I can merge in.

It should however be possible to implement this as a mutation filter plugin

http://pitest.org/quickstart/advanced/

This would allow people to opt in to the functionality without adding the overhead for other users.

I think the reason that the integration test is failing is probably that the pit-lombok project been placed in the wrong location - it needs to be in the src/test/resources folder of the verification project.

hcoles avatar Jun 06 '17 15:06 hcoles

I can't do this just with the informations contained in the MutationDetails, I need the data who are contained in the classInfo.

If I do a mutationFilterFactory and add the classInfo into the MutationDetails, will you accept the pullRequest?

sarahBuisson avatar Jun 06 '17 22:06 sarahBuisson

@sarahBuisson

You should be able to implement the filter without using classinfo. If you parse the source file and identify methods/classes with relevant annotations you can then remove any mutations in these location from the list within the filter.

If you implement in this way you will not need to create a pull request - the plugin will be separate from the main pitest codebase and you will have complete control over it.

hcoles avatar Jun 07 '17 08:06 hcoles

@hcoles I need classInfo to find the sourceFile. currently MutationDetails class contains just the name of the file, not the path to this file.

sarahBuisson avatar Jun 07 '17 15:06 sarahBuisson

@sarahBuisson

Both MutationDetails and ClassInfo have only the name of the file. Paths to source files are only resolved when the generating the html report.

Your code looks to be currently assuming that the file will always live in src/main/java. This will be true for (most) maven projects, but the convention may not be followed in codebases built with other tools.

hcoles avatar Jun 08 '17 09:06 hcoles

I am using Lombok 1.16.14 still poor coverage due to class level @Data annotation. Even I have covered all getter and setters in my test cases and in coverage details also shows everything green except @Data in red but coverage is missing for some unknown Lombok default generated code. My Maven dependency for Lombok (I am using Latest Jacoco in my eclipse - downloaded 06/06/2017 ) <groupId>org.projectlombok</groupId> <artifactId>lombok</artifactId> 1.16.14

gunjank avatar Jun 09 '17 14:06 gunjank

@hcoles Hello everyone, I'm back ! The good new : I've create a mutation filter plugin to ignore the generated code line

The bad new n° 1 : I'll need to add the sourceDirs to the CodeSource class

( in order to detect the annotation, I still need to access the source. I choose to use the same way your plugin use to generate the report. This should not have any impact on anythings.)

If you want to check the code before I do an official Pull request, it's here https://github.com/sarahBuisson/pitest/tree/addFileLocatorToMutationFilter

The bad new n°2: It's seem to have a bug in the registration plugin : my Meta Inf file is not used, it's alway the file in the pitest project who is used. My code who produce the bug: https://github.com/sarahBuisson/pitest/tree/pluginGeneratedByAnnotationLineFilter

sarahBuisson avatar Jun 12 '17 23:06 sarahBuisson

Hi @sarahBuisson

Thanks for the update. I'd prefer not to add sourceDirs to CodeSource, but there are two other easy solutions.

a. Instead of implementing MutationFilter implement MutationInterceptor.

This has only just been added in the latest snapshot - it's very similar to a filter but (among other differences) its factory method receives ReportOptions object from which the source dirs can be retrieved.

b. Change the MutationFilterFactory so that it also receives ReportOptions instead of its current maxMutationsPerClass parameter.

I don't quite follow what the issue is on point 2.

There is no need to fork pitest to implement your plugin. Part of the point of implementing this as a plugin is to allow the functionality to be optionally included without affecting the main codebase.

Some examples of producing stand alone plugins are here

https://github.com/hcoles/pitest-plugins

Be aware that this repo hasn't been updated for a while so may be out of sync due to changes in recent current versions of pitest.

hcoles avatar Jun 13 '17 14:06 hcoles

Hi @hcoles and @sarahBuisson First, thanks for all the great work you are doing on pitest. Was there any conclusion about how to indicate to pitest to exclude the lombok annotated classes/methods from mutation analysis? Lombok introduced the @Generated annotation, but reading this log, I am not clear if the solution is available. Please advise.

mgoldver avatar Jul 06 '18 02:07 mgoldver

Hi @hcoles, just checking to find out if any work is planned on this. We are starting to look at pitest in conjunction with SonarQube and we do use lombok in our code. Right now it's definitely reporting on the class-level lombok annotations (@Data)... Thanks!

innanill avatar Apr 03 '19 22:04 innanill

@innanill have you configured lombok to add Generated annotations? @kernle32dll described how to set it up earlier in the thread.

hcoles avatar Apr 04 '19 10:04 hcoles

@hcoles, Yes. My lombok.config looks exactly as he has it.

innanill avatar Apr 04 '19 21:04 innanill

This should not be coupled to any framework or any specific annotation, it's the wrong approach!

Let the user provide Annotation classes to ignore in their gradle config

i.e.

pitest {
    excludeAnnotations = [MyCustomCoverageIgnore.class, .... ]
}

alexmnyc avatar Nov 19 '19 21:11 alexmnyc

@alexmnyc 's suggesion is appealing. Any progress? Like others, my mutation line coverage is low owing to Lombok-generated code. My lombok.config includes:

lombok.addLombokGeneratedAnnotation = true

I found these release notes: https://github.com/hcoles/pitest/releases/tag/pitest-parent-1.2.1. There isn't documentation I could find for "features" or the "FANN".

I did eventually work out that <features><value>+FANN(...)</value></features> goes into the execution configuration, not the plugin configuration, but trying it, my mutation score dropped, a surprise.

Am I doing it wrong?

boxleytw avatar Oct 06 '20 14:10 boxleytw

@alexmnyc @hcoles @kernle32dll Any update on this issue? I am also facing this, I have the lombok.config configured with these options:

config.stopBubbling = true
lombok.addLombokGeneratedAnnotation = true

kamesh95 avatar Jul 06 '21 16:07 kamesh95

If you are using a recent version of pitest, and lombok is adding the annotation, pitest should not be mutating any classes/methods where the annotation had been added.

If this is not working for you, create a minimal (but complete) project that reproduces the issue and I can take a look.

hcoles avatar Jul 06 '21 16:07 hcoles

@hcoles Added an example reproducing this issue. Also committed the generated pitest report in the root folder of the repo. Repo - https://github.com/kamesh95/openapi-springboot-sample Run below commands after cloning the repo:

./gradlew build
./gradlew test
./gradlew pitest

kamesh95 avatar Jul 07 '21 05:07 kamesh95

@kamesh95 thanks, which of the mutants do you think are within generated code?

hcoles avatar Jul 07 '21 07:07 hcoles

@hcoles If you look at the attached image, the packages inside the box are not present in the source code and are generated through opengenerator config, but these packages are still coming here in mutation report. These packages are used in opengenerator config:

// Builds a spring client by default.
openApiGenerate {
    apiPackage = 'org.openapitools.api'
    invokerPackage = 'org.openapitools.invoker'
    modelPackage = 'org.openapitools.model'
Screenshot 2021-07-07 at 3 07 21 PM

kamesh95 avatar Jul 07 '21 09:07 kamesh95

Those packages are not generated by lombok, so will not contain the lombok generated annotation. Mutations in the lombok generated code are being correctly suppressed.

The classes in the openapi packages have been annotated with javax.annotation.Generated, but this has only source retention so is invisible to pitest.

If you wish these classes to be ignored, whatever generates them will need to be updated to add an annotation with at least class level retention. Alternately, the package can be excluded by passing "org.openapitools.*" to excludedClasses.

hcoles avatar Jul 07 '21 09:07 hcoles

Oh okay, thanks @hcoles Yeah, I am now excluding classes based on this pattern.

kamesh95 avatar Jul 07 '21 11:07 kamesh95