Issue #12282: Resolve Pitest suppression for ParenPad

[Kevin222004]

Issue #12282: Resolve Pitest suppression for ParenPad

Diff Regression config: https://gist.githubusercontent.com/Kevin222004/e206f8b15b0d8c04e1d6b1fbded90dbe/raw/6b6a30c0a864caad500abed316e2e7a51d99c59b/config.xml

[Kevin222004]

Github, generate report

[romani]

Tests are failing, no need for report.
Sorry ch mutation should not be survival.
Did you try to remove others? May be we misunderstood indexation of cases PR may be ... Please do several more removals of others case ( but remove only one at the time)

[github-actions]

Report generation job failed on phase "make_report", step "Generate report".
Link: https://github.com/checkstyle/checkstyle/actions/runs/3294877363

[rnveach]

Suppressions:

checkstyle/.ci/pitest-suppressions/pitest-whitespace-suppressions.xml

Lines 93 to 109 in fa68569

	<mutation unstable="false">
	<sourceFile>ParenPadCheck.java</sourceFile>
	<mutatedClass>com.puppycrawl.tools.checkstyle.checks.whitespace.ParenPadCheck</mutatedClass>
	<mutatedMethod>visitToken</mutatedMethod>
	<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.RemoveSwitchMutator_3</mutator>
	<description>RemoveSwitch 3 mutation</description>
	<lineContent>switch (ast.getType()) {</lineContent>
	</mutation>
	<mutation unstable="false">
	<sourceFile>ParenPadCheck.java</sourceFile>
	<mutatedClass>com.puppycrawl.tools.checkstyle.checks.whitespace.ParenPadCheck</mutatedClass>
	<mutatedMethod>visitToken</mutatedMethod>
	<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.RemoveSwitchMutator_7</mutator>
	<description>RemoveSwitch 7 mutation</description>
	<lineContent>switch (ast.getType()) {</lineContent>
	</mutation>

There are 11 cases, 5 groups. We are looking for a 3 and a 7 (0 index).

case 0 fails tests.
case 1 fails tests.
case 2 fails tests.
case 3 fails tests.
case 4 fails tests.
case 5 fails just 1 test.
case 6 (ENUM_CONSTANT_DEF) does NOT fail tests.
case 7 fails tests.
case 8 (LITERAL_SYNCHRONIZED) does NOT fail tests.
case 9 fails tests.
case 10 fails just 1 test.

My local bytecode:

    //    2    6:lookupswitch    11: default 156
    //                   27: 104 // 0
    //                   28: 124 // 1
    //                   59: 124 // 2
    //                   67: 140 // 3 <--
    //                   91: 132 // 4
    //                   109: 124 // 5
    //                   136: 140 // 6
    //                   155: 140 // 7 <--
    //                   159: 140 // 8
    //                   176: 148 // 9
    //                   181: 140 // 10

Looking at the table, you can identify that the bytecode switch mapping does not correlate to the source. 140 which is group 3, has 2 items separated from the pack. The ones we are looking for are in this 140 group. I don't know which each one represents, but this starts to line up with my findings for ENUM_CONSTANT_DEF and LITERAL_SYNCHRONIZED. There are 5 cases pointing to 140 and the tokens I pointed to that are not failing tests are in a group of 5.

67 and 155 actually represent token IDs. 67 = LITERAL_SYNCHRONIZED (case 3) and 155 = ENUM_CONSTANT_DEF (case 7).

So it is all confirmed.

Also to note, the cases are ordered ascending by token ID.

[rnveach]

@Kevin222004 I believe the issue is ordering. This does actually have a bytecode connection as the internals of the switch have a different order than the source. We will need to update our wiki with this information.

I believe the tokens you want to investigate are ENUM_CONSTANT_DEF and LITERAL_SYNCHRONIZED.

[rnveach]

Report generation job failed on phase "make_report", step "Generate report".

Error that failed report is: Caused by: java.lang.OutOfMemoryError: Java heap space.

Diff Regression config: https://gist.githubusercontent.com/Kevin222004/e206f8b15b0d8c04e1d6b1fbded90dbe/raw/6c6ca45c3c652fe259dc5844d874b1091554ae0b/config.xml

This may show that CI can't handle big loads and you may have to trim the projects or use your local with more memory. I normally have to give regression 4 gigs of RAM.

One thought is for tokens, I don't believe you need to separate out individual tokens. They all could have been set as all the acceptable tokens.

[romani]

Am I understand this correctly?

<description>RemoveSwitch 3 mutation

Means 3rd group of cases in switch have survavals. If there are few cases, some of them.

[rnveach]

Means 3rd group of cases in switch have survavals. If there are few cases, some of them.

No. It doesn't count by groups, only by case XX:. However order of cases is based on bytecode and not source. Bytecode order seems to be based on actual integer of the switch (sorted).

[rnveach]

Imagine you have source like:

case 3: // 1st case, 1st group
  ...; break;
case 1: // 2nd case, 2nd group
case 2: // 3rd case, 2nd group
  ...; break;
default: // default, 3rd group
  ...; break;

Pitest actually sees and counts:

default: // default
 functGroup3(); break; // group 3 logic
case 1: // 0th case
 functGroup2(); break; // group 2 logic
case 2: // 1st case
 functGroup2(); break; // group 2 logic
case 3: // 2nd case
 functGroup1(); break; // group 1 logic

Groups only matter for code being executed when case matches. Order of cases is based on how it lays them out, which seems to be by number, sorted.

[romani]

Thanks a lot, I placed note to https://github.com/checkstyle/checkstyle/wiki/How-to-Generate-and-Analyze-Pitest-Reports#experimental-switch-mutator
If you notice how to improve this page further please update it to let students get this quicker and let us help to explain same stuff again less.

[Kevin222004]

@rnveach thanks for the lots of information

67 and 155 actually represent token IDs. 67 = LITERAL_SYNCHRONIZED (case 3) and 155 = ENUM_CONSTANT_DEF (case 7).

can you please tell how you decided it that the int 67 is for LITERAL_SYNCHRONIZED and same for 155

---

Is it expected that if we make changes and test tare not failing the mutation will kill
I have done for the LITERAL_SYNCHRONIZED I change the label accordingly, tests are not failing but still mutation is not killed
it is look like here i also cant create any test because all the different cases are available in input file and if it is not for what i can create it

[rnveach]

can you please tell how you decided

I didn't decide, I determined. ANTLR is the one that decides on the numbering.

that the int 67 is for LITERAL_SYNCHRONIZED and same for 155

Either System.out the value of these tokens or you can look in JavaLanguageLexer.java once the file is generated.

Is it expected that if we make changes and test tare not failing the mutation will kill

If you apply the mutation, and tests are failing, then either the mutation should have been killed, or there is other factors at play (like caching).
If you apply the mutation, and tests are passing, then the mutation is surviving and we need to determine what we can do to kill it.

it is look like here i also cant create any test because all the different cases are available in input file and if it is not for what i can create it

Please do things in order.
Update the branch with only the mutation so we can make sure it is correct.
Once that is done, provide us regression. If regression finds differences, then confirm and trim them to an input.
If regression is clean, then tell us why in details you can't create an input.

[Kevin222004]

Github, generate report

[Kevin222004]

@rnveach all the unit test are passing but all the it testing are failing. I am looking for it that why it is happening

[rnveach]

Only src\test is used for pitest. Anything else, like Checkstyle or such that is failing can be ignored for the mutation only branch.

[github-actions]

https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/6aa50c2_2022192038/reports/diff/index.html

[romani]

Сcan you copy code from https://checkstyle-diff-reports.s3.us-east-2.amazonaws.com/6aa50c2_2022192038/reports/diff/java-design-patterns/index.html#A1 to our Inputs to recheck is pitest is become killed

[Kevin222004]

@romani I have created the test file as you asked but it is throwing all the time exception that AST is null. All the things are looking good, but i don't understand that, I am updating the branch

[nrmancuso]

but it is throwing all the time exception that AST is null.

@Kevin222004 Every case of NullPointerException (or any other failure in the check) is a new test case that kills your mutation :)

[Kevin222004]

@nrmancuso Yes, the issue has been resolved i will update the branch

[Kevin222004]

https://kevin222004.github.io/solvedParenPad/com.puppycrawl.tools.checkstyle.checks.whitespace/ParenPadCheck.java.html
Line:-250

[romani]

Items

[romani]

Items

[romani]

ok to merge