Issue #14019: Kill mutation for setSeverity in Checker #14462

Zopsss · 2024-02-11T06:15:13Z

Issue #14019 Kill mutation for setSeverity in checker

created a test case to kill the mutation

romani

Very good, minor changes:

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java

Zopsss · 2024-02-12T10:02:59Z

⚠️ Failed to start an instance: FAILED_PRECONDITION: Monthly compute limit exceeded!

Cirrus is failing

romani

Items

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java

romani

last minor:

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java

romani

items:

src/test/resources/com/puppycrawl/tools/checkstyle/checker/InputCheckerTestSeverity.java

romani

ok to merge if CI pass

Zopsss · 2024-02-17T15:31:50Z

ok to merge if CI pass

no this is not okay, the test case should be failing after mutation please check this comment #14462 (comment)

romani · 2024-02-17T22:20:24Z

mutatedClass com.puppycrawl.tools.checkstyle.Checker

so you need to set property in Checker not in LineLength.
Play with:

<module name="Checker">
  <property name="severity" value="ignore"/>

Zopsss · 2024-02-18T03:22:44Z

mutatedClass com.puppycrawl.tools.checkstyle.Checker

so you need to set property in Checker not in LineLength. Play with:
<module name="Checker">
  <property name="severity" value="ignore"/>

did this but it is still giving the same result, the test case is still passing even after mutation. Pushed the changes for you to review it. The test case was giving violation on line 11 after I set the severity property to Checker instead of LineLength

Zopsss · 2024-02-19T16:51:00Z

@romani ping

Zopsss · 2024-02-20T07:24:20Z

I tried debugging the code and found that createConfiguration() method used inside verifyWithInlineConfigParser() is not generating proper config. It is not adding the severity property to the Checker module. Then verifyViolations() method is using that inappropriate config to check the violation. We're expecting that there will be no violation as we have added the severity property and set it to ignore but the createConfiguration() is generating the config without severity property and it is expecting violation on line 11. That's why our test case is failing. I have attached the config's ss generated by createConfiguration() below:

the propertyMap should have severity property set to ignore but it contains only charset property. Should we go back to using verify() method or is there a solution available to fix this?

romani · 2024-02-20T14:30:15Z

lets try to move forward and fix code to make in-memory config to be correct (what we expect).

Zopsss · 2024-02-20T14:39:51Z

lets try to move forward and fix code to make in-memory config to be correct (what we expect).

Can you explain me what do you mean by in-memory config? Do you mean the config created by createConfiguration()?

romani · 2024-02-20T16:32:23Z

Do you mean the config created by createConfiguration()?

Yes.

You can fix problem in this method in this PR, looks like it is first time we use config like this, do it will be natural to see it here and immediately use it in Input.

Zopsss · 2024-02-20T17:52:51Z

Do you mean the config created by createConfiguration()?

Yes.

You can fix problem in this method in this PR, looks like it is first time we use config like this, do it will be natural to see it here and immediately use it in Input.

Ohkay and should I keep it in seperate commit?

romani · 2024-02-21T14:36:53Z

in same commit is ok.

Zopsss · 2024-02-24T09:55:38Z

fixed a small bug in createConfiguration(), now our test case is passing and when we mutate the code it is failing as expected.

Reason behind making new createConfiguration():

the regular createConfiguration() does not checks for the global properties, to check for the global properties we needed the whole xml config to see if it contains any global properties before any child modules, for this I created a new createConfiguration() which takes filePath as a parameter. With the help of filePath we can get the whole xml config, after getting it I ran a loop to check if it contains any global properties before starting a new child module, if it does then we add it to the root config, after checking the the xml config we call the regular createConfiguration() method which generates the config with global properties available to it.

I could also modify the current createConfiguration() method to check for global properties but then I will also need to update all the caller methods of it and it might cause some unexpected behaviour, so I thought it would be more reliable to use the new createConfiguration() method.

And also I'm sorry this is taking longer than I expected, I was little bit busy with college stuff....

Zopsss · 2024-02-26T15:41:39Z

@romani

romani · 2024-02-27T17:00:56Z

This is not good, we should reuse existing parser. I can not help on the go, I need big screen time to find solution and guide you. It would be good if you find solution your self

Zopsss · 2024-02-29T19:20:42Z

src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

@@ -247,7 +247,7 @@ inputSource, new PropertiesExpander(System.getProperties()),
                throw new CheckstyleException(
                        "First module should be Checker, but was " + configName);
            }
