"Replaced constant value of 31 with 32 : KILLED" but "Substituted 31 with 32 : SURVIVED"

[hcoles]

From esko.luo...@gmail.com on December 13, 2011 19:59:31

I enabled all mutations and I'm having some confusing test results between INLINE_CONSTS and EXPERIMENTAL_INLINE_CONSTS. Could one of them have a bug? What steps will reproduce the problem? 1. Checkout the code from https://github.com/orfjackal/jumi 2. Build it with Maven: mvn clean verify -P coverage-report
3. See the report for TestId at \jumi-api\target\pit-reports What is the expected output? What do you see instead? The report shows two mutations which seemingly do the same thing, but one of them is killed and another survives:

Replaced constant value of 31 with 32 : KILLED -> fi.jumi.api.drivers.TestIdTest.hashCode_has_good_dispersion(fi.jumi.api.drivers.TestIdTest)
Substituted 31 with 32 : SURVIVED What version of the product are you using? On what operating system? pitest-maven 0.24
jdk1.6.0_24 64-bit
Windows 7 64-bit Please provide any additional information below. I've attached the pitest's HTML report and the .class file in question.

The source code of the method being mutated: https://github.com/orfjackal/jumi/blob/3c33370a5e82423401c343c23cbe977bfe8908c0/jumi-api/src/main/java/fi/jumi/api/drivers/TestId.java#L91 The test which kills the other mutation - the test is deterministic: https://github.com/orfjackal/jumi/blob/3c33370a5e82423401c343c23cbe977bfe8908c0/jumi-api/src/test/java/fi/jumi/api/drivers/TestIdTest.java#L46-67 The bytecode of the method being mutated:

public int hashCode();
Code:
0: iconst_1
1: istore_1
2: aload_0
3: astore_2
4: aload_2
5: invokevirtual #6; //Method isRoot:()Z
8: ifne 29
11: bipush 31
13: iload_1
14: imul
15: aload_2
16: invokevirtual #11; //Method getIndex:()I
19: iadd
20: istore_1
21: aload_2
22: invokevirtual #7; //Method getParent:()Lfi/jumi/api/drivers/TestId;
25: astore_2
26: goto 4
29: iload_1
30: ireturn

Attachment: fi.jumi.api.drivers.TestId_and_2_others.html TestId.class

Original issue: http://code.google.com/p/pitestrunner/issues/detail?id=17

[hcoles]

From henry.co...@googlemail.com on December 16, 2011 15:26:53

I've been able to replicate, but not yet had chance to dig into what's going on.

It certainly doesn't look correct. I'd suggest that you disable the inline_consts mutator for now - the new experimental version appears to behave correctly.

Status: Accepted

[hcoles]

From stefan.p...@googlemail.com on January 13, 2012 04:53:59

First let me explain that the expected behaviour is: When replacing 31 with 32 in TestId.hashCode() this is an equivalent mutation with respect to your tests. The expected result is that both mutations survive. One can check this by replacing 31 with 32 manually and running all tests from jumi-api manually. No test should fail.

In my opinion neither the original InlineConstantMutator nor the experimental one have a bug. The bug must be somewhere else in pit. That's what I found out.

1. I could reproduce the bug as well checking out the version of jumi as of December 13th (3c33370a5e82423401c343c23cbe977bfe8908c0)
2. But before I tried your current head and could not reproduce the bug as you reported: Both Mutants were killed!!!
3. I switched back to the older version and tried to get more information. I wrapped some tests with a try-catch block catching any throwable, printing the stacktrace to the console and rethrowing the throwable. The attached file contains the relevant stack trace: getIndex() is called on the Root-Node. Looking at the code, this can only happen, if isRoot() returns false. While playing around I noticed other exceptions as well (see second log).

It seems to me that the mutations are not propperly isolated.

Attachment: pit-log1.log pit-log2.log

[hcoles]

From stefan.p...@googlemail.com on January 13, 2012 04:56:57

Additional note: Changing the order of mutations in the configuration (pom.xml) does change the result. I tried to switch the experimental mutator and the regular one and suddenly both mutations were killed.

[hcoles]

From henry.co...@googlemail.com on January 13, 2012 12:41:26

A little worrying. Out of interest, if Root is changed to no longer be a singleton can the problem still be demonstrated?

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 13:12:23

Interesting bug. I get the same results as your logs. I see that right before "Running mutation hashCode : 91 -> Replaced constant value of 31 with 32 (14)" it did "Running mutation : 155 -> changed conditional boundary (0)", which would explain the exception "java.lang.IllegalArgumentException: illegal index: 0". Somehow the latter execution sees also the former mutation.

