Resolve Pitest Issues - CustomImportOrderCheck (4)

[rnveach]

Child issue of #7797 ,

checkstyle/.ci/pitest.sh

Line 80 in abf829f

"CustomImportOrderCheck.java.html:<td class='covered'><pre><span class='survived'> if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)) {</span></pre></td></tr>"

[malinthar]

I am on it

[malinthar]

@rnveach May I work on this. This issue has been mentioned by @wltan to @Abhishek-kumar09

[rnveach]

It is possible 2 separate pitest failures could be killed with the same UT if one is found. Nothing has been provided in the other issue, so it is hard to tell if this is the case. If you are worried, there are pitests related to IllegalImportCheck that have not been touched that can be worked on.

[malinthar]

I understand. I will work on this. If the other issue is resolved with the same UT, I will focus on the one of the IllegalImportCheck pitests.Thanks

[malinthar]

@rnveach ,I have looked deep into this, modifying test cases won't help killing mutators as the two occurrences of the customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)) in CustomImportOrderCheck class compensate for the error created by each other. Moreover, I tried modifying the source code of CustomImportOrderCheck as well. But did not work out. I am trying few other methods.However, I am open to any suggestions.

[rnveach]

@Malintha1996 Please follow steps in #7797 (comment) , 1 by 1 and post them here as you complete them. This will help guide you in this issue and help me identify where you are stuck.

I also recommend looking at how other students completed this, like #7799 .

[malinthar]

pitest report

https://malintha1996.github.io/checkstyle-tester-reports/pit-reports/202003181856/com.puppycrawl.tools.checkstyle.checks.imports/CustomImportOrderCheck.java.html

Surviving mutations

• line 528 : removed conditional - replaced equality check with true → SURVIVED
  if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP))
• line 765: removed conditional - replaced equality check with true → SURVIVED
  if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP))
• line 773: removed conditional - replaced equality check with true → SURVIVED
  if (bestMatch.group.equals(NON_GROUP_RULE_GROUP))
  I am focusing on line 528 , however, I found that killing mutation in 528 also kills the surviving mutation in 765 which is focused by Resolve Pitest Issues - CustomImportOrderCheck (2) #7803.
  The line 773 is focused by Resolve Pitest Issues - CustomImportOrderCheck (3) #7804

As #7803 is taken, what shoud I do. May I focus on another issue (IllegalImportChek) or keep solving this.

Hardcoded mutation:

mutation branch - malinthar@22e5990

Regression report

https://malintha1996.github.io/checkstyle-tester-reports/fix-mutation-7805/diff/index.html

[rnveach]

@Malintha1996 The other issue hasn't provided any details and there is no guarantee they will work or finish the issue. It has been 3 days without an update.

Please continue with this issue. If PR is started and shows multiple mutations can be solved, then that person will be allowed to complete.

[malinthar]

@rnveach Diff report does not suggest any UT that would kill the surviving mutation. The diff report was generated for all projects with 7 distinct configurations.
My suggestion is to remove the condition.
When removed, the surviving mutation on line 773 will be killed as well.

Code analysis

public void visitToken(DetailAST ast) {
        if (ast.getType() == TokenTypes.PACKAGE_DEF) {
            if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)) {
                samePackageDomainsRegExp = createSamePackageRegexp(
                        samePackageMatchingDepth, ast);
            }
        } else if

if the condition is true it sets the value of samePackageDomainsRegExp which has only one use as below.

private String getImportGroup(boolean isStatic, String importPath) {
        RuleMatchForImport bestMatch = new RuleMatchForImport(NON_GROUP_RULE_GROUP, 0, 0);
        if (isStatic && customImportOrderRules.contains(STATIC_RULE_GROUP)) {
            bestMatch.group = STATIC_RULE_GROUP;
            bestMatch.matchLength = importPath.length();
        }
        else if (customImportOrderRules.contains(SAME_PACKAGE_RULE_GROUP)) {
            final String importPathTrimmedToSamePackageDepth =
                    getFirstDomainsFromIdent(samePackageMatchingDepth, importPath);
            if (samePackageDomainsRegExp.equals(importPathTrimmedToSamePackageDepth)) {
                bestMatch.group = SAME_PACKAGE_RULE_GROUP;
                bestMatch.matchLength = importPath.length();
            }
        }

The attribute samePackageDomainsRegExp will not be used if the SAME_PACKAGE_RULE_GROUP() is not included in the CustomImportOrderRules . Therefore, removing the condition on line 528 would do no haram.

[rnveach]

The pitest report, branch and regression look good to me. i would avoid talking about mutations you are not doing except for mentioning killing 1 may kill another.

This code is a bit more complex, so I may need more information.

Are we basically caching the samePackageDomainsRegExp for use later and all its uses are wrapped around another similar SAME_PACKAGE_RULE_GROUP check? So we don't have to worry about it being used inappropriately anywhere else and that is why it is completely safe to remove it? There is no way we will get an NPE or use it under the wrong circumstance?

[malinthar]

Are we basically caching the samePackageDomainsRegExp for use later and all its uses are wrapped around another similar SAME_PACKAGE_RULE_GROUP check? So we don't have to worry about it being used inappropriately anywhere else and that is why it is completely safe to remove it? There is no way we will get an NPE or use it under the wrong circumstance?

Yes. it is used under a simillar SAME_PACKAGE_RULE_GROUP check. And only used in that check. There won't be a NPE or any other exception related to samePackageDomainsRegExp . Moreover, it is initially assigned an empty string. So no possibility for a NPE.

[rnveach]

Then I agree with you the code needs to be changed and there is no other way to kill the mutation.

Please start a PR.

[malinthar]

Then I agree with you the code needs to be changed and there is no other way to kill the mutation.

Please start a PR.

done

[strkkk]

Fix is merged