Error trying to mock constructor for ProcessBuilde

问题:

I am trying to mock the constructor for ProcessBuilder. The problem is that when the constructor is called it return null.

Class code:

 public static void enable() throws IOException, InterruptedException {


        logger.info("Enable NTP server...");

        String ntpAddress = AppConfig.getInstance().getString(AppConfig.NTP_SERVER, "");
        AppConfig.getInstance().getBoolean(AppConfig.NTP_ENABLED, true);
        String enableNtp = "chkconfig ntpd on " + SEPARATOR + " service ntpd stop " + SEPARATOR + " ntpdate " + ntpAddress + " " + SEPARATOR + " service ntpd start";

        String[] commandArr = {"bash", "-c", enableNtp};

        ProcessBuilder pb = new ProcessBuilder(commandArr);
        pb.redirectErrorStream(true);
        Process proc = pb.start();
        try (BufferedReader in = new BufferedReader(new InputStreamReader(
                proc.getInputStream()))) {

            String line;
            while ((line = in.readLine()) != null) {
                logger.info(line);
            }
        } catch (IOException ex) {
            logger.log(Level.SEVERE, "Error while trying to enable NTP Server");
        }

        proc.waitFor();
        proc.destroy();
        logger.info("NTP server has been enabled");


    }

Test code:

@RunWith(PowerMockRunner.class)
@PrepareForTest({NtpServerUtil.class, ProcessBuilder.class})
public class NtpServerUtilTest extends AbstractDbTest {

    @Test
    public void testEnableNtp() throws Exception {
        ProcessBuilder pb = PowerMockito.mock(ProcessBuilder.class);
        PowerMockito.whenNew(ProcessBuilder.class).withAnyArguments().thenReturn(pb);

        NtpServerUtil.enable();
        PowerMockito.verifyNew(ProcessBuilder.class).withArguments(Matchers.anyString());
    }

}

So, when it goes for new ProcessBuilder(command), the result is null. After that, when processBuilder.start() is called an exception is thrown. I tried some methods for mocking that constructor. Any ideas, please?

回答1:

In my team it is prohibited to use PowerMock for any newly written code as it is an indicator of poorly written (untestable) code. If you take it as a rule you will normally end up with much cleaner code.

So for your case, your problem is that you are constructing a concrete new instance of ProcessBuilder, but your code does not really care if it operates on a concrete instance of this class or on an interface that defines the contract for all the methods that you need. Effectively you only use the start method, so define a corresponding interface first (it is really terrible that Java does not define it already):

public interface ProcessStarter {
    Process start() throws IOException;
}

Then add a package visible field or an argument to your method, if you do not like package visible fields for testing purposes, of the sort: Function<String[], ProcessStarter> processStarterProvider and use it in your code:

ProcessStarter starter = processStarterProvider.apply(commandArr);
Process proc = starter.start();

Finally, provide the default implementation. If you go for a field:

Function<String[], ProcessStarter> processStarterProvider = (commandArr) -> {
    ProcessBuilder pb = new ProcessBuilder(commandArr);
    pb.redirectErrorStream(true);
    return (ProcessStarter) pb::start;
};

Now you do not need any PowerMock and can do with a trivial mock!

Having had a second thought about it, even though the above approach with an interface is generally applicable and I would advise to use it throughout, in this particular case you and if you are happy to wrap your IOException into a runtime one, then you can go with a single Function<String[], Process> interface and the following default implementation:

Function<String[], Process> processProvider = (commandArr) -> {
    ProcessBuilder pb = new ProcessBuilder(commandArr);
    pb.redirectErrorStream(true);
    try {
        return pb.start();
    } catch (IOException ex) {
        throw new RuntimeException(ex);
};

It is shorter and as testable as the above one. For clarity I would probably stick to the above longer one though.

In both cases, in test you will need to come up with an instance of Process somehow, which itself also does not implement any interfaces (poor design) and so might need a similar wrapper interface as shown above. But, given an instance of Process your processStarterProvider may look like this in test:

Process mockedProcess = ...

myInstance.processStarterProvider = (commandArr) -> () -> mockedProcess;

In the case of Function<String[], Process> it is even simpler:

Process mockedProcess = ...

myInstance.processProvider = (commandArr) -> mockedProcess;

回答2:

I would try few things in this case:

1. change PowerMockito.mock to Mockito.mock (it should work normally).

2. Try whenNew passing the correct arguments (or, at least, Matchers.anyString()) as parameter.

3. What if you create NtpServerUtil as an instance and spy it?

   private ProcessBuilder processBuilder;
   private NtpServerUtil ntpServerUtil;
   
   @Before
   public void init(){
      processBuilder = Mockito.mock(ProcessBuilder.class);
      ntpServerUtil = Mockito.spy(NtpServerUtil.class);
   
      MockitoAnnotations.initMocks(this);
      PowerMockito.whenNew(ProcessBuilder.class).withArguments(Mockito.anyString()).thenReturn(processBuilder);
   }
   
   @Test
   public void testEnableNtp() throws Exception {
      ...
   
      NtpServerUtil.enable();
   
      ...
   }
   

Ps.: you forgot to init your mocks there. Maybe it is useful also.