8 branches for try with resources - jacoco coverag

问题:

I've got some code that uses try with resources and in jacoco it's coming up as only half covered. All the source code lines are green, but I get a little yellow symbol telling me that only 4 of 8 branches are covered.

I'm having trouble figuring out what all the branches are, and how to write code that covers them. Three possible places throw PipelineException. These are createStageList(), processItem() and the implied close()

1. Not throwing any exceptions,
2. throwing an exception from createStageList()
3. throwing an exception from processItem()
4. throwing an exception from close()
5. throwing an exception from processItem() and close()

I can't think of any other cases, yet I still only have 4 of 8 covered.

Can someone explain to me why it's 4 of 8 and is there anyway to hit all 8 branches? I'm not skilled with decyrpting/reading/interpreting byte code, but maybe you are... :) I've already seen https://github.com/jacoco/jacoco/issues/82, but neither it nor the issue it references help very much (other than noting that this is due to compiler generated blocks)

Hmm, just as I finish writing this I had a thought on what case(s) might not be not tested by what I mention above... I'll post an answer if I got it right. I'm sure this question and it's answer will help someone in any case.

EDIT: Nope, I didn't find it. Throwing RuntimeExceptions (not handled by the catch block) didn't cover any more branches

回答1:

Well I can't tell you what the exact problem with Jacoco is, but I can show you how Try With Resources is compiled. Basically, there are a lot of compiler generated switches to handle exceptions thrown at various points.

If we take the following code and compile it

public static void main(String[] args){
    String a = "before";

    try (CharArrayWriter br = new CharArrayWriter()) {
        br.writeTo(null);
    } catch (IOException e){
        System.out.println(e.getMessage());
    }

    String a2 = "after";
}

And then disassemble, we get