-            handleXmlConfig(testInputConfigBuilder, inputFilePath, xmlConfig.getChildren());
+            handleXmlConfig(testInputConfigBuilder, inputFilePath, xmlConfig);


removed the getChildren() so that we can also set the properties of Checker module to the testInputConfigBuilder instead of directly setting the children module's properites. By adding Checker's properties to the testInputConfigBuilder, we can access it from childrenModules declared in the TestInputConfiguration.java

Zopsss · 2024-02-29T19:24:25Z

src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

+
+                if ("Checker".equals(moduleName)) {
+                    handleXmlConfig(testInputConfigBuilder, inputFilePath, module.getChildren());
+                }


if the current module is Checker then we're recursively calling the handleXmlConfig() method to set properties of Checker's children modules, this way the testInputConfigBuilder will contain Checker properties as well as Checker's children modules' properties.

it is weird that if ("Checker".equals(moduleName)) { is below of if ("TreeWalker".equals(moduleName)) {
but it is better to fix it at new PR, here is new issue on this - #14653

Zopsss · 2024-02-29T19:37:03Z

src/test/java/com/puppycrawl/tools/checkstyle/bdd/TestInputConfiguration.java

+
+            childrenModules.remove(checkerConfig);
+        }
+


This code checks if the childrenModules contains Checker's properties? if yes then add those properties to the root config and remove the Checker properties' object from childrenModules as we don't need it anymore

Zopsss · 2024-02-29T19:42:13Z

@romani I tried different approach this time, I need your opinion on this. Is it okay or do we need to change something? The test case is passing and after mutating the code it is failing as we expected.

Zopsss · 2024-03-01T03:00:42Z

CircleCI is complaining, idk why though

Zopsss · 2024-03-01T03:15:14Z

src/test/java/com/puppycrawl/tools/checkstyle/bdd/InlineConfigParser.java

@@ -320,6 +324,9 @@ private static String getFullyQualifiedClassName(String filePath, String moduleN
                "com.puppycrawl.tools.checkstyle.checks.javadoc.SummaryJavadocCheck");
        moduleMappings.put("LineLength",
                "com.puppycrawl.tools.checkstyle.checks.sizes.LineLengthCheck");
+        moduleMappings.put("Checker",
+                "com.puppycrawl.tools.checkstyle.CheckerCheck");
+


Had to add Checker here, cuz some tests started throwing an error something like:

config comment is not defined properly

Zopsss · 2024-03-08T19:27:04Z

@romani ping

resolved the merge conflicts. I've made comments explaining the changes, let me know if this is the correct way to enable the verifyWithInlineConfigParser() to check for the global properties ( Checker module's properties ) or not.

Zopsss · 2024-03-12T04:42:33Z

@romani

romani

I confirming that we can not do better solution for now, more refactoring is required.
#14653

but this will unblock all PRs that planning to use Checkers/Treewalker in their tests.

Zopsss force-pushed the checker branch 2 times, most recently from 6d8e4b6 to b3abaa1 Compare February 11, 2024 18:51

Zopsss marked this pull request as ready for review February 11, 2024 19:10

romani requested changes Feb 11, 2024

View reviewed changes

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java Outdated Show resolved Hide resolved

romani mentioned this pull request Feb 11, 2024

Issue #11446: Update TestTabViolation Method Of Checkertest #14461

Merged

nrmancuso assigned romani Feb 12, 2024

Zopsss force-pushed the checker branch 5 times, most recently from a103d61 to 5e614d1 Compare February 12, 2024 09:30

romani requested changes Feb 12, 2024

View reviewed changes

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java Show resolved Hide resolved

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java Outdated Show resolved Hide resolved

Zopsss force-pushed the checker branch from 5e614d1 to b66f5ee Compare February 13, 2024 05:19

romani requested changes Feb 13, 2024

View reviewed changes

src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java Outdated Show resolved Hide resolved

Zopsss force-pushed the checker branch from b66f5ee to 317625a Compare February 17, 2024 10:24

romani requested changes Feb 17, 2024

View reviewed changes

src/test/resources/com/puppycrawl/tools/checkstyle/checker/InputCheckerTestSeverity.java Show resolved Hide resolved

Zopsss force-pushed the checker branch from 317625a to cd7cc4b Compare February 17, 2024 15:26

romani approved these changes Feb 17, 2024

View reviewed changes

romani self-requested a review February 17, 2024 22:21

Zopsss force-pushed the checker branch from cd7cc4b to 488bf2f Compare February 18, 2024 03:21

