Unit-Test, Integration test or problem in design?

2019-07-13 09:12发布

问题:

I'm written my first unit-test and I think it is too dependent on other modules and I'm not sure whether it's because:

  • It's a complex test
  • I've actually written an integration test or
  • I have a problem in my design

I'll first say that although I have around 4 years of experience in development I never learned, nor were taught, automated testing.
I've just finished a major change in our DAL implementation, with Hibernate, and a colleague of mine suggested I write unit-tests for the new parts.
The main change was with respect to switching to the Session-per-Request pattern and the more constructive use of application transactions.
Because of the nature of the above change the unit-test begins at the point where a specific request arrives and begins a transaction and the test ends after the transaction ends and it checks whether the transaction performed the changes it was supposed to.
This one test involves initializing the following objects:

  • In-memory DB- so that there will be data to work against.
  • Initialize company logger- as method depends on it.
  • Initialize a repository designed as a singleton-it is the function's gate to the DAL, but it also stores other things so it's a big object to create.
  • Initialize the handler of requests which is also a singleton- this holds the method to be tested.

I think I've actually written an integration test, as I need to init the DB, Hibernate and the repository, but I'm not sure how I could've written it otherwise given the circumstances where the tested method uses all these objects for its action and I'm interested to see how the transaction handling performs (which is done on the tested method).

I'd appreciate all comments and thoughts and will gladly elaborate or clear things up if they are not clear enough.

Thanks,
Ittai

P.S. The HibernateSessionFactory is in-fact the commonly known HibernateUtil from the Hibernate In Action book, wrongly named for historical reasons.

