Skip to content

Issue #13809: Kill mutation in Filters#13841

Merged
nrmancuso merged 1 commit intocheckstyle:masterfrom
Kevin222004:filterTest
Oct 26, 2023
Merged

Issue #13809: Kill mutation in Filters#13841
nrmancuso merged 1 commit intocheckstyle:masterfrom
Kevin222004:filterTest

Conversation

@Kevin222004
Copy link
Copy Markdown
Contributor

@Kevin222004 Kevin222004 commented Oct 7, 2023 •
edited by romani
Loading

Issue #13809: Kill mutation in Filters

this is typical extra data storage that pitest catch as not critical for execution.
I think it is fine to remove it here as cost of taking String value of regexp from Pattern object is cheap, as it is stored as is (String) in Patter class (it is single getter to get String on Pattern class).

https://github.com/openjdk/jdk/blob/ecd25e7d6f9d69f9dbdbff0a4a9b9d6b19288593/src/java.base/share/classes/java/util/regex/Pattern.java#L1138-L1144

@Kevin222004 Kevin222004 mentioned this pull request Oct 7, 2023
Closed
33 tasks
rnveach
rnveach suggested changes Oct 7, 2023
View reviewed changes
@Kevin222004 Kevin222004 force-pushed the filterTest branch 2 times, most recently from 90e8c5d to 3be5d33 Compare October 9, 2023 20:45
romani
romani requested changes Oct 10, 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.

items:

@romani
Copy link
Copy Markdown
Member

romani commented Oct 18, 2023

@Kevin222004 , please consider #13903 , please reuse this code to make CI green

romani
romani requested changes Oct 18, 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.

Items

@romani
Copy link
Copy Markdown
Member

romani commented Oct 18, 2023

@Kevin222004 , please update Checker

@Kevin222004 Kevin222004 force-pushed the filterTest branch 2 times, most recently from 38ce21c to 8677e6f Compare October 19, 2023 18:50
@romani
Copy link
Copy Markdown
Member

romani commented Oct 22, 2023

@Kevin222004 , please remove second commit, make PR ready for review and merge.
Regression testing for this update doesn't not required and doesn't make sense, as this purely config nuances.

@Kevin222004
Copy link
Copy Markdown
Contributor Author

Kevin222004 commented Oct 22, 2023

OK I just need diff of checker.

@romani
Copy link
Copy Markdown
Member

romani commented Oct 22, 2023

For now, let's just suppress Checker.

@romani
Copy link
Copy Markdown
Member

romani commented Oct 22, 2023

pitest passed on my local, so we are moving forward with review.

romani
romani approved these changes Oct 22, 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 assigned rnveach and unassigned romani Oct 22, 2023
@romani romani force-pushed the filterTest branch 3 times, most recently from 16dd9d1 to 3c681e8 Compare October 22, 2023 14:59
rnveach
rnveach suggested changes Oct 22, 2023
View reviewed changes
rdiachenko
rdiachenko approved these changes Oct 23, 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

@romani
Copy link
Copy Markdown
Member

romani commented Oct 23, 2023 •
edited
Loading

@Kevin222004 , please fix Checker https://github.com/checkstyle/checkstyle/actions/runs/6616385677/job/17970549707?pr=13841#step:6:640

New surviving error(s) found:

  <checkerFrameworkError unstable="false">
    <fileName>src/main/java/com/puppycrawl/tools/checkstyle/site/XdocsTemplateSink.java</fileName>
    <specifier>override.receiver</specifier>
    <message>Incompatible receiver type</message>
    <lineContent>public void println() {</lineContent>
    <details>
      found   : @GuardedBy CustomPrintWriter
      required: @GuardSatisfied PrintWriter
      Consequence: method in @GuardedBy CustomPrintWriter
      void println(@GuardedBy CustomPrintWriter this)
      cannot override method in @GuardedBy PrintWriter
      void println(@GuardSatisfied PrintWriter this)
    </details>
  </checkerFrameworkError>

@romani
Copy link
Copy Markdown
Member

romani commented Oct 24, 2023 •
edited
Loading

I can not reproduce it on local, restarted.

Still failing
https://github.com/checkstyle/checkstyle/actions/runs/6616385677/job/18001969867?pr=13841#step:7:17
Looks like we need this diff in this PR.

@romani
Copy link
Copy Markdown
Member

romani commented Oct 25, 2023

@rnveach , CI is green, please finish review

@rnveach rnveach assigned nrmancuso and unassigned rnveach Oct 25, 2023
@rnveach rnveach requested a review from nrmancuso October 25, 2023 15:40
Vyom-Yadav
Vyom-Yadav approved these changes Oct 25, 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!

@nrmancuso nrmancuso merged commit 15e1fdd into checkstyle:master Oct 26, 2023
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

+3 more reviewers

@rnveach rnveach rnveach approved these changes

@nrmancuso nrmancuso nrmancuso approved these changes

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

Reviewers whose approvals may not affect merge requirements

Assignees

@nrmancuso nrmancuso

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@Kevin222004 @romani @rdiachenko @rnveach @nrmancuso @Vyom-Yadav