Issue #13109: Kill mutation for FullIdent.java-2 by Kevin222004 · Pull Request #13179 · checkstyle/checkstyle

Kevin222004 · 2023-06-07T18:48:36Z

Issue #13109: Kill mutation for FullIdent.java-2

Mutation covered

checkstyle/config/pitest-suppressions/pitest-api-suppressions.xml

Lines 48 to 55 in b8753a7

    
           <mutation unstable="false"> 
        
             <sourceFile>FullIdent.java</sourceFile> 
        
             <mutatedClass>com.puppycrawl.tools.checkstyle.api.FullIdent</mutatedClass> 
        
             <mutatedMethod>createFullIdentBelow</mutatedMethod> 
        
             <mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator> 
        
             <description>removed call to com/puppycrawl/tools/checkstyle/api/DetailAST::getFirstChild</description> 
        
             <lineContent>return createFullIdent(ast.getFirstChild());</lineContent> 
        
           </mutation>

Kevin222004 · 2023-06-08T04:34:16Z

Ci failure is not related to changes only test added

Kevin222004 · 2023-06-09T16:08:23Z

@romani This is ready fir review

romani · 2023-06-11T06:40:43Z

This class is mistake to be in API. Better to find way to cover it by Inputs.

Vyom-Yadav · 2023-06-12T10:21:56Z

Better to find way to cover it by Inputs.

Agreed. FullIdent is used by a lot of checks, used even in sevntu. You can find such a list here:

rdiachenko · 2023-06-16T18:03:08Z

@Kevin222004 could you resolve the conflict please and follow @Vyom-Yadav's suggestion to test it. Your current approach doesn't use any checks, you need a check which uses FullIdent class and input for that check to write a proper test.

Kevin222004 · 2023-06-18T14:26:39Z

@rdiachenko Please check i have updated branch

romani

All good except for minor:

pom.xml

romani

ok to merge

pom.xml

rdiachenko · 2023-06-19T18:19:45Z

src/main/resources/com/puppycrawl/tools/checkstyle/api/messages.properties

@@ -0,0 +1 @@
+import.ordering=Wrong order for ''{0}'' import.


Why do we include message properties specific to import.ordering only into main resources but not the test ones?

Do we need all the locals to do testing or it's just enough to use a single message properties to test the case?

I have just followed all the check, to throw the message in all the check it is on the resources of main, so I also put it in the resource section of main

import.ordering is only one which is require to throw the violation message at least for the test which I write so single message property is fine

But it's a copy paste of existing properties into a different directory under main (non test) resources. @romani why do we allow this?

Why do you need all those messages_XX.properties, shouldn't a single file be enough?

@rdiachenko for second question

