andymaster01 andymaster01 - 2 months ago 13
Java Question

Approach to test public methods depending on private methods

I'm trying to add tests to a legacy code, and as I start to adding code, I get the feeling that something is wrong.

In the following code, the public method RegisterChange is calling two private methods to:


  1. Get the object to store

  2. Store the object



public class ChangeService {

IRepository repository;

public ChangeService(IRepository repository){
this.repository = repository;
}

public bool RegisterChange( int entityId ){
var entity = GetParsedEntity( entityId );
SaveEntity( entity );
return true;
}

private Entity GetParsedEntity( int id ) {
var entity = repository.GetEntityById( id );
return new Entity{ Name = entity.Name };
}

private void SaveEntity( Entity entity ) {
repository.Save( Entity );
}
}

public class ChangeServiceFact(){

[Fact]
public void When_valid_entity__Should_save_entity(){

var mock = new Mock<IRepository>();
var service = new ChangeService(mock.object);

var result = service.RegisterChange( 0 );

Assert.True(result);
}
}


So, when Im mocking the repository, I had to go and check the private method's code to know which operations to mock.

The problem that I'm seeing with this approach is that, because the code is testing not only the test subject (the public method) but also the private methods, is not clear which should be the test result by looking at the test subject (public method).

In the case that, later on, someone decide to modify one private method (like throwing an exception from GetParsedEntity), the test will continue to pass correctly, but the client code could fail because of this change.

In this particular case, Im using C#, XUnit and Moq, but I think is more a general testing question.

Answer

The problem that I'm seeing with this approach is that, because the code is testing not only the test subject (the public method) but also the private methods, is not clear which should be the test result by looking at the test subject (public method).

The test subject you mention has no visible effect without knowing its full contract. What the full contract here is? The mentioned public method and constructor, which takes dependency. It's the dependency that's important here and interaction with this dependency is what should be tested. Private methods are (as always) implementation detail - irrelevant to unit testing.

Having said that, let's get back to the contract. What is the actual contract of test subject (ChangeService method)? To retrieve object from repository basing on some id, create different object and save the later in the same repository. And this is your test.

[Fact]
public void ChangeService_StoresNewEntityInRepository_BasedOnProvidedId()
{
    const string ExpectedName = "some name";
    var otherEntity = new OtherEntity { Name = ExpectedName };
    var mock = new Mock<IRepository>();
    var service = new ChangeService(mock.object);
    mock.Setup(m => m.GetEntityById(0)).Return(otherEntity);

    service.RegisterChange(0);

    mock.Verify(m => m.SaveEntity(It.Is<Entity>(e => e.Name == ExpectedName));
}