Skip to content

Issue #13109: Kill mutation for PackageDeclarationCheck#13133

Merged
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:p1
Jun 16, 2023
Merged

Issue #13109: Kill mutation for PackageDeclarationCheck#13133
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:p1

Conversation

@Kevin222004
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 commented Jun 1, 2023

Issue #13109: Kill mutation for PackageDeclarationCheck


Mutation

<mutation unstable="false">
<sourceFile>PackageDeclarationCheck.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.checks.coding.PackageDeclarationCheck</mutatedClass>
<mutatedMethod>beginTree</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.MemberVariableMutator</mutator>
<description>Removed assignment to member variable defined</description>
<lineContent>defined = false;</lineContent>
</mutation>

@Kevin222004 Kevin222004 marked this pull request as draft June 2, 2023 12:21
@romani
Copy link
Copy Markdown
Member

romani commented Jun 2, 2023

why draft ? no regression report is executed.

we already discussed somewhere that all reset in beginTree are for case when same instance of Check is used on two files, Check is state-ful, and we should not affect new files by values in fields from previous java file.

@Kevin222004
Copy link
Copy Markdown
Contributor Author

@romani
Copy link
Copy Markdown
Member

romani commented Jun 2, 2023

How long it takes for your local to generate such report ?
Do you able to do something else meanwhile ?

Link in report is not working https://kevin222004.github.io/reports/p1/2023-06-02-T-08-07-37/test-report/dbeaver/xref/__w/Kevin222004.github.io/Kevin222004.github.io/contribution/checkstyle-tester/repositories/dbeaver/plugins/org.jkiss.dbeaver.ext.mysql/src/MySQLErrorsTest.java.html#L18
Let me know if you do this on purpose to avoid huge size of website

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Jun 2, 2023

@romani i am not generating report on local
I generate using https://github.com/Kevin222004/Kevin222004.github.io

but ci takes almost or more than 3 hours for one single report, so mostly my local will crash if I generate on it :)

Link in report is not working

yes i have to look why it is not working
but i have generate the report for 4 projects in local and tested this failure on local they are not creating issue their
I am looking at #13133 (comment)

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Jun 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:


@Override
public void beginTree(DetailAST ast) {
defined = false;
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.

reminder, all initialization in this method, is reset of Check state before starting new file, to reproduce problem we need to run same Check instance on 2 special input files to let "defined = true" affect validation in second file even it has no java code that make "defined = true".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@romani I have tried to write a test case as in other check.

    @Test
    public void testClearStateInLambda() throws Exception {
        final PackageDeclarationCheck check = new PackageDeclarationCheck();
        final Optional<DetailAST> package_def = TestUtil.findTokenInAstByPredicate(
                JavaParser.parseFile(
                        new File(getPath("InputPackageDeclarationPlain.java")),
                        JavaParser.Options.WITHOUT_COMMENTS),
                ast -> ast.getType() == TokenTypes.PACKAGE_DEF);
        assertWithMessage("ast should contain PACKAGE_DEF")
                .that(package_def.isPresent())
                .isTrue();
        assertWithMessage("State is not cleared on beginTree")
                .that(TestUtil.isStatefulFieldClearedDuringBeginTree(check, package_def.get(),
                        "defined", defined -> {
                            return !((boolean) defined);
                        }))
                .isTrue();
    }

I am getting only null pointer exception here am i missing any thing.

java.lang.NullPointerException: Cannot invoke "com.puppycrawl.tools.checkstyle.api.FileContents.getFileName()" because "java.lang.ThreadLocal.get().fileContents" is null

	at com.puppycrawl.tools.checkstyle.api.AbstractCheck.getFilePath(AbstractCheck.java:317)
	at com.puppycrawl.tools.checkstyle.checks.coding.PackageDeclarationCheck.getDirectoryName(PackageDeclarationCheck.java:183)
	at com.puppycrawl.tools.checkstyle.checks.coding.PackageDeclarationCheck.visitToken(PackageDeclarationCheck.java:169)
	at com.puppycrawl.tools.checkstyle.internal.utils.TestUtil.isStatefulFieldClearedDuringBeginTree(TestUtil.java:139)

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.

Do not deal with pure Junit method , we should cover it by Inputs.
New or existing verify method that takes creates check ones and execute two files sequentially.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

@Kevin222004 Kevin222004 force-pushed the p1 branch 2 times, most recently from abbc72b to 3c3a6f1 Compare June 14, 2023 08:55
@Kevin222004 Kevin222004 marked this pull request as ready for review June 14, 2023 08:55
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.

Ok to merge.
Just Input based test extension, best scenario.

I move it further for review by others to learn on this.

@@ -0,0 +1,7 @@
package com.puppycrawl.tools.checkstyle.checks.coding.packagedeclaration;
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.

So, why do we need this new input file if it is not used in the new test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

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 Jun 14, 2023
Copy link
Copy Markdown
Member

@Vyom-Yadav Vyom-Yadav left a comment

Choose a reason for hiding this comment

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

LGTM!

@Vyom-Yadav Vyom-Yadav assigned romani and unassigned Vyom-Yadav Jun 16, 2023
@romani romani merged commit 3f25c37 into checkstyle:master Jun 16, 2023
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