[INFO] [INFO] --- maven-antrun-plugin:3.1.0:run (ant-phase-verify) @ checkstyle --- [INFO] Executing tasks [INFO] [echo] Checkstyle started (checkstyle-checks.xml): 22/06/2023 06:47:12 pm [INFO] [checkstyle] Running Checkstyle on 1626 files [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_de.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_fi.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_ru.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_pt.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_ja.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_fr.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_es.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_tr.properties' is missing. [Translation] [ERROR] [checkstyle] [ERROR] /home/kevin/Desktop/check_style/checkstyle/checkstyle/src/main/resources/com/puppycrawl/tools/checkstyle/api:1: Properties file 'messages_zh.properties' is missing. [Translation] [INFO] [echo] Checkstyle finished (checkstyle-checks.xml) : 22/06/2023 06:48:19 pm

it give error

@romani ping

Considering that FullIdent is used by other checks, it should already be covered by the tests of those checks. I just did what pitest suggested, removed .getFirstChild() and run tests in EqualsHashCodeCheckTest. Many of them failed.

@romani why can't we make that test as part of pitest-api profile?

@Kevin222004 could you try to extend pitest-api profile with additional test class in the pom.xml?

git diff pom.xml diff --git a/pom.xml b/pom.xml index 6bbbdfc62..92b07ca5d 100644 --- a/pom.xml +++ b/pom.xml @@ -4604,6 +4604,7 @@ <param>com.puppycrawl.tools.checkstyle.api.*</param>  <param>com.puppycrawl.tools.checkstyle.checks.coding.UnusedLocalVariableCheckTest</param> + <param>com.puppycrawl.tools.checkstyle.checks.coding.EqualsHashCodeCheckTest</param>  <param>com.puppycrawl.tools.checkstyle.checks.OuterTypeFilenameCheckTest</param> <param>org.checkstyle.suppressionxpathfilter.XpathRegressionUnusedLocalVariableTest</param>

Yes sure I will do that

I also not have any issue to following thing that i done.

if everyone will not agree on #13179 (comment)

No changes should done in src/main/resources folder at all.

I update this soon

This class is not API. Unfortunately leaked a long time ago. It is pure implementation and code in this implementation is not required by any of our Checks , it should be removed.

One day in future we will remove this class from API, it should be just another internal class.

rdiachenko · 2023-06-26T18:56:25Z

@Kevin222004 please resolve conflicts in the meantime.

rdiachenko

lgtm

Vyom-Yadav

Items:

Vyom-Yadav · 2023-07-08T07:26:20Z

src/test/java/com/puppycrawl/tools/checkstyle/api/FullIdentTest.java

+    @Test
+    public void testCreateFullIdentBelow2() throws Exception {
+        final DefaultConfiguration checkConfig = createModuleConfig(ImportOrderCheck.class);
+        final String[] expected = {
+            "3:1: " + getCheckMessage(ImportOrderCheck.class,
+                    MSG_ORDERING, "java.util.HashMap"),
+        };
+
+        verify(checkConfig, getPath("InputFullIdent.java"),
+                expected);
+    }


Why haven't we used the InlineConfigParser?

done removed verify method.

Vyom-Yadav

LGTM! if CI is green.

romani

Ok to merge

romani requested review from Vyom-Yadav, rdiachenko and romani June 11, 2023 06:39

romani assigned rdiachenko Jun 11, 2023

Kevin222004 force-pushed the API5 branch 2 times, most recently from 60a4eef to 13a3756 Compare June 18, 2023 11:59

romani requested changes Jun 19, 2023

View reviewed changes

pom.xml Outdated Show resolved Hide resolved

Kevin222004 force-pushed the API5 branch from 13a3756 to 4b2663f Compare June 19, 2023 11:41

romani approved these changes Jun 19, 2023

View reviewed changes

rdiachenko reviewed Jun 19, 2023

View reviewed changes

Kevin222004 force-pushed the API5 branch 3 times, most recently from 9289aa9 to 80777b6 Compare June 22, 2023 17:51

Kevin222004 force-pushed the API5 branch from 80777b6 to 4d6feee Compare June 26, 2023 19:14

Kevin222004 mentioned this pull request Jun 29, 2023

Targeted Pitest Mutation in July 2023 #13328

Closed

12 tasks

romani self-requested a review June 29, 2023 20:32

Kevin222004 force-pushed the API5 branch from 4d6feee to cf067d1 Compare July 5, 2023 15:02

rdiachenko approved these changes Jul 5, 2023

View reviewed changes

rdiachenko assigned Vyom-Yadav and unassigned rdiachenko Jul 5, 2023

Vyom-Yadav suggested changes Jul 8, 2023

View reviewed changes

Kevin222004 force-pushed the API5 branch 2 times, most recently from 3784688 to 8a20f5b Compare July 8, 2023 08:03

Vyom-Yadav approved these changes Jul 8, 2023

View reviewed changes

Vyom-Yadav assigned romani and unassigned Vyom-Yadav Jul 8, 2023

Issue checkstyle#13109: Kill mutation for FullIdent.java-2

e5ef456

Kevin222004 force-pushed the API5 branch from 8a20f5b to e5ef456 Compare July 8, 2023 08:10

romani approved these changes Jul 8, 2023

View reviewed changes

romani merged commit a2ebb4d into checkstyle:master Jul 8, 2023

	<mutation unstable="false">
	<sourceFile>FullIdent.java</sourceFile>
	<mutatedClass>com.puppycrawl.tools.checkstyle.api.FullIdent</mutatedClass>
	<mutatedMethod>createFullIdentBelow</mutatedMethod>
	<mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
	<description>removed call to com/puppycrawl/tools/checkstyle/api/DetailAST::getFirstChild</description>
	<lineContent>return createFullIdent(ast.getFirstChild());</lineContent>
	</mutation>

		@@ -0,0 +1 @@
		import.ordering=Wrong order for ''{0}'' import.

Uh oh!

Conversation

Kevin222004 commented Jun 7, 2023

Mutation covered

Uh oh!

Kevin222004 commented Jun 8, 2023

Uh oh!

Kevin222004 commented Jun 9, 2023

Uh oh!

romani commented Jun 11, 2023

Uh oh!

Vyom-Yadav commented Jun 12, 2023

Uh oh!

rdiachenko commented Jun 16, 2023

Uh oh!

Kevin222004 commented Jun 18, 2023

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rdiachenko commented Jun 26, 2023

Uh oh!

rdiachenko left a comment

Choose a reason for hiding this comment

Uh oh!

Vyom-Yadav left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Vyom-Yadav left a comment

Choose a reason for hiding this comment

Uh oh!

romani left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants