Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sync Checkstyle integration test #999

Merged
merged 6 commits into from
Jan 30, 2024
Merged

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Jan 27, 2024

The two commits are independently reviewable.

Suggested commit message:

Sync Checkstyle integration test (#999)

Summary of changes:
- Update the set of expected changes for compatibility with the latest
  features, except for the `ErrorProneRuntimeClasspath` check.
- Test against version 10.12.7 rather than 10.13.0.
- Omit the targeted tag from file names, so that similar upgrade PRs can be
  tested using an `/integration-test` GitHub comment.

@Stephan202 Stephan202 added this to the 0.15.0 milestone Jan 27, 2024
@Stephan202 Stephan202 requested a review from rickie January 27, 2024 13:00
@Stephan202
Copy link
Member Author

/integration-test

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202
Copy link
Member Author

/integration-test

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202
Copy link
Member Author

The /integration-test build fails, because (IIUC) the GitHub action config is derived from master. Perhaps we should rename the scripts to omit the exact tag used. Assuming we don't want to track multiple tags per project, this will simplify things.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202
Copy link
Member Author

Added a commit and updated the suggested commit message. Do still need to validate these changes locally.

@rickie
Copy link
Member

rickie commented Jan 27, 2024

Cool stuff! At least locally it now runs for me 😄. Already updated #867 with the latest state of this PR.

@rickie rickie added the chore A task not related to code (build, formatting, process, ...) label Jan 27, 2024
@Stephan202
Copy link
Member Author

I noticed that with my changes at some point the build would fail with:

[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.12.1:compile (error-prone-compile) on project checkstyle: Fatal error compiling: AmbiguousJsonCreator is not a valid checker name -> [Help 1]

I added a commit to fix that.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@Stephan202
Copy link
Member Author

K, validated the latest changes. PR is now properly ready for review.

@Stephan202 Stephan202 force-pushed the sschroevers/update-integration-test branch from ba46568 to eb1af08 Compare January 29, 2024 05:55
@Stephan202
Copy link
Member Author

Rebased and updated the PR to target version 10.13.0. Suggested commit message updated.

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this one up-to-date!

Smart move to drop the version from the file names :).

Ran it locally and it works :).

Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

Is the goal here to run /integration-tests/checkstyle.sh succesfully ?
Because It is not running for me (macOs).

usage: paste [-s] [-d delimiters] file ...
grep: invalid option -- P
usage: grep [-abcdDEFGHhIiJLlMmnOopqRSsUVvwXxZz] [-A num] [-B num] [-C[num]]
        [-e pattern] [-f file] [--binary-files=value] [--color=when]
        [--context[=num]] [--directories=action] [--label] [--line-buffered]
        [--null] [pattern] [file ...]

also fails with --sync flag. What am I missing ?

@rickie
Copy link
Member

rickie commented Jan 30, 2024

We're working on that in #867 😄. Although I myself get some changes on that branch... So Mac support is close I'm working on it.

@rickie rickie force-pushed the sschroevers/update-integration-test branch from eb1af08 to 6b3d076 Compare January 30, 2024 12:16
Copy link
Contributor

@mohamedsamehsalah mohamedsamehsalah left a comment

Choose a reason for hiding this comment

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

Clear 👍

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

@rickie rickie force-pushed the sschroevers/update-integration-test branch from 6b3d076 to 4638392 Compare January 30, 2024 14:05
@rickie
Copy link
Member

rickie commented Jan 30, 2024

Rebased,will merge!

Copy link

Looks good. No mutations were possible for these changes.
Mutation testing report by Pitest. Review any surviving mutants by inspecting the line comments under Files changed.

Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@rickie rickie merged commit 90066f8 into master Jan 30, 2024
16 checks passed
@rickie rickie deleted the sschroevers/update-integration-test branch January 30, 2024 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore A task not related to code (build, formatting, process, ...)
Development

Successfully merging this pull request may close these issues.

3 participants