Initial DSL mutator concept #221
Conversation
velo
force-pushed
the
dsl
branch
2 times, most recently
from
b66eb19
to
2ec6b61
Compare
|
|
||
|
|
||
| @Test | ||
| @Ignore |
There was a problem hiding this comment.
FWIW, my bytecode manipulation knowledge is too limited to get this one working, but I appreciate any help!
|
@hcoles |
|
@UrsMetz any ideas? |
|
@velo good catch: the reason why Concerning |
|
Yeah, I hit a dead end for this as well.... I notice the method will be called on parent and after that CAST to the invoking type. We would need to check for that, them pop the |
|
@velo today I tried to tackle the problem with subtyping: I played around with an approach inspired by http://stackoverflow.com/a/14723212. But I think the problem is that removing method calls having a return type that is a sub or super type of the receiver can produce invalid byte code. E.g. mutating (when Bar bar = new Foo().bar()to Bar bar = new Foo()would lead to an byte code validation error (marked blue in the HTML report). |
|
I found it helps a lot to have the original bytecode and your expected bytecode side by side on beyond compare =D |
|
Just need to pop invokeVirtual out... |
|
@velo: here is an example where my experiments led to an error by ASM saying "Incompatible return type" (the mutation consisted in removing the call to class Bar {
public Baz baz() {
return new Baz();
}
}
class Baz extends Bar {
}
class Caller implements Callable<Baz> {
public Baz call() throws Exception {
return new Bar().baz();
}
}So I think without more context of what the type of the "value" (variable or return value of a method) is there is no reliable way produce valid byte code when subtyping should also be taken in consideration. From what I gather @hcoles tends avoid mutation in PIT that lead to invalid byte code (cf. discussion in #139 or also #218). Am I seeing this right, @hcoles? Update: added |
|
But you can't remove you are breaking the method signature, like: |
|
Also, thinking o a builder for instance... doesn't make sense to remove the |
@velo That was actually my point (my comment was a bit unclear, sorry): removing the call to |
|
The example you pasted there is no subtypes... |
|
@velo you're right: I forgot the |
|
Still, if you remove baz() it does not compile! Unless you make That works like a charm |
That's actually the point I'm trying to make: we cannot remove calls to methods that return a different type than the type of the receiver (like removing the call to |
|
We can remove when subtyping... you removed when "supertyping" =D |
|
@velo depends how you define it :-). For me it is "sub typing" because the method we want to remove returns a subtype (in my example |
|
Baz is instance of Bar You could do what you suggest if |
|
@velo I think I have to clear up my point: If you take code like: class Bar {
public Baz baz() {
return new Baz();
}
}
class Baz extends Bar {
}
class Caller implements Callable<Baz> {
public Baz call() throws Exception {
return new Bar().baz();
}
}When a mutation that removes instance methods that return a type inheriting the type of the receiver is applied the resulting code is not valid (as we both agreed). For that reason I think we can't make the mutation do that (or at least only an optional feature). |
|
thinking on DSL I see 2 scenarios: You can see, the call to one type I just don't know how to turn that into code. |
|
Closing old pull requests w/o activity |
Also on users lists:
https://groups.google.com/forum/#!topic/pitusers/o4Vpb_KI9Fc
I'm looking into mutating DSLs, for instance camel DSL:
So I wanna mutations to remove DSL steps to make sure all my configurations (or at least most) are meaningful and being tested somewhere. So that piece of code would be mutated into something like:
AND/OR
I also wanna test @builders and queries DSL.
So pitest would remove query conditions or builder parameters.
I started creating my own mutator for it (which I will submit as a pull request once properly tested)
Right now, it does remove any method that return the same type as the class that declares it.
https://github.com/hcoles/pitest/pull/221/files#diff-abcacb866451ca350bef8b81ef634c9bR35
It is not merge ready, it is just to illustrate what I'm doing.
But when I do that, dsl.chain(1) become to have the same effect as dsl = null would be
https://github.com/hcoles/pitest/pull/221/files#diff-0f0887a036c22255aa52f202e494fe58R90
And a NPE happen:
I think I can find the problem by myself if is there any way to get ASMifier running on a class after mutated, is that possible?
Or, if what I'm doing wrong is obvious, can someone point out?
Is there anything obviously wrong that you guys see on my code?
Keep in mind this is just my initial work, as I master what need to be done I will fix checkstyle and make the design as similar as possible to what is already there.
Also, is this functionality something that worth incorporating to pitest?
Marvin