2

I am using JUNIT5, have been trying to fully coverage a piece of code that involves System.getenv(""); I writed a couple classes to replicate what I am experiencing right now and so you can use them to understand me also (minimal reproducible example):

First we have the service I need to get with full coverage (ServiceToTest.class) (it has a CustomContainer object which contains methods that it needs):

@Service
public class ServiceToTest {

    private final CustomContainer customContainer;

    public ServiceToTest() {
        Object configuration = new Object();
        String envWord = System.getenv("envword");
        this.customContainer = new CustomContainer(configuration, envWord == null ? "default" : envWord);

    }

    public String getContainerName() {
        return customContainer.getContainerName();
    }

}

CustomContainer.class:

public class CustomContainer {
    @Getter
    String containerName;
    Object configuration;

    public CustomContainer(Object configuration, String containerName) {
        this.configuration = configuration;
        this.containerName = containerName;
    }

}

I have tried using ReflectionTestUtils to set the envWord variable without success... I tried this https://stackoverflow.com/a/496849/12085680, also tried using @SystemStubsExtension https://stackoverflow.com/a/64892484/12085680, and finally I also tried using Spy like in this answer https://stackoverflow.com/a/31029944/12085680

But the problem is that this variable is inside the constructor so this only executes once and I think that it happens before any of this configs I tried before can apply, here is my test class:

@ExtendWith(MockitoExtension.class)
class TestService {
    // I have to mock this becase in real project it has methods which I need mocked behavour
    private static CustomContainer mockCustomContainer = mock(CustomContainer.class);
    // The serviceToTest class in which I use ReflectionTestUtils to use the mock above
    // Here is where the constructor gets called and it happens BEFORE (debuged) the setup method
    // which is anotated with @BeforeAll
    private static ServiceToTest serviceToTest = new ServiceToTest();

    @BeforeAll
    static void setup() {
        // set the field customContainer at serviceToTest class to mockCustomContainer
        ReflectionTestUtils.setField(serviceToTest, "customContainer", mockCustomContainer);
    }

    @Test
    void testGetContainerNameNotNull() {
        assertNull(serviceToTest.getContainerName());
    }

}

I need to write a test in which serviceToTest.getContainerName is not null but the real purpose of this is to have coverage of this sentence envWord == null ? "default" : envWord so it would be a test that is capable of executing the constructor and mocking System.getenv() so that it returns not null...

Right now the coverage looks like this and I can not find a way to make it 100% Any ideas??

coverage

EDIT: So after following tgdavies suggestion, the code can be 100% covered, so this is the way:

Interface CustomContainerFactory:

public interface CustomContainerFactory {
    CustomContainer create(Object configuration, String name);
}

CustomContainerFactoryImpl:

@Service
public class CustomContainerFactoryImpl implements CustomContainerFactory {

    @Override
    public CustomContainer create(Object configuration, String name) {
        return new CustomContainer(configuration, name);
    }

}

EnvironmentAccessor Interface:

public interface EnvironmentAccessor {
    String getEnv(String name);
}

EnvironmentAccessorImpl:

@Service
public class EnvironmentAccessorImpl implements EnvironmentAccessor {

    @Override
    public String getEnv(String name) {
        return System.getenv(name);
    }

}

Class ServiceToTest after refactoring:

@Service
public class ServiceToTest {

    private final CustomContainer customContainer;
    
    public ServiceToTest(EnvironmentAccessor environmentAccessor, CustomContainerFactory customContainerFactory) {
        Object configuration = new Object();
        String envWord = environmentAccessor.getEnv("anything");
        this.customContainer = customContainerFactory.create(configuration, envWord == null ? "default" : envWord);

    }

    public String getContainerName() {
        return customContainer.getContainerName();
    }

}

Finally the test case after refactoring (here is were I think it can be improved maybe?):

@ExtendWith(MockitoExtension.class)
class TestService {

    private static CustomContainer mockCustomContainer = mock(CustomContainer.class);
    private static CustomContainerFactory customContainerFactoryMock = mock(CustomContainerFactoryImpl.class);
    private static EnvironmentAccessor environmentAccessorMock = mock(EnvironmentAccessorImpl.class);

    private static ServiceToTest serviceToTest;

    @BeforeAll
    static void setup() {
        when(environmentAccessorMock.getEnv(anyString())).thenReturn("hi");
        serviceToTest = new ServiceToTest(environmentAccessorMock, customContainerFactoryMock);
        ReflectionTestUtils.setField(serviceToTest, "customContainer", mockCustomContainer);
        when(serviceToTest.getContainerName()).thenReturn("hi");
    }

    @Test
    void testGetContainerNameNotNull() {
        assertNotNull(serviceToTest.getContainerName());
    }

    @Test
    void coverNullReturnFromGetEnv() {
        when(environmentAccessorMock.getEnv(anyString())).thenReturn(null);
        assertAll(() -> new ServiceToTest(environmentAccessorMock, customContainerFactoryMock));
    }

}

