HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Do I unit test the correct thing

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thecorrecttestthingunit

Problem

Thats my method to test:

public LessonplannerRequest GetWeeklyLessonplanner(int schoolyearId)  
        {            
            Schoolyear schoolyear = _unitOfWork.SchoolyearRepository.GetById(schoolyearId);
            DayOfWeek firstDayOfWeek = schoolyear.FirstDayOfWeek;

            DateTime currentDate = DateTime.Now.Date;
            DateTime startDateOfWeek = _dateService.GetFirstDateOfWeek(currentDate, firstDayOfWeek);
            DateTime endDateOfWeek = _dateService.GetLastDateOfWeek(currentDate, firstDayOfWeek);

            var periods = _unitOfWork.PeriodRepository.GetWeeklyPeriods(schoolyearId, startDateOfWeek, endDateOfWeek );
            var weeks = _dateService.GetAllWeekStartingDays(startDateOfWeek, endDateOfWeek, firstDayOfWeek);
            var request = new LessonplannerRequest
            {
                 Periods = periods,
                 Weeks = weeks,
            };

            return request;
        }


Thats my unit test:

```
// Arrange
Mock unitOfWorkMock = new Mock();
Mock dateServiceMock = new Mock();
Mock contextMock = new Mock();
Mock> schoolyearRepositoryMock = new Mock>();
Mock lessonplannerRepoMock = new Mock();

int schoolyearId = 1;
DateTime start = new DateTime(2010, 1, 1);
DateTime end = new DateTime(2010, 1, 2);

unitOfWorkMock.Setup(s => s.SchoolyearRepository).Returns(schoolyearRepositoryMock.Object);
unitOfWorkMock.Setup(s => s.PeriodRepository).Returns(lessonplannerRepoMock.Object);
dateServiceMock.Setup(s => s.GetFirstDateOfWeek(DateTime.Now.Date, DayOfWeek.Sunday)).Returns(start);
dateServiceMock.Setup(s => s.GetLastDateOfWeek(DateTime.Now.Date, DayOfWeek.Sunday)).Returns(end);
schoolyearRepositoryMock.Setup(s => s.GetById(schoolyearId)).Returns(new Schoolyear() { FirstDayOfWeek = DayOfWeek.Sunday });

ILessonplannerService service = new Les

Solution

Almost everything is perfect. Some notes:

  • Do use var to avoid repeating in cases like Mock unitOfWorkMock = new Mock()



  • Move mocks initialisation and wiring up to SetUp method to avoid copy-pasting in different test methods, and leave only actual setups that provide test data in test method



  • It's hard to unit test the code that uses DateTime.Now. For example your test may fail when running near midnight (since you call DateTime.Now several times, and subsequent calls may return the next day). I would suggest to add the UtcNow property to your IDateService and always use it instead of DateTime.Now (I prefer to force using UTC time since it forces you to think about time zones). Then, most likely you won't need methods like GetFirstDateOfWeek in IDateService (if they depend on inputs only), they could be just extension methods to DateTime.



  • You don't need to verify that GetFirstDateOfWeek and GetLastDateOfWeek were called, just make sure that result matches expected value. The reason is because the call to those methods does not guarantee that returned values were used correctly. And when all tests pass without those calls - it only shows that you should add more tests to verify that returned values were used properly



As a result you'll have something like this:

[Test]
public void WhenWeekStartOnSunday...ShouldReturnSomething
{
    const int schoolyearId = 1;
    var currentDate = new DateTime(2010, 1, 1).ToUniversalTime();
    var periods = new [] {...}; //specify test periods here

    //Arrange
    _schoolyearRepositoryMock.Setup(s => s.GetById(schoolyearId)).Returns(new Schoolyear() { FirstDayOfWeek = DayOfWeek.Sunday });       
    _dateServiceMock.Setup(s => s.UtcNow).Returns(currentDate);
    _lessonplannerRepoMock.Setup(lp => lp.GetWeeklyPeriods(schoolyearId, new DateTime(2010, 1, 1), 
            new DateTime(2010, 1, 5))) //specify the dates expected according to "currentDate" value.
        .Returns(periods);

    var service = new LessonplannerService(_unitOfWorkMock.Object, _dateServiceMock.Object);            

    // Act
    LessonplannerRequest request = service.GetWeeklyLessonplanner(schoolyearId);

    // Assert         
    Assert.That(request.Periods, Is.SameAs(periods));
    Assert.That(request.Weeks, Is.Empty); //verify here expected list of weeks according to logic of the method
}

Code Snippets

[Test]
public void WhenWeekStartOnSunday...ShouldReturnSomething
{
    const int schoolyearId = 1;
    var currentDate = new DateTime(2010, 1, 1).ToUniversalTime();
    var periods = new [] {...}; //specify test periods here

    //Arrange
    _schoolyearRepositoryMock.Setup(s => s.GetById(schoolyearId)).Returns(new Schoolyear() { FirstDayOfWeek = DayOfWeek.Sunday });       
    _dateServiceMock.Setup(s => s.UtcNow).Returns(currentDate);
    _lessonplannerRepoMock.Setup(lp => lp.GetWeeklyPeriods(schoolyearId, new DateTime(2010, 1, 1), 
            new DateTime(2010, 1, 5))) //specify the dates expected according to "currentDate" value.
        .Returns(periods);

    var service = new LessonplannerService(_unitOfWorkMock.Object, _dateServiceMock.Object);            

    // Act
    LessonplannerRequest request = service.GetWeeklyLessonplanner(schoolyearId);

    // Assert         
    Assert.That(request.Periods, Is.SameAs(periods));
    Assert.That(request.Weeks, Is.Empty); //verify here expected list of weeks according to logic of the method
}

Context

StackExchange Code Review Q#23498, answer score: 3

Revisions (0)

No revisions yet.