Skip to content

Bad Side effects of Pitest #13142

@Kevin222004

Description

@Kevin222004

This issue is been created to make a list of some bad side effects of Pitest on the codebase.

All activity was done in scope of gsoc 2023 project https://github.com/checkstyle/checkstyle/wiki/Checkstyle-GSoC-2023-Project-Ideas#project-name-pitest-resolution

0 All caching of operations

Surviving pitest mutations for conditions reducing time complexity at PackageObjectFactory - #11902

1 creation of mutable collection consider as can be avoided

#13127
and #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.

but in same time if method is API, it is good to add pure unit test to kill mutation - Example.

2 does not allow performance optimization in logic

#13557 RequireEmptyLineBeforeBlockTagGroup
#13120 VariableDeclarationUsageDistanceCheck
#13149 UnusedLocalVariableCheck
#13196 MagicNumberCheck
#13416 BlockCommentPosition
#13430 CheckUtil

        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.


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


3. 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.


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

#13340


5. 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 #13810 (comment)

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

Read details at #13913
cast and creation of modifiable #13944 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions