hcoles/pitest

Bad side effects of Pitest

Closed this issue · 2 comments

1. Pitest config allows only whole Test class execution, but it would be good to have option to run single test method from class

see details at checkstyle/checkstyle#13810 (comment)


2. code might be less elegant if chained call of cast to type is present

ex checkstyle/checkstyle#13913
cast and creation of modifiable checkstyle/checkstyle#13944 (comment)


3. does not allow performance optimization in logic

        while (result
                && !isUsedVariableDeclarationFound
                && currentSiblingAst != null) {

mutated as

        while (!isUsedVariableDeclarationFound
                && currentSiblingAst != null) {

by the removal of the result, the output will not affect.
but the whole loop can run for more time than it was running with the result. This type of change is not good as it might lead to a problem.

For example the pull request :- checkstyle/checkstyle#13557 RequireEmptyLineBeforeBlockTagGroup
checkstyle/checkstyle#13120 VariableDeclarationUsageDistanceCheck
checkstyle/checkstyle#13149 UnusedLocalVariableCheck

Apart of this
It forcing to write less explicit code even it is redundant. So user readability is tiny decreasing sometimes.
Example: checkstyle/checkstyle#13381 (comment)


4. creation of mutable collection consider as can be avoided

checkstyle/checkstyle#13127
and checkstyle/checkstyle#13126

        public Set<DetailAST> getMethodCalls() {
            return Collections.unmodifiableSet(methodCalls);
        }

this code has been mutated as

        public Set<DetailAST> getMethodCalls() {
            return methodCalls;
        }

Modifying the code with Pitest from return Collections.unmodifiableSet(methodCalls) to return new HashSet<>(methodCalls) eliminates the guarantee of returning an unmodifiable set, potentially leading to unintended modifications and inconsistent behavior.


5. Extra code that does help to make code more robust to future changes that might miss extended testing

#13334 (ImportOrderCheck)
The code is extra the removal will not make any issue but it might be problematic in future and affect the user
for example in this pr #13334
we can simply follow the mutation and remove the code from the reset stage but this might lead to a bigger problem.


6. Copy of array(collections) to protect data from changes

checkstyle/checkstyle#13340

This issue seems to report 6 seperate bugs or feature requests. Clicking some of the links leads me to issues in the checkstyle project. Some of these issues are themselves just links to other discussions.

Could you please close this and instead create seperate issues for each point and include in each one a clear description of what pitest's current behaviour is and the desired new behaviour has have you have started to do for points 3 and 4.

yes will create seperate issue