Clean Code – Is code coverage criteria still valid or significant

4 minutes to read read

Working with several customers over the past years, one of the NFR being enforced by customers is Code Coverage, some customers push for 80% while some others ask for 90%. Even had a client which asks for 95% in older days. Also, there is significant cost and effort involved with increasing coverage compliance. And once the said coverage is achieved, its assumed the code is good and safe. Testability of code is a whole story alltogether that deserves a post separately, this post targets more of coverage part.

Offcourse 100% is something next to impossible and thats what nobody dares it to ask or enforce. And this gives a margin for 5 or 10% or even 20% and here comes the problem set of problems. Lets deep dive in, consider the following code.

using System.Linq;
using System.Threading.Tasks;

namespace ConsoleApp2
{
    public class MySmartWorkerBl : IMySmartWorkerBl
    {
        private readonly ISmartTaskRepository _smartTaskRepository;
        private readonly IExternalSystemProcessor _externalSystemProcessor;
        private readonly IRecordPrinter _recordPrinter;

        public MySmartWorkerBl(ISmartTaskRepository smartTaskRepository, IExternalSystemProcessor externalSystemProcessor, IRecordPrinter recordPrinter)
        {
            _smartTaskRepository = smartTaskRepository;
            _externalSystemProcessor = externalSystemProcessor;
            _recordPrinter = recordPrinter;
        }

        public async Task<bool> CreateAndProcessTask()
        {
            var availableSmartTasks = await _smartTaskRepository.GetAllActiveTasksAsync();
            _recordPrinter.Print("PROCESSING STARTING");
            foreach (var availableSmartTask in availableSmartTasks.OrderBy(b => b.Priority))
            {
                _recordPrinter.Print($"Record Processing with ID: {availableSmartTask.Id}");

                var taskProcessingResult = await _externalSystemProcessor.ProcessTask(availableSmartTask.Id);
                if (taskProcessingResult.Result && !taskProcessingResult.TaskFulfilled && availableSmartTask.Priority == 1)
                {
                    throw new PriorityTaskException("Unable to process priority task");
                }

                if (taskProcessingResult.Result && taskProcessingResult.TaskFulfilled)
                {
                    _recordPrinter.Print(availableSmartTask.TaskName);
                    await _externalSystemProcessor.CommitTransaction(availableSmartTask.Id);
                    availableSmartTask.IsProcessed = true;
                    await _smartTaskRepository.UpdateSmartTask(availableSmartTask);
                }
                else
                {
                    availableSmartTask.AttemptCount++;
                    await _smartTaskRepository.UpdateSmartTask(availableSmartTask);
                }
                _recordPrinter.Print($"Record Processing ended ID: {availableSmartTask.Id}");
            }
            _recordPrinter.Print("PROCESSING ENDED");

            return true;
        }
    }
}

And Lets say we have the following unit test for this class

using System.Collections.Generic;
using ConsoleApp2;
using Moq;
using NUnit.Framework;

namespace ConsoleApp.Tests
{
    internal class MySmartWorkerBlTests
    {
        private readonly IMySmartWorkerBl _subjectUnderTest;
        private readonly Mock<ISmartTaskRepository> _mockSmartTaskRepo;
        private readonly Mock<IExternalSystemProcessor> _mockExternalSystemProcessor;
        private readonly Mock<IRecordPrinter> _mockRecordPrinter;
        public MySmartWorkerBlTests()
        {

            _mockSmartTaskRepo = new Mock<ISmartTaskRepository>();
            _mockExternalSystemProcessor = new Mock<IExternalSystemProcessor>();
            _mockRecordPrinter = new Mock<IRecordPrinter>();
            _subjectUnderTest = new MySmartWorkerBl(_mockSmartTaskRepo.Object, _mockExternalSystemProcessor.Object,
                _mockRecordPrinter.Object);
        }
        [Test]
        public void TestSmartWork()
        {
            //Arrange
            _mockSmartTaskRepo.Setup(b => b.GetAllActiveTasksAsync())
                .ReturnsAsync(() => new List<SmartTaskEntity>()
                {
                    new SmartTaskEntity()
                });
            _mockExternalSystemProcessor.Setup(b => b.ProcessTask(It.IsAny<int>()))
                .ReturnsAsync(() => new ProcessingResult(){ Result = true, TaskFulfilled = true} );

            //Act
            var result = _subjectUnderTest.CreateAndProcessTask();

            //Assert
            Assert.IsTrue(true);
        }
    }
}

The code is not so smart or impressive, but lets say for an organization, it rather serves the purpose. The Organization rather enforces a limit of 80% code coverage. And the beautiful (😒) code written is well interfaced, follows principles and everything. Now checking the coverage for this specific class we get the following results.

Code coverage of the class showing 80% coverage criteria.

Now the developer has written unit tests and 80% coverage is obtained. But is it really significant. That’s a tough question to ask, but in our simple example its very simple to answer. The method as above is good 80%. The left out 20% missed an important piece. To get more insights lets check the coverage for the method.

Line by line code coverage.

The green lights on left show if the line is covered or not. And soon you will realize that although attaining the coverage compliance we missed two very specific use cases as below

if (taskProcessingResult.Result && !taskProcessingResult.TaskFulfilled && availableSmartTask.Priority == 1)
{
	throw new PriorityTaskException("Unable to process priority task");
}

else
{
	availableSmartTask.AttemptCount++;
	await _smartTaskRepository.UpdateSmartTask(availableSmartTask);
}

Well, first segment throws an exception if a priority 1 task is not fulfilled, second case updates the attempt count. Both of the cases may be very significant but are missed edge cases for a fully “coverage compliant” code.

The other possible way around is to target 100% but that is merely possible in any of the practical scenario. Also, have seen a lot of developers manually getting/setting properties just to increase the coverage, rather then actually testing any useful code.

Verdict

The solution to the above problem could be trivial or non-trivial depending upon the importance of the code, and criticality of the cases missed. And to my little knowledge there are not many (if not none) tools that can identity this in automated way, and the option left is, well, review it manually.

Secondly another approach is to target “specific” (read as functional and logical) code that adds value to business rather then just testing simple use cases which will perfect all the times (in most cases). This would not only save a lot of time of developers that would have been spent in writing test for “not so” important code, but more time could be spent (invested) on testing functional code.

This can be considered too harsh on the machines and great tools but for the moment true. Well, to consort machines, I am only a human and might have missed out some options that can resrot this automatically. Would request visitors to kindly put in comments any such useful tools.

Thats all folks for now for this. Next post in clean code series shall be soon and will be on What to Test. Always Remember, clean code is something which most people already know, but rather they dont find it important to follow or dont want to spend a little extra time which eventually saves a lot more. Happy Coding.

Leave a Reply

Your email address will not be published. Required fields are marked *