Resolve Pitest Issues - ImportOrderCheck (7)

[rnveach]

Child issue of #7797 ,

checkstyle/.ci/pitest.sh

Line 92 in abf829f

"ImportOrderCheck.java.html:<td class='covered'><pre><span class='survived'> staticImportSeparator = isStatic &#38;&#38; separated;</span></pre></td></tr>"

[aryaniiit002]

i would like to work on this.

[aryaniiit002]

i have observed something but not sure if i am correct;
so survived mutation here is
"ImportOrderCheck.java.html:<td class='covered'><pre><span class='survived'> staticImportSeparator = isStatic &#38;&#38; separated;</span></pre></td></tr>"
in ImportOrderCheck.java
method on which mutation surviving is this named needSeparator at line 692.

    /**
     * Checks whether import groups should be separated.
     * @param isStatic whether the token is static or not.
     * @return true if imports groups should be separated.
     */
    private boolean needSeparator(boolean isStatic) {
        final boolean typeImportSeparator = !isStatic && separated;
        final boolean staticImportSeparator;
        if (staticImportsApart) {
            staticImportSeparator = isStatic && separatedStaticGroups;
        }
        else {
            staticImportSeparator = isStatic && separated;
        }
        final boolean separatorBetween = isStatic != lastImportStatic
            && (separated || separatedStaticGroups) && staticImportsApart;

        return typeImportSeparator || staticImportSeparator || separatorBetween;
    }

An import could be either of type or static
if isStatic in above method is false then the result of the needSeparator will be true because there is no test for type imports using property separated with false value and default value of separated value is false so the whole method would results false. Also we need not to look at other variables because those are local variables.
so now, at line 692: isStatic is true and staticImportSeparator now depends on separated, so there is no need to have isStatic in the line.

Please have a look at these images
1st image is old pitest report

(at line 859)
new pitest report
with changed code line from
staticImportSeparator = isStatic && separated;
to
staticImportSeparator = separated;

I am not sure if i am right please have a look and let me know if there is something i missed or if this is the wrong approach.
Thanks

[strkkk]

I am not sure if i am right please have a look and let me know if there is something i missed or if this is the wrong approach.

There is either

1. Redundant condition
2. Lack of test case

You need to either add test case that will cover it, or prove that variable can be removed without breaking check logic.

[rnveach]

@aryaniiit002 Please read parent issue's instructions. All must be provided and will guide you on the best action for this pitest issue.
#7797

For each issue the following steps need to be taken:

[aryaniiit002]

You need to either add test case that will cover it, or prove that variable can be removed without breaking check logic.

i don't understand how test cases works exactly.
i was trying to prove removal of variable won't break the logic, so i test all the code example with configs we have here
Issue #7687: Adding code examples for ImportOrder check #9165 in ImportOrderCheck with both
staticImportSeparator = isStatic && separated;
and
staticImportSeparator = separated;
and i can't find any difference in the results.

[aryaniiit002]

Please respond somebody...

[strkkk]

@aryaniiit002 tests are passing if you remove this condition - this is exactly the problem in this issue. It is either lack of test to cover this, or this condition is redundant (requires checking of logic)
Have you read link from #7878 (comment)?

[aryaniiit002]

Yes i read the link and i am sure i don't understand few lines there and also i think i misunderstood checking of logic . But could you please confirm that you understand what i am trying to do?

checking of logic: i process this like checking/testing all java code example with provided configs for each one separately for both (with ifStatic and without it) and comparing the results.

[nrmancuso]

To get an idea of the work that needs to be done, see: #7985

Please follow all steps in #7797

Please read about pitest, and make sure that you understand exactly how it works, and why it is used.

[Abhishek-kumar09]

@aryaniiit002
As written in the PR description, you have to understand each variable of the function needSeperator.

See a mutation can be killed if you add a test case that fails when we change the code logic written in the mutation report. in your case (staticImportSeparator = isStatic && separated;).
So you can search or think of a case where the else part is executed and there is change in the test result when that else logic is modified with true/false values.
OR
It is possible that no test cases exist as such to kill the mutation, so you have to modify the code logic that is no more needed, like removing the full else part in your case or modifying the else part's logic in way it is not redundant.

You can always set a breakpoint on the part where your mutatioins are survived and take look at all the variables at that time how they are behaving with different test cases with the debugger.

[pbludov]

Fix is merged.