0

I want to test the behavior of a private method. The method "moveDataToArchive" does 4 steps.

It's 4x: calculate a date + call a sub method.

This is my test:

@Test
    public void testMoveData2Archive() throws Exception{

    final long now = 123456789000L;

    //Necessary to make the archivingBean runable.
    Vector<LogEntry> logCollector = new Vector<LogEntry>();
    Deencapsulation.setField(archivingBean, "logCollector", logCollector);

    new NonStrictExpectations(archivingBean) {
        {       //Lets fake the DB stuff.
            invoke(archivingBean, "getConnection");result = connection;
            connection.prepareStatement(anyString); result = prepStatement;
            prepStatement.executeUpdate(); returns(Integer.valueOf(3), Integer.valueOf(0), Integer.valueOf(3));
        }
    };

    new NonStrictExpectations(props) {
        {    //This is important. The numbers will be used for one of each 4 submethods
            props.getProperty(ArchivingHandlerBean.ARCHIVING_CREDMATURITY_OVER_IN_DAYS); result = "160";
            props.getProperty(ArchivingHandlerBean.ARCHIVING_CREDHIST_AGE_IN_DAYS); result = "150";
            props.getProperty(ArchivingHandlerBean.ARCHIVING_DEBTHIST_AGE_IN_DAYS); result = "140";
            props.getProperty(ArchivingHandlerBean.ARCHIVING_LOG_AGE_IN_DAYS); result = "130";
        }
    };

    new Expectations() {
        {
            Date expected = new Date(now - (160 * 24 * 60 * 60 * 1000));
            invoke(archivingBean, "moveCreditBasic2Archive", expected);

            expected = new Date(now - (150 * 24 * 60 * 60 * 1000));
            invoke(archivingBean, "moveCreditHistory2Archive", expected);

            expected = new Date(now - (999 * 24 * 60 * 60 * 1000));
            invoke(archivingBean, "moveDebtorHistory2Archive", expected);

            expected = new Date(now - (130 * 24 * 60 * 60 * 1000));
            invoke(archivingBean, "moveLog2Archive", expected);

        }
    };

    Calendar cal = Calendar.getInstance();
    cal.setTimeInMillis(now);
    Deencapsulation.invoke(archivingBean,"moveDataToArchive",cal, props);
}

Whats the problem? See the third expected date. It is wrong! (999 instead of 140). I also changed the order of the calls. I even made those private method public and tried it. All those changes did not change the outcome: Test is green.

What is wrong here? Why is the test green?

JoseK
  • 31,141
  • 14
  • 104
  • 131
KFleischer
  • 942
  • 2
  • 11
  • 33

2 Answers2

0

The test is misusing the mocking API, by mixing strict and non-strict expectations for the same mock (archivingBean). The first expectations recorded on this mock are non-strict, so JMockit regards it as a non-strict mock for the whole test.

The correct way to write the test would be to turn the strict expectation block (the one with the 4 calls to "sub methods") into a verification block at the end of the test.

(As an aside, the whole test has several problems. 1) In general, private methods should be tested indirectly, through some public method. 2) Also, private methods should not be mocked, unless there is a strong reason otherwise - in this case, I would probably write a test which verifies the actual contents of the output file. 3) Don't mock things unnecessarily, such as props - props.setProperty could be used instead, I suppose. 4) Use auto-boxing - Integer.valueOf(3) -> 3).

Rogério
  • 16,171
  • 2
  • 50
  • 63
0

@Rogério: Your assumptions do not work completely. i.e. I don't have a setProperty(). What I tried is to use a Verifications-Block.

Sadly I dont understand JMockit good enough to get it running...

I did 2 things. First I tried to mock the 4 private methods. I only want to see if they are called. But I don't want there logic to run. I tried it by extending the first NonStrictExpectations-Block like this:

 new NonStrictExpectations(archivingBean) {
        {
            invoke(archivingBean, "getConnection");result = connection;
            connection.prepareStatement(anyString); result = prepStatement;
            prepStatement.executeUpdate(); returns(Integer.valueOf(3), Integer.valueOf(0), Integer.valueOf(3));
            //New part
            invoke(archivingBean, "moveCreditBasic2Archive", withAny(new Date()));
            invoke(archivingBean, "moveCreditHistory2Archive", withAny(new Date()));
            invoke(archivingBean, "moveDebtorHistory2Archive", withAny(new Date()));
            invoke(archivingBean, "moveLog2Archive", withAny(new Date()));
        }
    };

On the other hand I moved the Expectations-Block down and made it a verifications Block. Now the JUnit fails with a

mockit.internal.MissingInvocation: Missing invocation of:
de.lpm.ejb.archiving.ArchivingHandlerBean#moveCreditBasic2Archive(java.util.Date pOlderThan)
with arguments: Tue Feb 03 03:39:51 CET 2009
on mock instance: de.lpm.ejb.archiving.ArchivingHandlerBean@1601bde
at de.lpm.ejb.archiving.ArchivingHandlerBean.moveCreditBasic2Archive(ArchivingHandlerBean.java:175)
[...]
Caused by: Missing invocation

This is Line 170-175 in ArchivingHandlerBean.java:

170: Connection connection = getConnection();
171: SQLService service = new SQLService(connection);
172:            
173: PreparedStatement prepStmtMove = null;
174:            
175: Vector<HashMap<String, String>> where_clauses = new Vector<HashMap<String,String>>();

I just want to verify that the 4 private methods are executed with the right date.

KFleischer
  • 942
  • 2
  • 11
  • 33
  • Yes, my answer wasn't totally correct because this test is doing *partial* mocking. So, in this case the expectations need to be recorded first so that JMockit knows *not* to execute the original method implementations. A verification block could still be written later, to verify that the invocations occurred as expected, but this would mean redundant code in the test. In the end, mocking is simply not a good choice for this test, due to the need for calling/mocking private methods, and the use of partial mocking. Personally, I would use a completely different approach. – Rogério Jun 26 '13 at 14:25
  • Could you describe how you would test this situation? – KFleischer Jun 26 '13 at 14:28
  • I would use one of two possible alternative approaches: 1) write a black box test with external inputs ("props", etc.) and outputs (the written archive file contents) only, no mocking; or 2) write a true unit test which verifies the `archivingBean` behavior throuh its interaction with separate units (new classes, extracted from the private methods called from `archivingBean.moveDataToArchive`). – Rogério Jun 26 '13 at 15:41