try with resources block generates unkillable mutants

[zygoth]

If you use a try-with-resources block in your Java code and run PITest on it, you get about 8 MUTANT SURVIVED messages on the closing brace of your try block. This is quite frustrating as it can drive down the overall mutant coverage percentage by a lot. This seems like an obvious bug. The messages all look like one or the other of these:

removed call to com/blah/MyClassExtendsCloseable::close \u2192 SURVIVED
removed call to java/lang/Throwable::addSuppressed \u2192 SURVIVED

[StefanPenndorf]

Could you provide a more detailed example and a full report for that class?

I'm not sure whether those mutations are really "unkillable": You might have to simulate throwing an exception from the close() method directly and after an exception has already been thrown in the try-block and examine the suppressed exception(s) available via Throwable.getSuppressed().

The try-with-resources block is just a handy shortcut the compiler provides. Taken from http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html the code

try (AutoClose autoClose = new AutoClose()) {
    autoClose.work();
}

is just a shortcut for

       AutoClose autoClose = new AutoClose();
       MyException myException = null;
       try {
           autoClose.work();
       } catch (MyException e) {
           myException = e;
           throw e;
       } finally {
           if (myException != null) {
               try {
                   autoClose.close();
               } catch (Throwable t) {
                   myException.addSuppressed(t);
               }
           } else {
               autoClose.close();
           }
       }

The survived mutations are just the calls removed from the finally block I guess. You've to check that logic as well to kill those mutants.

[zygoth]

Ok, but there is no reason for me to test auto-generated code, this stuff
should be exempt from mutations.

On Tue, Apr 19, 2016 at 1:26 PM, Stefan Penndorf notifications@github.com
wrote:

Could you provide a more detailed example and a full report for that class?

I'm not sure whether those mutations are really "unkillable": You might
have to simulate throwing an exception from the close() method directly and
after an exception has already been thrown in the try-block and examine the
suppressed exception(s) available via Throwable.getSuppressed().

The try-with-resources block is just a handy shortcut the compiler
provides. Taken from
http://www.oracle.com/technetwork/articles/java/trywithresources-401775.html
the code

try (AutoClose autoClose = new AutoClose()) {
autoClose.work();
}

is just a shortcut for

   AutoClose autoClose = new AutoClose();
   MyException myException = null;
   try {
       autoClose.work();
   } catch (MyException e) {
       myException = e;
       throw e;
   } finally {
       if (myException != null) {
           try {
               autoClose.close();
           } catch (Throwable t) {
               myException.addSuppressed(t);
           }
       } else {
           autoClose.close();
       }
   }

The survived mutations are just the calls removed from the finally block I
guess. You've to check that logic as well to kill those mutants.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#255 (comment)

[tobijdc]

PITEST works on the bytecode level.
I assume there is no easy way to distinguish between the two examples on bytecode level.

[StefanPenndorf]

@zygoth

Nevertheless this "auto-generated" code ships with some semantics and changes the behaviour of your method. I don't agree that this is auto-generated code which shouldn't be tested. If you don't need the behaviour or if you don't rely on that behaviour just don't use try-with-resources.

If you use try-with-ressources you SHOULD check at least:

• that your stream is closed on success
• that your stream is closed if an exception is thrown inside the try-block
• that an exception thrown inside the try-block gets propagated and will not be shadowed by an exception that occurs during close
• and maybe if you opt for 100% coverage you need to check that an exception thrown in close() will be added as suppressedException of the original exception thrown inside the try-block.

The last thing is the only "feature" that feels like an optional behaviour because it does not change the behaviour of your code if you don't query for suppressed exceptions.

---

As @tobijdc correctly mentioned those instructions might be really hard to detect because they will be introduced by the compiler and I'm not sure whether we can distinguish between manual implementation and compiler "expansion"

[christianhujer]

I have the same problem. Using try-with-resources creates unkillable mutants. It is pointless to test whether try-with-resources works. The whole point of try-with-resources is to prevent mistakes. Testing whether try-with-resources works makes test code unnecessarily complicated. How would you test something like this:

package com.nelkinda.epub;

import org.w3c.dom.DOMImplementation;
import org.w3c.dom.Document;
import org.w3c.dom.bootstrap.DOMImplementationRegistry;
import org.w3c.dom.ls.DOMImplementationLS;
import org.w3c.dom.ls.LSInput;
import org.w3c.dom.ls.LSParser;

import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;

import static org.w3c.dom.ls.DOMImplementationLS.MODE_SYNCHRONOUS;

enum XML {
    ;

    @SuppressWarnings("unchecked")
    static <DOMImpl extends DOMImplementation & DOMImplementationLS> DOMImpl getDOMImpl() throws InstantiationException, IllegalAccessException, ClassNotFoundException {
        return (DOMImpl) DOMImplementationRegistry.newInstance().getDOMImplementation("LS");
    }

    static Document readXML(final String path) throws IllegalAccessException, InstantiationException, ClassNotFoundException, IOException {
        try (final InputStream in = new FileInputStream(path)) {
            return readXML(in);
        }
    }

    static <DOMImpl extends DOMImplementation & DOMImplementationLS> Document readXML(final InputStream in) throws IllegalAccessException, ClassNotFoundException, InstantiationException {
        final DOMImpl domImpl = getDOMImpl();
        final LSInput lsInput = domImpl.createLSInput();
        lsInput.setByteStream(in);
        final LSParser lsParser = domImpl.createLSParser(MODE_SYNCHRONOUS, null);
        return lsParser.parse(lsInput);
    }
}

In particular, the readXML(final String path) method. I could test this by turning my Utility class into a real class, which would require that I replace static dependencies with injected dependencies (urgs!), then I could replace the new FileInputStream(String) constructor with a Function<String,InputStream> and inject a Mock which verifies that close() was called. Is it worth it? No, especially when the side-effects on architecture are damaging, as they are in this case.

[christianhujer]

As far as I understand, this should be discovered by the TryWithResourcesFilter. Somehow it seems that it is currently not working.
Setup:

• Kubuntu 18.04.3 LTS
• OpenJDK 13.0.1
• Maven 3.6.3
• Pitest 1.4.10

I'll share more details once I've created an isolated test case.

[pbludov]

@hcoles Does #860 fix this issue? Our build is green after upgrading Pitest to 1.6.3.

[hcoles]

@pbludov possibly. The bytecode that is generated for try with resources varies by compiler (java, eclipse, others) and by java version.

The change in #860 is confirmed to work with java eclipse, javac8 and javac11. There may be issues with other compilers or java versions not tested for, and it might be possible to trip it up with more complex code than has been tested for.

Guess it's a question for @zygoth about whether they still see this problem.

[Manfred73]

I have some classes that also use try-with-resources and see the following:

Running with maven 3.8.6 and java:
openjdk version "17.0.4" 2022-07-19
OpenJDK Runtime Environment Temurin-17.0.4+8 (build 17.0.4+8)
OpenJDK 64-Bit Server VM Temurin-17.0.4+8 (build 17.0.4+8, mixed mode, sharing)

[Manfred73]

Is there some way to ignore these try-with-resource branches?

[hcoles]

@Manfred73 What version of pitest are you using? Using temurin-17.0.5 (17.0.4 doesn't look to be available via sdkman) I do not see these mutants for roughly equivalent code for pitest 1.11.0.

The only way to ignore them is to improve pitest's filtering, or use the exclusions functionality from arcmutate.

https://docs.arcmutate.com/docs/exclusions.html

If you can create a minimal project that reproduces this issue, I can look at improving the filtering.