minor: add missing test case to SuperCloneCheckTest

[pbludov]

Discovered here

JFYI: the Pitest can not appy BooleanFalseReturnValsMutator to code like return a > b.
After extracting a local variable,

final boolean result = a > b;
return result;

the mutator BooleanFalseReturnValsMutator is able to add the mutation and the missing coverage is detected.

This PR adds the missing case.

[rnveach]

@pbludov please ensure you are on the latest master for the PR. Failures look related to that.

I assume pitest is not failing in master? So why isn't this fix as part of the other PR since that is where problem is?

[pbludov]

@pbludov please ensure you are on the latest master for the PR. Failures look related to that.

done

I assume pitest is not failing in master? So why isn't this fix as part of the other PR since that is where problem is?

This is not a part of #7487
This issue is blocker for #7487, but it is not directly related.

[rnveach]

If pitest is not failing master then how is it a blocker for another PR but not related to that PR?

I see in other PR AbstractSuperCheck failed in pitest from your changes.

Pitest can not appy BooleanFalseReturnValsMutator to code like return a > b.

It doesn't look like its an issue with the mutator but maybe with the compiler, as pitest deals with the byte code and it can vary from compiler to compiler.

http://rveach.no-ip.org/checkstyle/regression/pitest-reports/36/pitest-coding/com.puppycrawl.tools.checkstyle.checks.coding/AbstractSuperCheck.java.html#org.pitest.mutationtest.report.html.SourceFile@4038323f_139
An old report doesn't have it doing the mutation and the line has 4 mutations. This report was run on linux.

I just re-ran the report on Windows 7 and the line has 6 mutations and does not reach 100%.

1. replaced boolean return with false for com/puppycrawl/tools/checkstyle/checks/coding/AbstractSuperCheck::hasArguments → SURVIVED
2. replaced return of integer sized value with (x == 0 ? 1 : 0) → SURVIVED

The whole package has other areas that are not reaching 100%. AbstractSuperCheck, DeclarationOrderCheck, MagicNumberCheck are the checks in question.

Windows run total mutation count for package:

1909/1916

Linux run total mutation count for package:

1796/1796

Windows run added almost 113 more mutations than linux, and 7 are failing to be killed. List of active mutators did not change between runs. List is on the bottom of each class. Other classes missing coverage are not the same mutators, so no visible direct relation to each class.

I recommend we report the issue to pitest.

@romani ping

[romani]

@pbludov , as this minor become so controversial ..... please put all details in PR description and use prefix in commit Pull #7494: ... .
You can open issue on pitest.

[rnveach]

I am no longer able to reproduce the issue on my Windows. I thought I had it, but its not happening now and I don't know what I did to produce it the first time. I am thinking somehow eclipse/maven went through enough recompiles to change the byte code significantly.

[pbludov]

I am no longer able to reproduce the issue on my Windows. I thought I had it, but its not happening now and I don't know what I did to produce it the first time. I am thinking somehow eclipse/maven went through enough recompiles to change the byte code significantly.

I can confirm that this issue is fluky. I'll try to extract a small example to report it to the Pitest team.

[romani]

@rnveach, please finalize review, pitest issue should not block this PR

[pbludov]

I think i know what is going on.

For the Java code (new version)

    private static boolean hasArguments(DetailAST methodCallDotAst) {
        final DetailAST argumentsList = methodCallDotAst.getNextSibling();
        return argumentsList.hasChildren();
    }

the bytecode is

  private static hasArguments(Lcom/puppycrawl/tools/checkstyle/api/DetailAST;)Z
   L0
    LINENUMBER 138 L0
    ALOAD 0
    INVOKEINTERFACE com/puppycrawl/tools/checkstyle/api/DetailAST.getNextSibling ()Lcom/puppycrawl/tools/checkstyle/api/DetailAST; (itf)
    ASTORE 1
   L1
    LINENUMBER 139 L1
    ALOAD 1
    INVOKEINTERFACE com/puppycrawl/tools/checkstyle/api/DetailAST.hasChildren ()Z (itf)
    IRETURN

For the Java code (original version)

    private static boolean hasArguments(DetailAST methodCallDotAst) {
        final DetailAST argumentsList = methodCallDotAst.getNextSibling();
        return argumentsList.getChildCount() > 0;
    } 

the bytecode is

  private static hasArguments(Lcom/puppycrawl/tools/checkstyle/api/DetailAST;)Z
   L0
    LINENUMBER 138 L0
    ALOAD 0
    INVOKEINTERFACE com/puppycrawl/tools/checkstyle/api/DetailAST.getNextSibling ()Lcom/puppycrawl/tools/checkstyle/api/DetailAST; (itf)
    ASTORE 1
   L1
    LINENUMBER 139 L1
    ALOAD 1
    INVOKEINTERFACE com/puppycrawl/tools/checkstyle/api/DetailAST.getChildCount ()I (itf)
    IFLE L2
    ICONST_1
    GOTO L3
   L2
   FRAME APPEND [com/puppycrawl/tools/checkstyle/api/DetailAST]
    ICONST_0
   L3
   FRAME SAME1 I
    IRETURN

The mutator BooleanFalseReturnValsMutator changes IRETURN with ICONST_0; IRETURN

Then there comes the filter FRETEQUIV which filters return vals mutants with bytecode equivalent to the unmutated class. As you see, the original bytecode for the original hasArguments method already ends with ICONST_0; IRETURN. As a result, the mutation is removed.

This is a bug in the Pitest library. Once my example will be ready to discuss, I'll start an issue on Pitest tracker.