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

Resolve Pitest Issues #7797

Closed
rnveach opened this issue Mar 8, 2020 · 3 comments
Closed

Resolve Pitest Issues #7797

rnveach opened this issue Mar 8, 2020 · 3 comments
Milestone

Comments

@rnveach
Copy link
Member

rnveach commented Mar 8, 2020

This issue is the main tracker of all pitest work and will be split into multiple individual issues as new work is identified. Nothing can be done in this issue.

Shown in https://github.com/checkstyle/checkstyle/blob/master/.ci/pitest.sh ,

Checkstyle uses pitest (mutation testing) to test if there is enough tests to cover all code in main source. If new code is written, or existing code is re-written, and no tests are added, pitest will fail and require more tests to cover the new changes. It is extremely important to get familar with pitest as it is an integral part of working with Checkstyle.

Checkstyle adopted pitest after we already had a bunch of modules and tests. Sadly, not all tests cover all the code we have and pitest is flagging additional lines that need to be resolved. Following pitest's terminology, lines that still need to be resolved are called Surviving mutations. The overview of all unresolved mutations can be found at https://github.com/checkstyle/checkstyle/blob/master/.ci/pitest.sh .

If at anytime, more help is needed, don't hesitate to ping an admin for guidance.

For each issue the following steps need to be taken:

  1. The pitest report needs to be run on a clean checkstyle master branch and uploaded to the issue on exactly the type of mutation(s) that is surviving. It is possible that more than 1 mutation could be surviving on the same line, but it is also possible removing 1 mutation will remove the others too.
  2. The reported pitest mutation must be hardcoded into a new checkstyle branch and the branch uploaded to github and reported in the issue to verify the user understands the mutation and how pitest works behind the scenes. See https://pitest.org/quickstart/mutators/
  3. Regression must be run with the hardcoded mutation branch and the regression uploaded to the issue to show if regression can find test cases that can be added to our own UTs to kill the mutations. See https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-pitest-regression . We recommend running regression on all projects.
    Note: Step 3 may not be able to be done if the item to kill is throwing an exception that will terminate the checkstyle run. Regression is more for logic surrounding violations being printed or not.

4a) If regression does find a case, or a case is found by another means, a PR must be submitted adding only the new case to checkstyle. The mutated branch must not be used and can be deleted.
4aa) In the changes for the PR, pitest.sh has to be modified to remove the line in question to show the mutation is killed and no longer a concern.
4ab) With the PR a new pitest must be run and uploaded to the PR to show that the new test case does kill the mutation.

4b) If regression fails to find a case, it must be determined why the code exists and why regression didn't find anything. Some mutations require very specific code and may require additional analysis of surrounding code. Some code may never be possible to be hit and is infact unreachable.
4ba) When the admins/mentors have agree with the determination that the code is no longer needed, a PR must be submitted removing the identified code from checkstyle. The mutated branch must not be used and can be deleted.
4bb) If a UT can be found while analzing the code, then see 4a.

1: The command to run pitest is ./.ci/pitest.sh <Profile>, where <Profile> is the name of the profile given to the package. Pitest execution is split by package to reduce execution time. All pitest profiles can be seen at https://github.com/checkstyle/checkstyle/blob/master/.circleci/config.yml . The pitest report will specify the exact mutation being applied, and will report failures with a red line and saying "SURVIVED".

Example Command: To run the javadoc pitest, you would run ./.ci/pitest.sh pitest-javadoc.

Example Report with Failure: https://user-images.githubusercontent.com/812984/64864039-0d60f680-d5eb-11e9-8b55-10cc7c0b1fe1.png

2: Hardcoding the mutation depends entirely on the specific mutation being examined. Pitest's own website explains the mutations in more details. For example, if you get a surviving mutation saying replaced equality check with true → SURVIVED, then you must the specific equality check in the code and replace it entirely with true. 1 line of code can have multiple mutations connected to it. It is important to understand which part of the line is being mutated. If there is any confusion, you can space out the lines before running the report to help get a better idea.

3: It is important that regression hits all areas of the check in order to determine if the mutated code is even being hit. For most checks, this usually means enabling and disabling all properties for a module in as many permutations as possible to cover all expected avenues. See https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-pitest-regression . For modules that have properties that allow custom strings, a better understanding of the property and the underlying code may be needed to know who values to put into the property.

pitest: https://pitest.org/

overview of pitest mutations: https://pitest.org/quickstart/mutators/

unresolved pitest failures: https://github.com/checkstyle/checkstyle/blob/master/.ci/pitest.sh

Regression with pitest: https://github.com/checkstyle/contribution/tree/master/checkstyle-tester#checkstyle-pitest-regression

Additional work to add to this issue (which can't be worked on separately during GSoC):
#4605
#5673
#6294
#6295
#6320
#6658
#7132

@rnveach
Copy link
Member Author

rnveach commented Mar 15, 2020

@Mishrasubha This issue is not open to be worked on. Please see the open child issues.

@romani
Copy link
Member

romani commented Mar 17, 2020

@rnveach , I removed label from it to not attract attention

@nrmancuso nrmancuso added this to the 10.4 milestone Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants