8
\$\begingroup\$

I have read on numerous occasions that tests should not be coupled to implementation details - this makes perfect sense.

Now I am having some trouble decoupling my tests from implementation details and am kindly asking for some help.

The relevant portion of the repository that I am testing looks like this:

public class PostsRepository : IPostsRepository
{
    private readonly DatabaseContext context;

    public PostsRepository(DatabaseContext context)
    {
        this.context = context;
    }

    public Post Find(string slug)
    {
        return context
            .Posts
            .SingleOrDefault(post => post.Slug == slug);
    }

    public bool Delete(string slug)
    {
        var post = Find(slug);

        if (post == null) return false;

        context.Posts.Remove(post);
        return true;
    }
}

The context is mocked using the technique described here and then manually injected into the repository.

The corresponding tests that I am concerned about look like this:

[Test]
public void Delete_ExistentPost_DeletesPost()
{
    databaseSet.SetupSeedData(new List<Post>
    {
        PostsMother.CreatePost(withSlug: "abc")
    });

    repository.Delete("abc");

    databaseSet.Verify(d => d.Remove(It.IsAny<Post>()));
}

[Test]
public void Delete_NonExistentPost_DoesNotThrow()
{
    repository.Delete("abc");
}

[Test]
public void Delete_ExistentPost_ReturnsTrue()
{
    databaseSet.SetupSeedData(new List<Post>
    {
        PostsMother.CreatePost(withSlug: "abc")
    });

    var actual = repository.Delete("abc");
    Assert.True(actual);
}

[Test]
public void Delete_NonExistentPost_ReturnsFalse()
{
    var actual = repository.Delete("abc");
    Assert.False(actual);
}

These tests seem to work well enough but I am concerned about their brittleness. If you look at the implementation of the Delete method you will see that it uses the Remove method but what if that changed? The test would break.

Furthermore (and more importantly), the test is highly dependent on the Delete method using the Find method as the Find method uses the Posts property of the database set which is stubbed according to the seed data configured in the arrange phase of the test (using SetUpSeedData).

It is perfectly conceivable that I will adjust the implementation of the Delete method to catch an exception and return the boolean according to the exception instead of using the Find method.

How can I improve the resilience of my tests in this case?

\$\endgroup\$
3
\$\begingroup\$

Without trying to sound tautological, the tests in your original question are coupled to your implementation because that's exactly what they are testing.

You're testing that -

  • Remove is called on a database abstraction
  • The method doesn't throw an exception for an invalid slug
  • The method returns true when it finds something to delete
  • The method returns false when it does not find something to delete

These are all quite low level concepts, are any of them important?

As a developer that's new to a code base, I look to the tests to tell me what's important and needs to be preserved. Most of the time, this boils down to behaviour that the user can observe and interact with.

If you re-focus your tests so that they ensure -

  • When there are Posts in the repository, a user can view them
  • When a Post is deleted, it is no longer listed on an index
  • When a Post is deleted, it can no longer be viewed

Then you will not be coupling them to such details as "my repositories return false when they can't find something to delete".

\$\endgroup\$
0
\$\begingroup\$

The only other approach that I can conceive is to fake the database context and the database set instead of mocking them using the approach described here.

Doing so will enable me to write a test like this:

[Test]
public void Delete_DeletesPost()
{
    var context = new FakeDatabaseContext();
    var repository = new PostsRepository(context);
    var controller = new AdminController(repository);
    context.Posts.Add(new Post { Slug = "abc"});

    controller.Delete("abc");

    Assert.AreEqual(0, context.Posts.Count());
}

It occurred to me though - this test is no more resilient than the last.

While at a glance this test looks as though it is not at all coupled to implementation details it is. The test is coupled to the implementation indirectly via the manual fake.

I am virtually ready to conclude (but not really) that you cannot make the tests more resilient in the way that I described in the original post.

I think that when you write low-level unit tests like this and you need to emulate an ORM you have no choice but to have at least some coupling to implementation details.

But this answer by no means authoritative.

These are just my (probably naive) opinions at this time. I am going to proceed to use mocks as they require less overhead and aim to learn the hard way. If I develop any sound opinions about either approach I will document them here.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.