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-misc profile to 100% #4675

Closed
romani opened this Issue Jul 9, 2017 · 11 comments

Comments

Projects
3 participants
@romani
Member

romani commented Jul 9, 2017

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

Current threshold of pitest-checks-misc profile: XX

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 9, 2017

Member

@Nimfadora , FYI, missed set of classes. I created new profile.

Member

romani commented Jul 9, 2017

@Nimfadora , FYI, missed set of classes. I created new profile.

romani added a commit that referenced this issue Jul 9, 2017

romani added a commit that referenced this issue Jul 9, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 20, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 21, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 21, 2017

romani added a commit that referenced this issue Aug 21, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 22, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Aug 22, 2017

romani added a commit that referenced this issue Aug 23, 2017

ArneLimburg pushed a commit to ArneLimburg/checkstyle that referenced this issue Aug 23, 2017

ArneLimburg pushed a commit to ArneLimburg/checkstyle that referenced this issue Aug 23, 2017

ArneLimburg pushed a commit to ArneLimburg/checkstyle that referenced this issue Aug 23, 2017

ArneLimburg pushed a commit to ArneLimburg/checkstyle that referenced this issue Aug 23, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 1, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 1, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 3, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 3, 2017

Nimfadora pushed a commit to Nimfadora/checkstyle that referenced this issue Sep 3, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 10, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 11, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 11, 2017

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Sep 11, 2017

romani added a commit that referenced this issue Sep 21, 2017

@romani romani added this to the 8.3 milestone Sep 21, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Sep 21, 2017

Member

fix is merged

Member

romani commented Sep 21, 2017

fix is merged

@romani romani closed this Sep 21, 2017

@romani romani reopened this Oct 7, 2017

romani added a commit that referenced this issue Oct 7, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Oct 7, 2017

Member

problematic test is reverted to unlock release process. See issue #5141
https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java
Now uncovered mutations are:
image

image

Member

romani commented Oct 7, 2017

problematic test is reverted to unlock release process. See issue #5141
https://github.com/checkstyle/checkstyle/blob/master/src/main/java/com/puppycrawl/tools/checkstyle/checks/TranslationCheck.java
Now uncovered mutations are:
image

image

romani added a commit that referenced this issue Oct 7, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 10, 2017

Member

Latest report: http://rveach.no-ip.org/checkstyle/regression/pitest-reports/13/

Report shows TrailingCommentCheck is not at 100% code coverage, and TranslationCheck is missing mutation coverage.

Member

rnveach commented Dec 10, 2017

Latest report: http://rveach.no-ip.org/checkstyle/regression/pitest-reports/13/

Report shows TrailingCommentCheck is not at 100% code coverage, and TranslationCheck is missing mutation coverage.

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 10, 2017

Member

@romani Missing mutation coverage for TranslationCheck that was reverted might be brought back if Issue #5141 is fixed. It has a PR that has been sitting.

Member

rnveach commented Dec 10, 2017

@romani Missing mutation coverage for TranslationCheck that was reverted might be brought back if Issue #5141 is fixed. It has a PR that has been sitting.

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

romani added a commit that referenced this issue Dec 12, 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

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 25, 2017

Member

the only missed coverage we have is
image
removed call to com/puppycrawl/tools/checkstyle/api/MessageDispatcher::fireFileFinished → SURVIVED

@Nimfadora , please help us to finish this issue.

Member

romani commented Dec 25, 2017

the only missed coverage we have is
image
removed call to com/puppycrawl/tools/checkstyle/api/MessageDispatcher::fireFileFinished → SURVIVED

@Nimfadora , please help us to finish this issue.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Dec 26, 2017

Contributor

@romani, ok, I will do it today.

Contributor

Nimfadora commented Dec 26, 2017

@romani, ok, I will do it today.

@Nimfadora

This comment has been minimized.

Show comment
Hide comment
@Nimfadora

Nimfadora Dec 27, 2017

Contributor

@romani xml output varies only in order of lines when I comment out line with missed coverage. And comment in testLogOutput says:

            // order is not always maintained here for an unknown reason.
            // File names can appear in different orders depending on the OS and VM.
            // This ensures we pick up the correct file based on its name and the
            // number of children it has.

So, I don't really understand how to test it, if it influences only order, which we can't check by known reasons.

XmlLogger#fileMessages are rewritten two times, when we call XmlLogger#fileFinished() and XmlLogger#auditFinished() (called from Checker#process). This causes situation when selected line doesn't really change anything special, just rewrites messages in different order.

Contributor

Nimfadora commented Dec 27, 2017

@romani xml output varies only in order of lines when I comment out line with missed coverage. And comment in testLogOutput says:

            // order is not always maintained here for an unknown reason.
            // File names can appear in different orders depending on the OS and VM.
            // This ensures we pick up the correct file based on its name and the
            // number of children it has.

So, I don't really understand how to test it, if it influences only order, which we can't check by known reasons.

XmlLogger#fileMessages are rewritten two times, when we call XmlLogger#fileFinished() and XmlLogger#auditFinished() (called from Checker#process). This causes situation when selected line doesn't really change anything special, just rewrites messages in different order.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 27, 2017

Member

@Nimfadora , if line of code does not affect anything, and generated xml report is still correct (all violations reported on proper files, no false-postives).
Such line should be removed from code completely - please do.

Member

romani commented Dec 27, 2017

@Nimfadora , if line of code does not affect anything, and generated xml report is still correct (all violations reported on proper files, no false-postives).
Such line should be removed from code completely - please do.

Nimfadora added a commit to Nimfadora/checkstyle that referenced this issue Dec 27, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Dec 27, 2017

Member

@romani
Please look over #5057 in terms of this fire call.
Just removing this line will have weird affects on any listener as we are sending violations without triggering a file has been started or a previous file is done.
Other issue might be the reason this one is so hard to kill.

See

public void fileFinished(AuditEvent event) {

Probably why this isn't the case for this is because auditFinished finishes the job, but it shouldn't have to worry about it as fileFinished should be handling it.

Edit: I can confirm that fixing that issue will make pitest 100%.

Member

rnveach commented Dec 27, 2017

@romani
Please look over #5057 in terms of this fire call.
Just removing this line will have weird affects on any listener as we are sending violations without triggering a file has been started or a previous file is done.
Other issue might be the reason this one is so hard to kill.

See

public void fileFinished(AuditEvent event) {

Probably why this isn't the case for this is because auditFinished finishes the job, but it shouldn't have to worry about it as fileFinished should be handling it.

Edit: I can confirm that fixing that issue will make pitest 100%.

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

@romani romani modified the milestones: 8.3, 8.6 Dec 27, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Dec 27, 2017

Member

Final fix is merged

Member

romani commented Dec 27, 2017

Final fix is merged

@romani romani closed this Dec 27, 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