I did an experiment to find out that which class loader is being used. I added the following code to the aforementioned test:

    Class<?> c1 = TestId.class;
    System.err.println(c1);
    System.err.println(c1.getClassLoader());

    Class<?> c2 = TestId.of(100).getClass();
    System.err.println(c2);
    System.err.println(c2.getClassLoader());

    Class<?> c3 = ClassLoader.getSystemClassLoader().loadClass("fi.jumi.api.drivers.TestId");
    System.err.println(c3);
    System.err.println("c1.equals(c3) = " + c1.equals(c3));

Every time that the test printed something, it printed the same class loader instance (see below), which according to http://en.wikipedia.org/wiki/Java_Classloader is the system class loader.

class fi.jumi.api.drivers.TestId
sun.misc.Launcher$AppClassLoader@553f5d07
class fi.jumi.api.drivers.TestId$Child
sun.misc.Launcher$AppClassLoader@553f5d07
class fi.jumi.api.drivers.TestId
c1.equals(c3) = true

So it appears that all the classes can be found from the system class loader (do you give the classpath as JVM parameter?) and since by default a child class loader will look for classes from the parent class loader first, the same class will be everywhere. PIT is anyways mutating the classes (otherwise it would not nothing), but the classes are not reset between tests.

How are you handling the class loaders? Do you discard the class loader between every mutation? Are you making sure that none of the project's classes are accidentally loaded from the parent class loader?

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 14:01:53

Yes, the same thing happens when Root is not a singleton. I can also reproduce it with the latest 0.25-SNAPSHOT.

I think I have an idea where the problem might be. Every time that a test fails because of a mutation other than the current mutation, it is because of a previous mutation IN A DIFFERENT CLASS. For example (with latest Jumi and pitest-maven), it happens like this:

1. PIT mutates TestId$Root with "Running mutation isRoot : 130 -> Substituted 1 with 0 (0)"
2. PIT does NOT mutate TestId$Root (lines 126-148) before step Dependency analysis broken #3.
3. PIT mutates TestId with "Running mutation hashCode : 91 -> Substituted 31 with 32 (14)", but it fails because of step 4 unit tests failing when trying to compile the code #1's mutation: "java.lang.UnsupportedOperationException: root has no index" (The code tries to call Root.getIndex() because due to the mutation Root.isRoot() returns false.)

So it appears that when PIT does a mutation, it loads the unmodified bytecode of that class from disk and mutates it, thus undoing all previous mutations TO THE SAME CLASS. But it does NOT undo mutations to a class AFTER ITS TESTS HAVE BEEN RUN and it's time to mutate next some other class.

[hcoles]

From henry.co...@googlemail.com on January 13, 2012 14:05:15

PIT does not use classloaders to insert the mutations - it uses the instrumentation api (well almost, for mutations in static initializers it does use classloaders as otherwise the mutations would never be triggered).

It looks like there might be some issues with the current approach, possibly revealed when the class is a singleton.

An alternative approach I've been considering is to use mutant schemata - basically turning mutations on and off using a flag which might perform better (or might not). If it does turn out that there is an issue with the current approach the solution will probably be to switch to this scheme.

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 14:24:35

Here is a test to reproduce it. If any of Bar's mutations is run after Foo's mutations, then that Bar mutation fails for the wrong reason, because the test checks Foo first.

Expected: A mutation of fooMethod should result in "java.lang.AssertionError: found mutation in Foo", and a mutation of barMethod should result in "java.lang.AssertionError: found mutation in Bar".

Actual: But in log.txt you can see that in one instance "Running mutation barMethod : 13 -> replaced return of integer sized value with (x == 0 ? 1 : 0) (0)" resulted in "java.lang.AssertionError: found mutation in Foo".

Attachment: log.txt MutationTest.java Mutation.java

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 14:34:26

BTW, how are you handling isolation when executing the mutations with multiple threads, if you don't use multiple class loaders? One JVM per thread?

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 14:36:17

Or do you do only one mutation at a time, and use the threads for just running the tests in parallel?

[hcoles]

From henry.co...@googlemail.com on January 13, 2012 14:58:36

Thanks for the test case. I'm slightly hampered right now by not quite asleep children, but I think your analysis is about right, but I'm guessing that this is only revealed with nested classes? i.e. if Foo and Bar were two top level classes the issue is not revealed?

In answer to your question, there is one JVM per thread.

[hcoles]

From esko.luo...@gmail.com on January 13, 2012 15:06:54

Yes, it only happens for nested classes. Top-level classes are properly isolated.

[hcoles]

From henry.co...@googlemail.com on January 13, 2012 15:30:55

Ok, thanks again for looking into this. I think the fix ought to be reasonably straightforward, just need to swap the original class back in when the mutator moves from on from one nested class to the next.

[hcoles]

From henry.co...@googlemail.com on January 13, 2012 16:17:37

Ok, a slightly hacky fix is now in the latest 0.25 snapshot.

[hcoles]

From henry.co...@googlemail.com on January 20, 2012 05:06:45

Status: Fixed