Now the coverage is 100%:

coverage100

EDIT 2:

We can improve the test class and get the same 100% coverage like so:

@ExtendWith(MockitoExtension.class)
class TestService {

    private static CustomContainer mockCustomContainer = mock(CustomContainer.class);
    private static IContainerFactory customContainerFactoryMock = mock(ContainerFactoryImpl.class);
    private static IEnvironmentAccessor environmentAccessorMock = mock(EnvironmentAccessorImpl.class);

    private static ServiceToTest serviceToTest;

    @BeforeAll
    static void setup() {
        when(environmentAccessorMock.getEnv(anyString())).thenReturn("hi");
        when(customContainerFactoryMock.create(any(), anyString())).thenReturn(mockCustomContainer);
        serviceToTest = new ServiceToTest(environmentAccessorMock, customContainerFactoryMock);

    }

    @Test
    void testGetContainerNameNotNull() {
        assertNotNull(serviceToTest.getContainerName());
    }

    @Test
    void coverNullReturnFromGetEnv() {
        when(environmentAccessorMock.getEnv(anyString())).thenReturn(null);
        assertAll(() -> new ServiceToTest(environmentAccessorMock, customContainerFactoryMock));
    }

}
BugsOverflow
  • 386
  • 3
  • 19
  • If what you're trying to do is just ensure each branch is covered then what about refactoring the valueOrDefault check in line 13 into a utility method and testing that separately ... then the line 13 will be 100% covered. Of course if what you really want is to test what happens with the different versions of custom container in the service then you will need to have 2 different tests or configurations in the test.. – Mr R Nov 06 '22 at 01:59
  • *How* is but the form following the function of *why*. So *why* do you want 100% of coverage? – Turing85 Nov 06 '22 at 02:00
  • Create another component for accessing environment variables, which you can mock. – tgdavies Nov 06 '22 at 02:03
  • @tgdavies Could you show in an answer how can it be achieved? But I wonder if I do that, this new component will not be 100% covered? Because this System.getenv() will now go into it – BugsOverflow Nov 06 '22 at 02:05
  • @Turing85 I need it because I need a % of coverage in order to be able to submit merge request and also why not have coverage of something – BugsOverflow Nov 06 '22 at 02:07
  • "Coverage of something" doesn't sound like a good metric for code quality. Coverage of relevant business logic sounds much more sensible. – Turing85 Nov 06 '22 at 02:09
  • @MrR I am sorry I did not really understand the suggestion, if I execute the tests setting the environment variable then it will not be null, but now the branch of it being null will not be covered :/ – BugsOverflow Nov 06 '22 at 02:13
  • @BugsOverflow - you can use the idea suggested by tgdavies answer, or if it doesn't actually matter what the specific value is - then move the valueOrDefault selection code into a new static method you can tests to 100% (both ways - branch coverage), and then you get 100% method coverage in the constructor because there is no longer a branch in the constructor. – Mr R Nov 06 '22 at 02:18
  • 1
    `when(serviceToTest.getContainerName()).thenReturn("hi");` does nothing, because `serviceToTest` is not a mock. You want to mock that method on `mockCustomContainer`. You don't need to use `ReflectionTestUtils` -- get `customContainerFactoryMock` to return `mockCustomContainer`. – tgdavies Nov 06 '22 at 07:00

1 Answers1

2

Refactor your code to make it testable, by moving object creation and static method calls to components, which you can mock in your tests:

interface ContainerFactory {
    CustomContainer create(Object configuration, String name);
}

interface EnvironmentAccessor {
    String getEnv(String name);
}

@Service
public class ServiceToTest {

    private final CustomContainer customContainer;

    public ServiceToTest(ContainerFactory containerFactory, EnvironmentAccessor environmentAccessor) {
        Object configuration = new Object();
        String envWord = environmentAccessor.getEnv("envword");
        this.customContainer = containerFactory.create(configuration, envWord == null ? "default" : envWord);
    }

    public String getContainerName() {
        return customContainer.getContainerName();
    }
}
tgdavies
  • 10,307
  • 4
  • 35
  • 40
  • I will try this, appreciate it, but I have a question, this will make the EnvironmentAccesor implementation miss the branch also? Because I will move the System.getenv() to it, and now I need to coverage this implementation also – BugsOverflow Nov 06 '22 at 02:16
  • 1
    There's no branch in `EnvironmentAccessorImpl.getEnv`. – tgdavies Nov 06 '22 at 02:18
  • Hey, would appreciate if you could add how would the Test class look like, because I manage to make it work this way that you suggested, but im not sure if it was suppose to be made like I did it... I will share in a few moments in the EDIT of the question (with also how the Impl of the interfaces was made) – BugsOverflow Nov 06 '22 at 05:47
  • Review my question edit now pls – BugsOverflow Nov 06 '22 at 05:55