-
Notifications
You must be signed in to change notification settings - Fork 18
Test Suite Design with 100% Mutation Score
The source code and especially the test suite provides an example with 100% mutation score.
Some of the inferior implementation choices were made deliberately for demonstration purposes and are described for knowledge sharing in this article. It's generally not recommended to do it that way if there are better or cleaner alternatives.
One of the design goals of the test suite was to avoid the use of mocking. The testsuite makes heavy use of test harnesses provided by the SonarQube test API as well as custom test harnesses.
However there was one case - the ReportCollectorTest - where the use of mocking was inevitable to induce an IOException on filesystem level. For all other tests we accomplished to arrange the test setups using standard APIs of SonarQube, Java and test harnesses.
As conclusion for pursuing the no/less mock approach, it's definitely recommendable to reduce the use of mocks to the bare minimum. It simplifies the test-preparation code, provides much cleaner tests and in some cases even faster tests. Nevertheless, mocks are still usefull on clear boundaries of the system or for cases that are otherwise not or only with high costs doable.
The code base also provides some example of the (negative) effects of 100% mutation score when all mutation operators Pitest provides are enabled. In other words, a "killing spree", aiming at 100% mutation score, might end up in less good code. For this plugin we did it on purpose for demonstration effects and the cases were rather rare. In the following sections we'd like to share our insights.
The effects on the code base could be summarized in two categories:
- de-optimization (shortcuts in control flow)
- bloating code (varargs and enums, extraction of utility methods)
In some situations, short-cut code had to be removed because the existence of it was an equivalent mutation.
For instance a check list.isEmpty() is unnecessary if later on a stream iterates on zero elements.
if(list.isEmpty()) { //removing this short-cut is an equivalent mutation
return;
}
list.stream()....In "normal" such a removal - especially when it actually provides a performance optimization - is not recommendable, because it may prevent a much more expensive operation, such as creating a stream.
The uses of varargs (i.e. method(String... args)) produces compiler-generated artifacts which Pitest (at least in version 1.4) does not recognize as such and mutates them. But there are no means for killing them.
One thing Pitest does, is reordering the elements of the varargs. In case the order does not matter, i.e. when adding Extension classes to the Plugin Context, any reordering is an equivalent mutation.
One way to kill this mutant is to over-specify by testing for an order.
For example, for this code:
context.addExtensions(Class1.class, Class2.class, Class3.class);The InlineConst-Mutator generates mutations like "Substituted 1->0", "Substituted 2->3" etc
With a test like this
List<Class> extensions = context.getExtensions();
assertEquals(Class1.class, extensions.get(0));
assertEquals(Class2.class, extensions.get(1));
assertEquals(Class3.class, extensions.get(2));the mutants could be killed - some of it. However a big disadvantage is, that this specification is not needed as the order does not matter. Another effect is, that does not even kill all mutants. For some reasons, an additional mutation is generated that mutates a constant beyond the size of the array (for varargs[3] something like "Substituted 4->5" is produced) which is not killable - a "Off-by-one Super-Mutant" - a true nightmare in computer science (naming wise and off-by-one wise :).
The option is to search for alternate methods to use. For example the SonarQube plugin API provides a way to add Extensions class by class, see MutationAnalysisPlugin.
Instead of
context.addExtensions(Class1.class, Class2.class, Class3.class);Write
context.addExtension(Class1.class);
context.addExtension(Class2.class);
context.addExtension(Class3.class);and test it with
assertTrue(context.getExtensions().contains(Class1.class));Another "trick" we applied is use utility methods that produce an array without using compiler generated inline consts.
For the sensors, the languages and rules repositories have to be specified using a vararg:
descriptor.onlyOnLanguages("java", "kotlin");Replacing this with a an array onlyOnLanguages(new String[]{"java", "kotlin"}) doesn't the kill the Off-by-one Super Mutant ("Substituted 2->3").
So we sought for a way to create the array without using varargs or array-initializers. Therefore we created a helper method toArray that accepted fixed number of argument, creates a list from it and used the Stream API to stream/collect the list into an array.
descriptor.onlyOnLanguages(toArray("java", "kotlin"));
//...
private String[] toArray(String element1, String element2) {
final List<String> list = new ArrayList<>();
list.add(element1);
list.add(element2);
return list.stream().toArray(String[]::new);
}Calling toArray(new String[0]) directly on the list would introduce another mutation on the 1 argument, which again is equivalent as the toArray only uses the array to the get the type of the array.
An example of this can be found in PitestSensor
Another type of mutant that is impossible to kill occurs on enums that use a constructor with parameters and the parameter is assigned to an enum field.
enum Examples {
ONE("1"),
TWO("2"),
;
String number;
Example(String number){
this.number = number; //this assignment gets removed by Pitest
}
public String getNumber(){
return number;
}
}I'm not entirely sure about how the JVM deals with it internally, but I assume as enum are immutable they maybe get inlined and any change on the byte code have effect on the code once used.
But enums have an alternative way of providing values through accessor methods and that is by defining the method abstract and implement on every literal. This has no unkillable mutants, but somewhat bloats the code.
For example (the above enum)
enum Examples {
ONE {
@Override public String getNumber(){
return "1";
}
},
TWO {
@Override public String getNumber(){
return "2";
}
},
;
public abstract String getNumber();
}Some code has killable mutants, but the mutants are effectively equivalent.
For example the class java.util.stream.StreamSupprt has a method stream(Spliterator, boolean) that may be used to transfor an Iterator to a stream. The boolean flag indicates whether the stream is parallel (true) or sequential (false)
Unless the iterator is not thread safe, both options are effectively equivalent (except from performance perspective in some cases).
- Any "downgrade" from parallel to sequential is equivalent
- Any "upgrade" from sequential to parallel is equivalent if the Iterator / the underlying datastructure is thread safe
A solution we used is to extract the creation of Streams from an Iterable using StreamSupport in a utility class and write a dedicated test for this class, that tests for paralell/sequential Streams.
Example
public static <T> Stream<T> sequentialStream(Iterable<T> elements){
return StreamSupport.stream(elements.spliterator(), false);
}
public static <T> Stream<T> parallelStream(Iterable<T> elements){
return StreamSupport.stream(elements.spliterator(), true);
}and test it with
Stream<String> stream = sequentialStream(Arrays.asList("one", "two"));
assertFalse(stream.isParallel());
Stream<String> stream = parallelStream(Arrays.asList("one", "two"));
assertTrue(stream.isParallel());see Streams