3
\$\begingroup\$

I'm not experienced with testing, therefore I'd like you to look at my code proposal of unit test. My solution has following projects

  • Tests\Application.Tests
  • Application
  • Domain
  • Infrastructure
  • Web

And I am not sure with Application Service Test. The tested class is DeliveryDateService

namespace Application.Services
{
    public class DeliveryDateService : IDeliveryDateService
    {
        private readonly IHttpContextAccessor _httpAccessor;
        private readonly IDeliveryDateRepository _repository;

        public DeliveryDateService(IHttpContextAccessor httpAccessor, IDeliveryDateRepository repository)
        {
            _httpAccessor = httpAccessor;
            _repository = repository;
        }

        public void Handle(Import cmd)
        {
            var deliveryDates = new List<DeliveryDate>();
            foreach (DeliveryDateDto item in cmd.Items)
            {
                deliveryDates.Add(new DeliveryDate(
                    id: null,
                    customerNumber: item.CustomerNumber,
                    date: item.Date,
                    modifiedBy: _httpAccessor.HttpContext.User.Identity.Name,
                    modifiedOn: DateTime.Now
                    ));
            }
            _repository.Import(deliveryDates);
        }
    }
} 

The real repository use entity framework to save data into database. But for the test the idea is that I will use Fake Repository, which will contains data only in memory. Therefore I am not testing the repositories, but only the service itself.

namespace Application.Tests.Fakes
{
    public class DeliveryDateRepositoryFake : IDeliveryDateRepository
    {
        private List<DeliveryDate> Dates = new List<DeliveryDate>();

        public List<DeliveryDate> FindAll()
        {
            return Dates;
        }

        public void Import(IList<DeliveryDate> dates)
        {
            Truncate();
            foreach (var date in dates)
            {
                Dates.Add(date);
            }
        }

        public void Truncate()
        {
            foreach (var date in Dates)
            {
                Dates.Remove(date);
            }
        }
    }
}

And the test looks like

namespace Application.Tests.Services
{
    public class DeliveryDateServiceTests
    {
        private DeliveryDateRepositoryFake _repository;
        private IDeliveryDateService _service;

        private void Setup()
        {
            _repository = new DeliveryDateRepositoryFake();

            _service = new DeliveryDateService(
                MockHelper.MockIHttpContextAccessor().Object,
                _repository);
        }

        private void Cleanup()
        {
            _repository = null;
            _service = null;
        }

        [Fact]
        public void Import()
        {
            Setup();

            var items = new List<DeliveryDateDto>
            {
                new DeliveryDateDto(customerNumber: 123456, date: DateTime.Now),
                new DeliveryDateDto(customerNumber: 123457, date: DateTime.Now)
            };

            _service.Handle(new Commands.DeliveryDate.Import(items));

            Assert.Equal(
                expected: items.Count, 
                actual: _repository.FindAll().Count);

            Cleanup();
        }
    }
}

Does it make sense, can it be improved ? I read, that Setup and Cleanup methods are not ideal, but also I don't want to repeat the code. Also I have to write Fake Repository for each Real Repository, it's extra work, but it's not complicated yet and it looks like it might work well.

Assertions (updated)

I added some assertions also on items data

// Assertions

var actualCustomerOne = items.ToArray()[0];
var actualCustomerTwo = items.ToArray()[1];
var expectedCustomerOne = _repository.FindAll().ToArray()[0];
var expectedCustomerTwo = _repository.FindAll().ToArray()[1];

Assert.Equal(items.Count, _repository.FindAll().Count);

Assert.Equal(actualCustomerOne.CustomerNumber, expectedCustomerOne.CustomerNumber);
Assert.Equal(actualCustomerOne.Date, expectedCustomerOne.Date);
Assert.NotNull(expectedCustomerOne.ModifiedBy);
Assert.NotEqual(DateTime.MinValue, expectedCustomerOne.ModifiedOn);

Assert.Equal(actualCustomerTwo.CustomerNumber, expectedCustomerTwo.CustomerNumber);
Assert.Equal(actualCustomerTwo.Date, expectedCustomerTwo.Date);
Assert.NotNull(expectedCustomerTwo.ModifiedBy);
Assert.NotEqual(DateTime.MinValue, expectedCustomerTwo.ModifiedOn);
\$\endgroup\$
0
\$\begingroup\$

Fake Repository Responsibility

If the fake repository is meant to only provide testability of the service, then I would say you have hidden the true responsibility of the fake too much.

It comes to what you want to know about the repository during the test? I would say you need to know how many times the Import method was called and what was the argument of each call.

What you have there seems more like a (yet simple) real in-memory implementation of the repository. The fake has its own list of items and adds items from all calls to Import to the list, making it impossible to see which items were to be added in which call (not that you have shown us a test for multiple executions).

I would expect the fake to have about this structure:

class DeliveryDateRepositoryFake : IDeliveryDateRepository
{
  public int ImportCalled {get;};
  public IList<DeliveryDate> LastImportedDates {get;};

  // if you wanna keep track of all calls
  public IEnumerable<IList<DeliveryDates>> AllImportedDates{get;}
}

The implementation could be using List<DeliveryDate> property, or a list of such lists if you wanna keep track of all consecutive calls.

Too Many Fakes

If you are worried about having to implement a fake repository for every entity. Then you should have been worried if you have to implement a real repository for every entity.

What I mean is if you had a template interface for all repositories, maybe you might need just one template implementation of the fake.

Setup and Cleanup

You are repeating some code anyway (Setup(); and Cleanup();). You could also have just one factory method, returning both the service and the respository as a tuple, destroying them automatically on the end of the test. And also removing need for properties definition and the "nulling" cleanup code.

Assertions

I'm not sure if you are just showing us the test in its simplests form or you are really just testing the counts of items. Anyway, I suggest you also test the items data, not just their count, because picking the data from command and passing them to the repository is responsibility of the service and thus should be subject of this test.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for great answer ! :-) yes, the fake repository is there only for testing purpose. I was thinking about implementing generic repository (and specific repositories only when needed), but for the simplicity I decided to implement each repository for each aggregate/entity for now. I will see how it will go. I will also look into factory, good idea. I added new assertions to test items data, it makes sense to test items data too. \$\endgroup\$
    – Muflix
    Jan 3 '20 at 9:29

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.