1

I have several Spring beans/components implementing AutoCloseable, and I expect the Spring Container to close them when the application context is destroyed.

Anyway, my code coverage tool is complaining because the close() methods are "uncovered by tests" from its point of view.

What should I do:

  1. Introduce some trivial close() tests?
  2. Just let it go and accept they will lower the coverage %
  3. Something else?
alessiop86
  • 1,285
  • 2
  • 19
  • 38
  • Seeing your code provides better answers for this question. But I have a suggestion for you, you can write some test functions for `close()` method includes try/catch blocks. I don't know about `AutoCloseable` but I hope it works. – stuck Aug 08 '16 at 07:54

1 Answers1

1

You're not testing that the app closes the bean, you're testing that the bean closes properly when closed. If the implementation is non-trivial, then you should write a test for that behaviour. If all your method does is call close on a single field then don't bother testing it. However, if your close method calls close on multiple fields or does something a bit more complicated then you should test it.

For instance, given the following Closer class that must close all its Readers when it is closed...

public class Closer implements AutoCloseable {

    private Reader[] readers;

    public Closer(Reader... readers) {
        this.readers = readers;
    }

    @Override
    public void close() {
        try {
            for (Reader reader : readers) {
                reader.close();
            }
        } catch (IOException ex) {
            // ignore
        }
    }
}

You may wish to test as such:

public class CloserTest {

    @Test
    public void allReadersClosedWhenOneReaderThrowsException() {
        // given
        Reader badReader = mock(Reader.class);
        Reader secondReader = mock(Reader.class);
        doThrow(new IOException()).when(badReader).close();
        Closer closer = new Closer(badReader, secondReader);

        // when
        closer.close();

        // then
        verify(badReader).close();
        verify(secondReader).close(); // fails as loop stops on first exception
    }
}

Excessively high code coverage can be a bad thing if it means that your unit tests contain large amounts of trivial tests, especially if those tests are fragile. They will increase the amount of effort required to maintain the unit tests, without actually adding anything.

Dunes
  • 37,291
  • 7
  • 81
  • 97
  • the `close` implementation is trivial, that is the point. It is just closing Apache HttpClient instances for each of the `AutoCloseable` beans – alessiop86 Aug 08 '16 at 08:29
  • 1
    If they HttpClient instances are beans, then let Spring manage their lifecycle and get rid of your close method. If you manually create the instances, then you need a close method, and you need to test it closes everything properly. – Dunes Aug 08 '16 at 08:39