.method static public main : ([Ljava/lang/String;)V
    .limit stack 2
    .limit locals 7
    .catch java/lang/Throwable from L26 to L30 using L33
    .catch java/lang/Throwable from L13 to L18 using L51
    .catch [0] from L13 to L18 using L59
    .catch java/lang/Throwable from L69 to L73 using L76
    .catch [0] from L51 to L61 using L59
    .catch java/io/IOException from L3 to L94 using L97
    ldc 'before'
    astore_1
L3:
    new java/io/CharArrayWriter
    dup
    invokespecial java/io/CharArrayWriter <init> ()V
    astore_2
    aconst_null
    astore_3
L13:
    aload_2
    aconst_null
    invokevirtual java/io/CharArrayWriter writeTo (Ljava/io/Writer;)V
L18:
    aload_2
    ifnull L94
    aload_3
    ifnull L44
L26:
    aload_2
    invokevirtual java/io/CharArrayWriter close ()V
L30:
    goto L94
L33:
.stack full
    locals Object [Ljava/lang/String; Object java/lang/String Object java/io/CharArrayWriter Object java/lang/Throwable
    stack Object java/lang/Throwable
.end stack
    astore 4
    aload_3
    aload 4
    invokevirtual java/lang/Throwable addSuppressed (Ljava/lang/Throwable;)V
    goto L94
L44:
.stack same
    aload_2
    invokevirtual java/io/CharArrayWriter close ()V
    goto L94
L51:
.stack same_locals_1_stack_item
    stack Object java/lang/Throwable
.end stack
    astore 4
    aload 4
    astore_3
    aload 4
    athrow
L59:
.stack same_locals_1_stack_item
    stack Object java/lang/Throwable
.end stack
    astore 5
L61:
    aload_2
    ifnull L91
    aload_3
    ifnull L87
L69:
    aload_2
    invokevirtual java/io/CharArrayWriter close ()V
L73:
    goto L91
L76:
.stack full
    locals Object [Ljava/lang/String; Object java/lang/String Object java/io/CharArrayWriter Object java/lang/Throwable Top Object java/lang/Throwable
    stack Object java/lang/Throwable
.end stack
    astore 6
    aload_3
    aload 6
    invokevirtual java/lang/Throwable addSuppressed (Ljava/lang/Throwable;)V
    goto L91
L87:
.stack same
    aload_2
    invokevirtual java/io/CharArrayWriter close ()V
L91:
.stack same
    aload 5
    athrow
L94:
.stack full
    locals Object [Ljava/lang/String; Object java/lang/String
    stack 
.end stack
    goto L108
L97:
.stack same_locals_1_stack_item
    stack Object java/io/IOException
.end stack
    astore_2
    getstatic java/lang/System out Ljava/io/PrintStream;
    aload_2
    invokevirtual java/io/IOException getMessage ()Ljava/lang/String;
    invokevirtual java/io/PrintStream println (Ljava/lang/String;)V
L108:
.stack same
    ldc 'after'
    astore_2
    return
.end method

For those who don't speak bytecode, this is roughly equivalent to the following pseudo Java. I had to use gotos because the bytecode doesn't really correspond to Java control flow.

As you can see, there are a lot of cases to handle the various possibilities of suppressed exceptions. It's not reasonable to be able to cover all these cases. In fact, the goto L59 branch on the first try block is impossible to reach, since the first catch Throwable will catch all exceptions.

try{
    CharArrayWriter br = new CharArrayWriter();
    Throwable x = null;

    try{
        br.writeTo(null);
    } catch (Throwable t) {goto L51;}
    catch (Throwable t) {goto L59;}

    if (br != null) {
        if (x != null) {
            try{
                br.close();
            } catch (Throwable t) {
                x.addSuppressed(t);
            }
        } else {br.close();}
    }
    break;

    try{
        L51:
        x = t;
        throw t;

        L59:
        Throwable t2 = t;
    } catch (Throwable t) {goto L59;}

    if (br != null) {
        if (x != null) {
            try{
                br.close();
            } catch (Throwable t){
                x.addSuppressed(t);
            }
        } else {br.close();}
    }
    throw t2;
} catch (IOException e) {
    System.out.println(e)
}

回答2:

I can cover all 8 branches, so my answer is YES. Look at the following code, this is only a fast try, but it works (or see my github: https://github.com/bachoreczm/basicjava and the 'trywithresources' package, there you can find, how try-with-resources works, see 'ExplanationOfTryWithResources' class):

import java.io.ByteArrayInputStream;
import java.io.IOException;

import org.junit.Test;

public class TestAutoClosable {

  private boolean isIsNull = false;
  private boolean logicThrowsEx = false;
  private boolean closeThrowsEx = false;
  private boolean getIsThrowsEx = false;

  private void autoClose() throws Throwable {
    try (AutoCloseable is = getIs()) {
        doSomething();
    } catch (Throwable t) {
        System.err.println(t);
    }
  }

  @Test
  public void test() throws Throwable {
    try {
      getIsThrowsEx = true;
      autoClose();
    } catch (Throwable ex) {
      getIsThrowsEx = false;
    }
  }

  @Test
  public void everythingOk() throws Throwable {
    autoClose();
  }

  @Test
  public void logicThrowsException() {
    try {
      logicThrowsEx = true;
      everythingOk();
    } catch (Throwable ex) {
      logicThrowsEx = false;
    }
  }

  @Test
  public void isIsNull() throws Throwable {
    isIsNull = true;
    everythingOk();
    isIsNull = false;
  }

  @Test
  public void closeThrow() {
    try {
      closeThrowsEx = true;
      logicThrowsEx = true;
      everythingOk();
      closeThrowsEx = false;
    } catch (Throwable ex) {
    }
  }

  @Test
  public void test2() throws Throwable {
    try {
      isIsNull = true;
      logicThrowsEx = true;
      everythingOk();
    } catch (Throwable ex) {
      isIsNull = false;
      logicThrowsEx = false;
    }
  }

  private void doSomething() throws IOException {
    if (logicThrowsEx) {
      throw new IOException();
    }
  }

  private AutoCloseable getIs() throws IOException {
    if (getIsThrowsEx) {
      throw new IOException();
    }
    if (closeThrowsEx) {
      return new ByteArrayInputStream("".getBytes()) {

        @Override
        public void close() throws IOException {
          throw new IOException();
        }
      };
    }
    if (!isIsNull) {
      return new ByteArrayInputStream("".getBytes());
    }
    return null;
  }
}

回答3:

No real question, but wanted to throw more research out there. tl;dr = It looks like you can achieve 100% coverage for try-finally, but not for try-with-resource.

Understandably, there's a difference between old-school try-finally and Java7 try-with-resources. Here's two equivalent examples showing the same thing using alternate approaches.

Old School example (a try-finally approach):

final Statement stmt = conn.createStatement();
try {
    foo();
    if (stmt != null) {
        stmt.execute("SELECT 1");
    }
} finally {
    if (stmt != null)
        stmt.close();
}

Java7 example (a try-with-resource approach):

try (final Statement stmt = conn.createStatement()) {
    foo();
    if (stmt != null) {
        stmt.execute("SELECT 1");
    }
}

Analysis: old-school example:
Using Jacoco 0.7.4.201502262128 and JDK 1.8.0_45, I was able to get 100% line, instruction and branch coverage on the Old School example using the following 4 tests:

• Basic grease path (statement not null, and execute() is exercised normally)
• execute() throws exception
• foo() throws exception AND statement returned as null
• statement returned as null

Jacoco indicates 2 branches inside the 'try' (on the null check) and 4 inside the finally (on the null check). All are covered fully.

Analysis: java-7 example:
If the same 4 tests run against the Java7 style example, jacoco indicates 6/8 branches are covered (on the try itself) and 2/2 on the null-check within the try. I tried a number of additional tests to increase coverage, but I can find no way to get better than 6/8. As others have indicated, the decompiled code (which I did also look at) for the java-7 example suggests that the java compiler is generating unreachable segments for try-with-resource. Jacoco is reporting (accurately) that such segments exist.

Update: Using Java7 coding style, you might be able to get 100% coverage IF using a Java7 JRE (see Matyas response below). However, using Java7 coding style with a Java8 JRE, I believe you will hit the 6/8 branches covered. Same code, just different JRE. Seems like the byte code is being created differently between the two JREs with the Java8 one creating unreachable pathways.

回答4:

Four years old, but still...

1. Happy path with non-null AutoCloseable
2. Happy path with null AutoCloseable
3. Throws on write
4. Throws on close
5. Throws on write and close
6. Throws in resource specification (the with part, e.g. constructor call)
7. Throws in try block but AutoCloseable is null

Above lists all 7 conditions - the reason for the 8 branches is due to repeated condition.

All branches can be reached, the try-with-resources is fairly simple compiler sugar (at least compared to switch-on-string) - if they cannot be reached, then it is by definition a compiler bug.

Only 6 unit tests are actually required (in the example code below, throwsOnClose is @Ingored and branch coverage is 8/8.

Also note that Throwable.addSuppressed(Throwable) cannot suppress itself, so the generated bytecode contains an additional guard (IF_ACMPEQ - reference equality) to prevent this). Luckily this branch is covered by the throw-on-write, throw-on-close and throw-on-write-and-close cases, as the bytecode variable slots are reused by the outer 2 of 3 exception handler regions.

This is not an issue with Jacoco - in fact the example code in the linked issue #82 is incorrect as there are no duplicated null checks and there is no nested catch block surrounding the close.

JUnit test demonstrating 8 of 8 branches covered

import static org.hamcrest.Matchers.arrayContaining;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.sameInstance;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

import java.io.IOException;
import java.io.OutputStream;
import java.io.UncheckedIOException;

import org.junit.Ignore;
import org.junit.Test;

public class FullBranchCoverageOnTryWithResourcesTest {

    private static class DummyOutputStream extends OutputStream {

        private final IOException thrownOnWrite;
        private final IOException thrownOnClose;


        public DummyOutputStream(IOException thrownOnWrite, IOException thrownOnClose)
        {
            this.thrownOnWrite = thrownOnWrite;
            this.thrownOnClose = thrownOnClose;
        }


        @Override
        public void write(int b) throws IOException
        {
            if(thrownOnWrite != null) {
                throw thrownOnWrite;
            }
        }


        @Override
        public void close() throws IOException
        {
            if(thrownOnClose != null) {
                throw thrownOnClose;
            }
        }
    }

    private static class Subject {

        private OutputStream closeable;
        private IOException exception;


        public Subject(OutputStream closeable)
        {
            this.closeable = closeable;
        }


        public Subject(IOException exception)
        {
            this.exception = exception;
        }


        public void scrutinize(String text)
        {
            try(OutputStream closeable = create()) {
                process(closeable);
            } catch(IOException e) {
                throw new UncheckedIOException(e);
            }
        }


        protected void process(OutputStream closeable) throws IOException
        {
            if(closeable != null) {
                closeable.write(1);
            }
        }


        protected OutputStream create() throws IOException
        {
            if(exception != null) {
                throw exception;
            }
            return closeable;
        }
    }

    private final IOException onWrite = new IOException("Two writes don't make a left");
    private final IOException onClose = new IOException("Sorry Dave, we're open 24/7");


    /**
     * Covers one branch
     */
    @Test
    public void happyPath()
    {
        Subject subject = new Subject(new DummyOutputStream(null, null));

        subject.scrutinize("text");
    }


    /**
     * Covers one branch
     */
    @Test
    public void happyPathWithNullCloseable()
    {
        Subject subject = new Subject((OutputStream) null);

        subject.scrutinize("text");
    }


    /**
     * Covers one branch
     */
    @Test
    public void throwsOnCreateResource()
    {
        IOException chuck = new IOException("oom?");
        Subject subject = new Subject(chuck);
        try {
            subject.scrutinize("text");
            fail();
        } catch(UncheckedIOException e) {
            assertThat(e.getCause(), is(sameInstance(chuck)));
        }
    }


    /**
     * Covers three branches
     */
    @Test
    public void throwsOnWrite()
    {
        Subject subject = new Subject(new DummyOutputStream(onWrite, null));
        try {
            subject.scrutinize("text");
            fail();
        } catch(UncheckedIOException e) {
            assertThat(e.getCause(), is(sameInstance(onWrite)));
        }
    }


    /**
     * Covers one branch - Not needed for coverage if you have the other tests
     */
    @Ignore
    @Test
    public void throwsOnClose()
    {
        Subject subject = new Subject(new DummyOutputStream(null, onClose));
        try {
            subject.scrutinize("text");
            fail();
        } catch(UncheckedIOException e) {
            assertThat(e.getCause(), is(sameInstance(onClose)));
        }
    }


    /**
     * Covers two branches
     */
    @SuppressWarnings("unchecked")
    @Test
    public void throwsOnWriteAndClose()
    {
        Subject subject = new Subject(new DummyOutputStream(onWrite, onClose));
        try {
            subject.scrutinize("text");
            fail();
        } catch(UncheckedIOException e) {
            assertThat(e.getCause(), is(sameInstance(onWrite)));
            assertThat(e.getCause().getSuppressed(), is(arrayContaining(sameInstance(onClose))));
        }
    }


    /**
     * Covers three branches
     */
    @Test
    public void throwsInTryBlockButCloseableIsNull() throws Exception
    {
        IOException chucked = new IOException("ta-da");
        Subject subject = new Subject((OutputStream) null) {
            @Override
            protected void process(OutputStream closeable) throws IOException
            {
                throw chucked;
            }
        };

        try {
            subject.scrutinize("text");
            fail();
        } catch(UncheckedIOException e) {
            assertThat(e.getCause(), is(sameInstance(chucked)));
        }

    }
}

Caveat

Though not in OP's sample code, there is one case that can't be tested AFAIK.

If you pass the resource reference as an argument, then in Java 7/8 you must have a local variable to assign to:

    void someMethod(AutoCloseable arg)
    {
        try(AutoCloseable pfft = arg) {
            //...
        }
    }

In this case the generated code will still be guarding the resource reference. The syntatic sugar is updated in Java 9, where the local variable is no longer required: try(arg){ /*...*/ }

Supplemental - Suggest use of library to avoid branches entirely

Admittedly some of these branches can be written off as unrealistic - i.e. where the try block uses the AutoCloseable without null checking or where the resource reference (with) cannot be null.

Often your application doesn't care where it failed - to open the file, write to it or close it - the granularity of failure is irrelevant (unless the app is specifically concerned with files, e.g. file-browser or word processor).

Furthermore, in the OP's code, to test the null closeable path - you'd have to refactor the try block into a protected method, subclass and provide a NOOP implementation - all this just get coverage on branches that will never be taken in the wild.

I wrote a tiny Java 8 library io.earcam.unexceptional (in Maven Central) that deals with most checked exception boilerplate.

Relevant to this question: it provides a bunch of zero-branch, one-liners for AutoCloseables, converting checked exceptions to unchecked.

Example: Free Port Finder

int port = Closing.closeAfterApplying(ServerSocket::new, 0, ServerSocket::getLocalPort);

回答5:

Jacoco has recently fixed this issue, Release 0.8.0 (2018/01/02)

"During creation of reports various compiler generated artifacts are filtered out, which otherwise require unnecessary and sometimes impossible tricks to not have partial or missed coverage:

• Part of bytecode for try-with-resources statements (GitHub #500)."

http://www.jacoco.org/jacoco/trunk/doc/changes.html

回答6:

i had a similar issue with something like this:

try {
...
} finally {
 if (a && b) {
  ...
 }
}

it complained that 2 of 8 branches weren't covered. ended up doing this:

try {
...
} finally {
 ab(a,b);
}

void ab(a, b) {
 if (a && b) {
...
 }
}

no other changes and i now reached 100%....