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

pitest: increase mutation coverage for pitest-checks-metrics profile to 100% #5005

Closed
Nimfadora opened this Issue Aug 25, 2017 · 10 comments

Comments

Projects
3 participants
@Nimfadora
Contributor

Nimfadora commented Aug 25, 2017

We should increase coverage for pitest-checks-metrics profile up to 100%.
This issue is a subtask of #3708

Current threshold of pitest-checks-metrics profile: 91

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 22, 2017

Member

from @Nimfadora:

I have tried to create test for multiple file execution on the same config, but it didn't kill any mutations in NPathComplexityCheck

Member

romani commented Sep 22, 2017

from @Nimfadora:

I have tried to create test for multiple file execution on the same config, but it didn't kill any mutations in NPathComplexityCheck

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 23, 2017

Member

@Nimfadora , please share with me link to branch where you tried this multifile execution. All that internal variables cleanup should affect execution, it might be matter of set of input files.
Please remove such lines and launch diff report, smth will be found.

Member

romani commented Sep 23, 2017

@Nimfadora , please share with me link to branch where you tried this multifile execution. All that internal variables cleanup should affect execution, it might be matter of set of input files.
Please remove such lines and launch diff report, smth will be found.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 8, 2017

Member

http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_170
http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_215
http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_254
All survivals are related to grabbing the package name of the current class. The reason why is we never use the package name to it's full extent.

referencedClassNames.remove(parentContext.getPackageName() + DOT + className);

referencedClassNames always contains the class name, not the fully qualified package.
Even more weird, isFromExcludedPackage doesn't even use the getPackageName. It gets it's package information from candidateClassName which is completely separate.

@romani Unless you know how to fix this code, I think we should remove all this code related to the package as it does nothing.

Member

rnveach commented Dec 8, 2017

http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_170
http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_215
http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/AbstractClassCouplingCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@37ddd371_254
All survivals are related to grabbing the package name of the current class. The reason why is we never use the package name to it's full extent.

referencedClassNames.remove(parentContext.getPackageName() + DOT + className);

referencedClassNames always contains the class name, not the fully qualified package.
Even more weird, isFromExcludedPackage doesn't even use the getPackageName. It gets it's package information from candidateClassName which is completely separate.

@romani Unless you know how to fix this code, I think we should remove all this code related to the package as it does nothing.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 8, 2017

Member

http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/NPathComplexityCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@36a92cfb_436
Report is hard to understand, but the conditional boundary surviving is columnNo > endColumnNo and I have problems getting it just like in coding package.

Member

rnveach commented Dec 8, 2017

http://rveach.no-ip.org/checkstyle/regression/pitest-reports/6/com.puppycrawl.tools.checkstyle.checks.metrics/NPathComplexityCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@36a92cfb_436
Report is hard to understand, but the conditional boundary surviving is columnNo > endColumnNo and I have problems getting it just like in coding package.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 8, 2017

Member

@rnveach ,

Unless you know how to fix this code

Pitest is catching dead and unnecessary code. Refactor it as you think it should be, launch regression and if no obvious problems I will be ok to merge. In worse case users will report problem and we will know exact test case, to make our code more robust.

Member

romani commented Dec 8, 2017

@rnveach ,

Unless you know how to fix this code

Pitest is catching dead and unnecessary code. Refactor it as you think it should be, launch regression and if no obvious problems I will be ok to merge. In worse case users will report problem and we will know exact test case, to make our code more robust.

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 8, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 8, 2017

Member

Will do in the next PR.

Member

rnveach commented Dec 8, 2017

Will do in the next PR.

romani added a commit that referenced this issue Dec 8, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 8, 2017

Member

columnNo > endColumnNo

This can't be removed because this test and line fails:


It's violation changes from Complexity is 6 to Complexity is 8.
lineNo == endLineNo can't be removed because a case shows so in regression. I will add it later.
I don't see how it is possible to kill this one. I think we need to use mocking/reflection here.

package name in AbstractClassCouplingCheck

Regression is showing this can't be completely removed. It is because referencedClassNames holds what is written in the class. So package name is only used if user defines the complete qualified path of the class as the type.

Member

rnveach commented Dec 8, 2017

columnNo > endColumnNo

This can't be removed because this test and line fails:


It's violation changes from Complexity is 6 to Complexity is 8.
lineNo == endLineNo can't be removed because a case shows so in regression. I will add it later.
I don't see how it is possible to kill this one. I think we need to use mocking/reflection here.

package name in AbstractClassCouplingCheck

Regression is showing this can't be completely removed. It is because referencedClassNames holds what is written in the class. So package name is only used if user defines the complete qualified path of the class as the type.

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 9, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 9, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 10, 2017

rnveach added a commit to rnveach/checkstyle that referenced this issue Dec 10, 2017

romani added a commit that referenced this issue Dec 10, 2017

@rnveach rnveach added this to the 8.6 milestone Dec 10, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 10, 2017

Member

100% has been met, issue is done.

Member

rnveach commented Dec 10, 2017

100% has been met, issue is done.

@rnveach rnveach closed this Dec 10, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

@romani romani moved this from To Do to Done in Practice What You Preach Mar 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment