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