Skip to content

Issue #13501: Kill mutation for ImportOrderCheck-2#13232

Merged
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:IOC2
Sep 13, 2023
Merged

Issue #13501: Kill mutation for ImportOrderCheck-2#13232
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:IOC2

Conversation

@Kevin222004
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 commented Jun 15, 2023

Issue #13501: Kill mutation for ImportOrderCheck-2


Check :
https://checkstyle.org/config_imports.html#ImportOrder
https://checkstyle.org/checks/imports/importorder.html#ImportOrder


Mutation covered

<mutation unstable="false">
<sourceFile>ImportOrderCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck</mutatedClass>
<mutatedMethod>getGroupNumber</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
<description>removed call to java/util/regex/Matcher::end</description>
<lineContent>bestEnd = matcher.end();</lineContent>
</mutation>
<mutation unstable="false">
<sourceFile>ImportOrderCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.imports.ImportOrderCheck</mutatedClass>
<mutatedMethod>getGroupNumber</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.NonVoidMethodCallMutator</mutator>
<description>removed call to java/util/regex/Matcher::start</description>
<lineContent>if (matcher.start() &lt; bestPos) {</lineContent>
</mutation>


Explaination

https://pitest.org/quickstart/mutators/#non-void-method-call-mutator-non_void_method_calls

test was added.


Regression

Diff Regression config: https://gist.githubusercontent.com/Kevin222004/64068f25e12fc53287e29c691b44b0cd/raw/3f08e73e3946ef9ef59bd119028bb67eff99b15b/ImportOrder.xml
Diff Regression projects: https://gist.githubusercontent.com/Kevin222004/21e3934e85f802e2fbd48af06d122364/raw/604256badd733d8568064f371d55657c04b00dfd/test-projects-2.properties
Report label: R2

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 changed the title Issue #13109: Kill mutation for ImportOrderCheck-2 Issue #13501: Kill mutation for ImportOrderCheck-2 Aug 2, 2023
@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 2, 2023

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Github, generate report

@Kevin222004 Kevin222004 marked this pull request as ready for review August 2, 2023 17:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Aug 2, 2023

Copy link
Copy Markdown
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Items:

}
else if (matcher.start() == bestPos && matcher.end() > bestEnd) {
bestIndex = i;
bestEnd = matcher.end();
Copy link
Copy Markdown
Member

@romani romani Aug 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update means that we do select of first match.

To use this method, we need to make config, where user provided multiple patterns that overlap and we have import that can match several, and we select only best match.

So regression will not find anything, we just need to make bunch of unrealistic configs.

To catch a case, you can make this method as public, and trace all possible values to patterns and name to catch what we need. After that you simply take required patterns and put in Input.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kevin222004 , ping.
Please play with almost same groups.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kevin222004 , here is how to quicky play with this methods coverage - e0e1161

ones you kill it by pure unit test, you can convert it to Input based.

Copy link
Copy Markdown
Contributor Author

@Kevin222004 Kevin222004 Aug 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is getting a long delay in this pr but I am trying to find test

Just a update on this pr I have found a pure ut which kill one mutation if (matcher.start() < bestPos) {

@Test
public void testPitest() throws Exception {
    Pattern[] patterns = new Pattern[6];

    patterns[0] = Pattern.compile("awt");
    patterns[5] = Pattern.compile("jarInputStream");
    patterns[4] = Pattern.compile(".util");
    patterns[3] = Pattern.compile(".jar.");
    patterns[2] = Pattern.compile(".util.");
    patterns[1] = Pattern.compile("jar");

    assertWithMessage("")
            .that(ImportOrderCheck.getGroupNumber(patterns, "java.util.jar.JarInputStream"))
            .isEqualTo(4);
}

I am trying to make this input base and also for other mutation trying to find test

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we missed a fact that we treat provided packages as full name from the beginning,

if (!pkg.endsWith(".")) {
pkgBuilder.append('.');
}
grp = Pattern.compile("^" + Pattern.quote(pkgBuilder.toString()));

so we can not use .util or awt , all patterns must have full name from the beginning. So it meant that getGroupNumber should be aware of this, so finding most early bestPos is nonsense at line if (matcher.start() < bestPos) {.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, we can use regexp, but we have to use / around values to make it treated as-is - https://checkstyle.org/checks/imports/importorder.html#Properties

@romani romani assigned romani and unassigned Vyom-Yadav Aug 8, 2023
@romani romani force-pushed the IOC2 branch 4 times, most recently from 532f58f to 065ca7c Compare September 13, 2023 13:12
Copy link
Copy Markdown
Member

@romani romani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just test.

Ok to merge

@romani romani removed their assignment Sep 13, 2023
@romani romani removed the request for review from Vyom-Yadav September 13, 2023 13:32
Copy link
Copy Markdown
Member

@rdiachenko rdiachenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@rdiachenko rdiachenko assigned Vyom-Yadav and unassigned rdiachenko Sep 13, 2023
@romani romani merged commit 89daee5 into checkstyle:master Sep 13, 2023
@romani
Copy link
Copy Markdown
Member

romani commented Sep 13, 2023

Just test, we merge on single review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants