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

Issue #7877: Removed redundant conditional #9307

Merged
merged 1 commit into from Feb 21, 2021

Conversation

bossever
Copy link
Contributor

@bossever bossever commented Feb 18, 2021

Fixes #7877

Pitest Report before fix: https://bossever.github.io/issue7877/pit-reports/202102122217/com.puppycrawl.tools.checkstyle.checks.imports/ImportOrderCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@476dda8b_878

Hardcoded mutation on line 878: removed conditional - replaced equality check with true → SURVIVED

Pitest Report after fix: https://bossever.github.io/issue7877/pit-reports/202102182308/com.puppycrawl.tools.checkstyle.checks.imports/index.html

Rationale behind removing !beforeFirstImport

I tried coming up with test cases where the mutation could be killed, but came to the conclusion that there exists no such test case, please read comment on issue - #7877 (comment).
As @nmancus1 has pointed out in issue conversation, I suppose this condition can be safely removed if no test cases were found, and mvn clean verify is passing.
Please let me know if I have missed anything, or if there is another way to fix this issue.


Diff Regression config: https://gist.githubusercontent.com/bossever/ff126a13b0f1422daa5626db6fcdc384/raw/96d1898cdc28233c7a7fef6b3f855b939b2cf694/config.xml

Diff Regression projects: https://gist.githubusercontent.com/bossever/ff126a13b0f1422daa5626db6fcdc384/raw/6e8f8cbf0eb40baaf072ec9a3fe93883614709a9/projects-to-test-on.properties

Report label: Diff Regression Report after fix

@bossever
Copy link
Contributor Author

GitHub, generate report

@github-actions
Copy link
Contributor

Copy link
Member

@nrmancuso nrmancuso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make a test case for this condition, I think we would need to report a violation for an internal separation of imports before the first import (while processing the first import, really). I also don't think it makes sense to make some impossible test case(via reflection, etc.) to kill this mutation.

@pbludov pbludov requested a review from strkkk February 21, 2021 06:23
@pbludov pbludov assigned strkkk and unassigned pbludov Feb 21, 2021
@strkkk strkkk merged commit de37ff1 into checkstyle:master Feb 21, 2021
@bossever bossever deleted the pitestIssue6 branch February 21, 2021 15:11
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

Successfully merging this pull request may close these issues.

Resolve Pitest Issues - ImportOrderCheck (6)
4 participants