romani mentioned this pull request Feb 18, 2024

doc: mention text block suppression #14491

Merged

Zopsss force-pushed the checker branch 2 times, most recently from cceaed1 to 2800511 Compare February 20, 2024 07:21

Zopsss force-pushed the checker branch 3 times, most recently from 8bcc43d to e71b610 Compare February 24, 2024 09:34

Zopsss force-pushed the checker branch from e71b610 to 895db36 Compare February 29, 2024 19:16

Zopsss commented Feb 29, 2024

View reviewed changes

Zopsss commented Mar 1, 2024

View reviewed changes

Issue checkstyle#14019: Kill mutation for severity in Checker

fa1fafa

Zopsss force-pushed the checker branch from c0010c0 to fa1fafa Compare March 8, 2024 19:25

romani approved these changes Mar 12, 2024

View reviewed changes

romani merged commit 2c00d23 into checkstyle:master Mar 12, 2024
113 checks passed

Zopsss deleted the checker branch March 14, 2024 10:42

Lmh-java mentioned this pull request Mar 16, 2024

Issue #14653: support XML configs without detalization #14659

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #14019: Kill mutation for setSeverity in Checker #14462

Issue #14019: Kill mutation for setSeverity in Checker #14462

Zopsss commented Feb 11, 2024 •

edited

romani left a comment

Zopsss commented Feb 12, 2024 •

edited

romani left a comment

romani left a comment

romani left a comment

romani left a comment

Zopsss commented Feb 17, 2024

romani commented Feb 17, 2024

Zopsss commented Feb 18, 2024 •

edited

Zopsss commented Feb 19, 2024

Zopsss commented Feb 20, 2024 •

edited

romani commented Feb 20, 2024

Zopsss commented Feb 20, 2024

romani commented Feb 20, 2024

Zopsss commented Feb 20, 2024

romani commented Feb 21, 2024

Zopsss commented Feb 24, 2024 •

edited

Zopsss commented Feb 26, 2024

romani commented Feb 27, 2024 •

edited

Zopsss Feb 29, 2024 •

edited

Zopsss Feb 29, 2024 •

edited

romani Mar 12, 2024

Zopsss Feb 29, 2024 •

edited

Zopsss commented Feb 29, 2024

Zopsss commented Mar 1, 2024

Zopsss Mar 1, 2024

Zopsss commented Mar 8, 2024 •

edited

Zopsss commented Mar 12, 2024

romani left a comment •

edited

Issue #14019: Kill mutation for setSeverity in Checker #14462

Issue #14019: Kill mutation for setSeverity in Checker #14462

Conversation

Zopsss commented Feb 11, 2024 • edited

romani left a comment

Choose a reason for hiding this comment

Zopsss commented Feb 12, 2024 • edited

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

Zopsss commented Feb 17, 2024

romani commented Feb 17, 2024

Zopsss commented Feb 18, 2024 • edited

Zopsss commented Feb 19, 2024

Zopsss commented Feb 20, 2024 • edited

romani commented Feb 20, 2024

Zopsss commented Feb 20, 2024

romani commented Feb 20, 2024

Zopsss commented Feb 20, 2024

romani commented Feb 21, 2024

Zopsss commented Feb 24, 2024 • edited

Zopsss commented Feb 26, 2024

romani commented Feb 27, 2024 • edited

Zopsss Feb 29, 2024 • edited

Choose a reason for hiding this comment

Zopsss Feb 29, 2024 • edited

Choose a reason for hiding this comment

romani Mar 12, 2024

Choose a reason for hiding this comment

Zopsss Feb 29, 2024 • edited

Choose a reason for hiding this comment

Zopsss commented Feb 29, 2024

Zopsss commented Mar 1, 2024

Zopsss Mar 1, 2024

Choose a reason for hiding this comment

Zopsss commented Mar 8, 2024 • edited

Zopsss commented Mar 12, 2024

romani left a comment • edited

Choose a reason for hiding this comment

Zopsss commented Feb 11, 2024 •

edited

Zopsss commented Feb 12, 2024 •

edited

Zopsss commented Feb 18, 2024 •

edited

Zopsss commented Feb 20, 2024 •

edited

Zopsss commented Feb 24, 2024 •

edited

romani commented Feb 27, 2024 •

edited

Zopsss Feb 29, 2024 •

edited

Zopsss Feb 29, 2024 •

edited

Zopsss Feb 29, 2024 •

edited

Zopsss commented Mar 8, 2024 •

edited

romani left a comment •

edited