Skip to content

Issue #13109: Kill mutation for FileText 2#13158

Merged
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:FT1
Jun 7, 2023
Merged

Issue #13109: Kill mutation for FileText 2#13158
romani merged 1 commit intocheckstyle:masterfrom
Kevin222004:FT1

Conversation

@Kevin222004
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 commented Jun 4, 2023

Issue #13109: Kill mutation for FileText 2

Mutation covered

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

Lines 75 to 82 in 82b6ac4

<mutation unstable="false">
<sourceFile>FileText.java</sourceFile>
<mutatedClass>com.puppycrawl.tools.checkstyle.api.FileText</mutatedClass>
<mutatedMethod>&lt;init&gt;</mutatedMethod>
<mutator>org.pitest.mutationtest.engine.gregor.mutators.experimental.MemberVariableMutator</mutator>
<description>Removed assignment to member variable lineBreaks</description>
<lineContent>lineBreaks = null;</lineContent>
</mutation>

Explaination

Our code is

    public FileText(FileText fileText) {
        file = fileText.file;
        charset = fileText.charset;
        fullText = fileText.fullText;
        lines = fileText.lines.clone();
        if (fileText.lineBreaks == null) {
            lineBreaks = null;
        }
        else {
            lineBreaks = fileText.lineBreaks.clone();
        }
    }

Here the part of code

        if (fileText.lineBreaks == null) {
            lineBreaks = null;
        }

is mutated as

        if (fileText.lineBreaks == null) {
        }

Here the condition in if statment is
something like

fileText.lineBreaks == null

know if we see logically their is 2 possiblity
either fileText.lineBreaks is null or it is
not null if it is null then the if block will
execute and in the case of not null else will execute.

know instead of doing this fileText.lineBreaks == null in if block and set the lineBreaks to null
we can change the if condition to

        if (fileText.lineBreaks != null) {
            lineBreaks = fileText.lineBreaks.clone();
        }

so the new code look like

    public FileText(FileText fileText) {
        file = fileText.file;
        charset = fileText.charset;
        fullText = fileText.fullText;
        lines = fileText.lines.clone();
        if (fileText.lineBreaks != null) {
            lineBreaks = fileText.lineBreaks.clone();
        }
        else {
            lineBreaks = null;
        }
    }

Here lineBreaks set to null which is its default value and this is constructor. so if we remove the default value it won't effect anything.
in this case when fileText.lineBreaks == null by default it will asign lineBreaks = null;
it will not make any effect to code

so finall code we write as

    public FileText(FileText fileText) {
        file = fileText.file;
        charset = fileText.charset;
        fullText = fileText.fullText;
        lines = fileText.lines.clone();
        if (fileText.lineBreaks != null) {
            lineBreaks = fileText.lineBreaks.clone();
        }
    }

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Jun 4, 2023

How to do regression testing here?

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Jun 4, 2023

ready for review

@romani
Copy link
Copy Markdown
Member

romani commented Jun 4, 2023

resolve Checker violation.

This class is used by all Checks and Filters, I think this is a case where we do not need diff regression. Green CI is enough.

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Jun 4, 2023

Done
I have suppressed the new surviving one.
Reason
<message>the constructor does not initialize fields: lineBreaks</message>
if I will initialize the lineBreak then it is still the same pitest issue their

@Kevin222004 Kevin222004 mentioned this pull request Jun 4, 2023
Merged
romani
romani approved these changes Jun 4, 2023
View reviewed changes
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

@romani romani requested review from Vyom-Yadav and rdiachenko June 4, 2023 15:42
rdiachenko
rdiachenko approved these changes Jun 6, 2023
View reviewed changes
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 6, 2023
Vyom-Yadav
Vyom-Yadav approved these changes Jun 7, 2023
View reviewed changes
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 7, 2023
@romani romani merged commit e1ad371 into checkstyle:master Jun 7, 2023
@Kevin222004 Kevin222004 mentioned this pull request Jun 29, 2023
Closed
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@romani romani romani approved these changes

@rdiachenko rdiachenko rdiachenko approved these changes

+1 more reviewer

@Vyom-Yadav Vyom-Yadav Vyom-Yadav approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@romani romani

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Kevin222004 @romani @rdiachenko @Vyom-Yadav