public class AdminMessageRepositoryUpdaterTest {
private static WardId wardId;
private static EmployeeId employeeId;
private static WardId prevWardId;
private static EmployeeId prevEmployeeId;

@Test
public void testHandleEmployeeLoginToWard(){
    AgentEmployeesWardsEngine agentEmployeesWardsEngine = new AgentEmployeesWardsEngine();
    AgentEngine agentEngine = new AgentEngine();
    //Remove all entries from AgentEmployeesWards table
    HibernateSessionFactory.beginTransaction();
    for (Agent agent : agentEngine.findAll()){
        agentEmployeesWardsEngine.removeAgentEntries(agent.getId());
    }
    HibernateSessionFactory.commitTransaction();//no need to try catch as this is done in a controlled environment
    int i=0;
    //build expectedSet
    Set<AgentEmployeesWards> expectedMappingsToChangeSet = new HashSet<AgentEmployeesWards>();
    //Mappings which should have ward updated
    expectedMappingsToChangeSet.add(new AgentEmployeesWards(new AgentId(1).getValue(), employeeId.getValue(), prevWardId.getValue(), true, TimestampUtils.getTimestamp(), i++));
    expectedMappingsToChangeSet.add(new AgentEmployeesWards(new AgentId(2).getValue(), employeeId.getValue(), prevWardId.getValue(), true, TimestampUtils.getTimestamp(), i++));
    //Mappings which should have employee updated
    expectedMappingsToChangeSet.add(new AgentEmployeesWards(new AgentId(3).getValue(), prevEmployeeId .getValue(), wardId.getValue(), false, TimestampUtils.getTimestamp(), i++));
    expectedMappingsToChangeSet.add(new AgentEmployeesWards(new AgentId(4).getValue(), prevEmployeeId.getValue(), wardId.getValue(), false, TimestampUtils.getTimestamp(), i++));

    //Prepare clean data for persistence
    Set<AgentEmployeesWards> cleanSet = new HashSet<AgentEmployeesWards>(expectedMappingsToChangeSet);
    //Mappings which should NOT have ward updated
    cleanSet.add(new AgentEmployeesWards(new AgentId(5).getValue(), employeeId.getValue(), prevWardId.getValue(), false, TimestampUtils.getTimestamp(), i++));
    cleanSet.add(new AgentEmployeesWards(new AgentId(6).getValue(), employeeId.getValue(), prevWardId.getValue(), false, TimestampUtils.getTimestamp(), i++));
    //Mappings which should NOT have employee updated
    cleanSet.add(new AgentEmployeesWards(new AgentId(7).getValue(), prevEmployeeId .getValue(), wardId.getValue(), true, TimestampUtils.getTimestamp(), i++));
    cleanSet.add(new AgentEmployeesWards(new AgentId(8).getValue(), prevEmployeeId.getValue(), wardId.getValue(), true, TimestampUtils.getTimestamp(), i++));
    HibernateSessionFactory.beginTransaction();
    for (AgentEmployeesWards agentEmployeesWards : cleanSet){
        agentEmployeesWardsEngine.saveNewAgentEmployeesWardsEntry(agentEmployeesWards);
    }
    HibernateSessionFactory.commitTransaction();//no need to try catch as this is done in a controlled environment
    //Close the session as to neutralize first-level-cache issues
    HibernateSessionFactory.closeSession();
    //Perform the action so it can be tested
    AdminMessageReposityUpdater.getInstance().handleEmployeeLoginToWard(employeeId, wardId, TimestampUtils.getTimestamp());

    //Close the session as to neutralize first-level-cache issues
    HibernateSessionFactory.closeSession();

    //Load actualSet from DAL
    Set<AgentEmployeesWards> actualSet = new HashSet<AgentEmployeesWards>(agentEmployeesWardsEngine.findByPrimaryEmployeeId(employeeId));
    actualSet.addAll(agentEmployeesWardsEngine.findByPrimaryWardId(wardId));

    //Prepare expected
    Set<AgentEmployeesWards> expectedSet = new HashSet<AgentEmployeesWards>();
    for (AgentEmployeesWards agentEmployeesWards : expectedMappingsToChangeSet){
        //We need to copy as the wardId and employeeId are properties which comprise the equals method of the class and so 
        //they cannot be changed while in a Set
        AgentEmployeesWards agentEmployeesWardsCopy = new AgentEmployeesWards(agentEmployeesWards);
        if (agentEmployeesWardsCopy.isEmployeePrimary()){
            //If this is a employee primary we want it to be updated to the new org-unit id
            agentEmployeesWardsCopy.setWardId(wardId.getValue());
        } else {
            //Otherwise we want it to be updated to the new employee id
            agentEmployeesWardsCopy.setEmployeeId(employeeId.getValue());
        }
        expectedSet.add(agentEmployeesWardsCopy);
    }
     //Assert between actualSet and expectedSet
    // Assert actual database table match expected table
   assertEquals(expectedSet, actualSet);


}
@BeforeClass
public static void setUpBeforeClass() throws SQLException,ClassNotFoundException{
    Class.forName("org.h2.Driver");
    Connection conn = DriverManager.getConnection("jdbc:h2:mem:MyCompany", "sa", "");

    ConfigurationDAO configDAO = new ConfigurationDAO();
    HibernateSessionFactory.beginTransaction();
    configDAO.attachDirty(new Configuration("All","Log", "Level", "Info",null));
    configDAO.attachDirty(new Configuration("All","Log", "console", "True",null));
    configDAO.attachDirty(new Configuration("All","Log", "File", "False",null));

    HibernateSessionFactory.commitTransaction();
    Logger log = new Logger();
    Server.getInstance().initialize(log);
    Repository.getInstance().initialize(log);
    AdminMessageReposityUpdater.getInstance().initialize(log);

    AdminEngine adminEngine = new AdminEngine();
    EmployeeEngine employeeEngine = new EmployeeEngine();
    HibernateSessionFactory.beginTransaction();
    Ward testWard = new Ward("testWard", 1, "Sales", -1, null);
    adminEngine.addWard(testWard);
    wardId = new WardId(testWard.getId());
    Ward prevWard = new Ward("prevWard", 1, "Finance", -1, null);
    adminEngine.addWard(prevWard);
    prevWardId = new WardId(prevWard.getId());

    Employee testEmployee = new Employee("testEmployee", "test", null, "employee", "f", prevWardId.getValue(), null, false, true);
    employeeEngine.setEmployee(testEmployee);
    employeeId = new EmployeeId(testEmployee.getId());

    Employee prevEmployee = new Employee("prevEmployee", "prev", null, "employee", "f", wardId.getValue(), null, false, true);
    employeeEngine.setEmployee(prevEmployee);
    prevEmployeeId = new EmployeeId(prevEmployee.getId());
    HibernateSessionFactory.commitTransaction();
    HibernateSessionFactory.closeSession();
}
@AfterClass
public static void tearDownAfterClass(){
    AdminEngine adminEngine = new AdminEngine();
    EmployeeEngine employeeEngine = new EmployeeEngine();
    HibernateSessionFactory.beginTransaction();
    employeeEngine.removeEmployeeById(employeeId);
    employeeEngine.removeEmployeeById(prevEmployeeId);
    adminEngine.removeWardById(wardId);
    adminEngine.removeWardById(prevWardId);
    HibernateSessionFactory.commitTransaction();
    HibernateSessionFactory.closeSession();
}
}

回答1:

Yep, this definitely is an integration test. There is nothing wrong wih integration tests and they are an important part of a test strategy, but they must be limited to verifying iif the modules are properly assembled and the configuration is properly set.

If you start using them to test functionality, you will get too much of them and 2 very bad things happen :

  1. The tests become frustratingly slow

  2. Design ossification sets in too early

The latter problem is because you are now coupling your design in the integration tests, even if the modules themselves are perfectly decoupled. If you find an opportunity to refactor, chances are it will break a dozen integration tests, and either you won't find the courage, or management will prevent you from cleaning up (The "I works!!! Do not touch it" syndrome).

The solution is to unit test all parts you have written by "mocking" out the environment. There are nice frameworks to help make mock objects on the fly, I personally use EasyMock a lot. YOu then describe the interactions with the rest of the world while verifying the functionality of your methods

In the unit tests you will get now a nice detailed description of the dependencies your code is relying on. You will also spot design problems here because if you get convoluted mock behavior in the unit tests, then it means there are design issues. This is great early feedback.

It does not make sense to unit-test the infrastructure code, as it probably already has been unit-tested, and there is nothing you can do about it anyway.

Then add 1 or 2 targeted integration tests to verify all the parts work as expected, the queries return the right objects, the transactions are properly handled, etc...

This balances out the need to verify everything works when assembled, with the ability to refactor, keeping your test times short and designeing loosely coupled modules.

It does take some experience though to pull it off. I would recommend to find an experienced developer who has done this before and offer beverages in exchange for mentoring in this area. Ask a lot of questions.