JohnN6TSM
Posted on October 5, 2022
As I discussed in Part 1 the premise of this series is a simple natural experiment: comparing 2 large codebases written by the same solo programmer before and after introduction of SOLID Design principles. PhotoDoc, the pre-intervention project, is an electronic medical record dedicated to medical forensics. Melville.PDF is a free, open-source PDF renderer for .NET. In this article I will discuss testing differences between the two projects.
Both projects use similar testing infrastructure. I write unit tests in C# using XUnit.net. I frequently use mock objects in testing, and MOQ is my tool of choice. I utilize continuous testing and coverage analysis through Rider. I do not have specific objectives for code coverage. When writing complicated algorithms, I frequently shoot for 100% coverage of the algorithm. I test simple properties inconsistently, and frequently do not test guard clauses.
I ran each project’s unit test under the coverage analyzer in Rider and considered results only from assemblies which might actually run in production. Thus, unit test assemblies, performance test harnesses, and apps designed only for developer use are not included. The production assemblies of Melville.Pdf have significantly more unit test coverage than PhotoDoc (30.3% vs 85.6%, p < 0.0001.) I was actually quite surprised to find the low test coverage for PhotoDoc, as I have for many years considered myself to be a test driven developer.
I think there are two primary reasons that Melville.PDF has significantly more test coverage, one of which is due to SOLID design, or more particularly Clean Architecture.
Lesson #1: Poorly chosen dependencies make testing hard.
One of my biggest surprises in reading Clean Architecture was the assertion that the UI should be a plugin on the periphery of the system. (Martin, Clean Architecture pg 172-173.) PhotoDoc began its life as a WPF application. (Such things were fashionable in 2007.) If my memory serves correctly, I began the PhotoDoc project by hitting File|New Project and generating a shell of an app that popped up an empty window titled “PhotoDoc.” PhotoDoc has forever been a WPF app, and since its original goal was to manipulate photos, it made sense to embed WPF imaging classes deep in the new applications data model. I now regret that decision, and despite multiple attempts, the effort that would be required to reverse such a fundamental decision have always proven to be more effort than my estimation of their worth, so I live with an awkward architecture 15 years later.
That decision makes testing hard. WPF has strong thread affinity and runs only in single-threaded apartments. Thus my 16-processor computer runs the PhotoDoc tests one at a time. My tests run much more slowly, and thus get run less frequently, than they might otherwise.
PhotoDoc uses multithreading extensively, whereas single threaded code is easier to unit test. Early on I eliminated multithreading using #fdef DEBUG statements, and for many years I could not even run the unit tests in release mode so my actual production code went out the door untested.
Depending on the UI in the PhotoDoc business logic interacts with my other mistake: flagrant violations of the Law of Demeter. PhotoDoc’s metaphor is of a “patient chart” that contains “sheets” for different kinds of data. One type of “sheet” is a folder that can contain other sheets. Early in the design I included in each sheet a reference to the folder that contained it, and the sheets would liberally walk the folder tree looking for information they found useful.
That design decision turned into a nightmare for testing. Eventually the simplest tests required constructing elaborate object model. Because many of those objects contain WPF objects, they run on STA threads, and many access system-wide resources. These system dependencies are often hidden deep in the domain model. In 2016 I made a major effort to simplify the system by segmenting objects and using dependency injection. I eventually broke the reference between sheets and their parents, but did so with a more complicated IOC configuration than I would have chosen. I did manage to cut down the 10,000 line ShellViewModel class to 1200 lines, but the 18 constructor parameters make it a real bear to construct. I require a separate inversion of control framework just for the test cases. It is more brittle that I would like, but at least it lets me do some automated testing.
In contrast, WPF is one of two, currently, front ends that plug into the Melville.PDF business objects. There are a small number of tests for the WPF functionality – and all the problems described above apply to that small subset of the test suite. The remainder of the test suite runs multithreaded. Inside the Melville.PDF business logic classes rely upon a small number of abstractions that are easy to mock, and most of the unit tests run quickly and within a very narrow scope.
Lesson #2: Integration testing can greatly improve testing effectiveness
Another reason that Melville.PDF has so much higher test coverage is extensive use of integration testing. Part of this is in the nature of the code – because Melville.Pdf is a rendering library it is trivial to generate a bitmap of any page of a PDF document. This contributed to easy integration testing,
The first element of the integration testing is a collection of reference documents. I store my collection of reference documents as a C# assembly where each class implementing the IPdfGenerator interface generates one PDF document. This allows a hierarchy of classes to concisely describe the differences between different test documents. Test programs use reflection over this assembly to get a tree of reference documents.
The comparing reader is a WPF application that will simultaneously display the generated reference PDF files in four different renderings. The first is a PDF renderer included in the Windows API, the second and third render with the two bindings of Melville.Pdf. The fourth renderer shows a tree view of the PDF objects that make up the document. The comparing reader also loads external PDF files, making it an ideal tool for investigating rendering failures. The reader can also open document using Adobe’s PDF reader as well.
The Comparing reader is essential because the PDF rendering algorithms are complicated and high fidelity to existing renderers is essential to the purpose of the code. (In other words, an effective PDF renderer needs to produce output that looks like all the other PDF renderers.) The reference document generator makes it easy to establish a library of test documents that fully exercise the renderer. The comparing reader makes it easy to ensure high-fidelity rendering of each document. Incidentally, this test mechanism must be effective, as it uncovered numerous instances where the Microsoft renderer disagrees with Adobe PDF reader on how various files should appear.
The comparing reader works while writing rendering code, but it does not prevent rendering regressions over time. The RenderingTest class in Melville.PDF.IntegrationTesting assembly will render the first page of each reference document to a PNG file using both the WPF and Skia renderers. The resulting PNG files is hashed and the hashes are stored in the codebase and checked into source control. This gives a robust set of integration tests to prevent rendering regressions. These integration tests contribute significantly to code coverage because much of the rendering code with written directly to the integration tests, without an intermediate unit test.
Lesson 3: Single Responsibility Principle allows unit testing of private methods.
Object oriented code says that classes should hide their implementations. A consumer should only depend on the public members of a class, and everything not meant for public consumption should be private to allow the class to change in the future. Unfortunately making things private often works against unit tests which, absent messy reflection hacks, can only call or inspect public members.
Say we have a class this code:
class Sut
{
private int field;
public void A(byte[] inputData) => field = B(TrickyComputation(inputData));
private int B(double computedValue) => //Something worth testing on its own
}
`
Method B is going to be difficult to unit test because designing input data to method A that covers all the cases we want to test for method B may be nontrivial. Furthermore the public method A stores the result in a private field, and it may be difficult to get Sut to give up the private value. This is a common pattern in PhotoDoc, I will have groups of methods within large classes with a private field or fields dedicated to those methods.
The insight to resolve this problem comes from the Single Responsibility Principle. We reason that if we want to unit test B alone, then B must have a responsibility, and since B is private it must not be SUT’s only responsibility. The solution is to move B into its own class. There are many patterns for doing so, but here is one.
`
class Sut
{
private int field;
private BHolder sub = new();
public void A(byte[] inputData) => field = sub.B(TrickyComputation(inputData));
}
public class BHolder {
public int B(double computedValue) => //Something worth testing on its own
}
`
The critical observation is that Sut’s encapsulation is not violated by this refactoring. Sut has exactly the same public interface, but its implementation dependence on B() is hidden in a private field rather than a private method. B() can now be trivially tested. As a bonus, BHolder’s implementation of B is now available to other classes which may desire it.
This is exactly what happened when I was writing the Lempel-Ziv-Walsh(LZW) decoder for Melville.PDF. LZW requires a stream of bytes to be read as a stream of bits. This was a responsibility I wanted to test independently, so I made a class, BitReader, to handle this responsibility. I tested byte-to-binary stream conversion long before I started LZW implementation. Eventually, however, the CCITT and JBIG decoders also need to read bytes as a bit stream. This single class, with minimal changes, now serves all three decoders. This class is both reusable and easy to test because it follows the SRP – it does exactly one thing well.
Conclusions
Melville.PDF has much better test coverage than PhotoDoc. Even though test coverage is an imprecise metric of test quality, this confirms my subjective impression that Melville.PDF is the much better tested codebase. One reason for this is accidental. The other is architectural.
The accidental reason is that the problem Melville.PDF solves is easy to test. At the highest level of abstraction, Melville.PDF converts ASCII strings into graphics. Testing Melville.PDF comes down to generating interesting ASCII Strings and making sure you like the output. PhotoDoc on the other hand started as image processing application that runs with a GUI. GUIs are classically difficult to test.
While GUIs are hard to test, PhotoDoc shot itself in the foot by embedding system level concepts, like WPF imaging classes or system calls to record audio deep in the domain model. An enormous ShellViewModel object at the root of the domain model was too attractive a target for violations of the law of Demeter. This means that every unit test is essentially an integration test because I have to create large portions of the domain model to test anything.
The sad part of this story is that I tried to refactor my way out of it, and was only partially successful. The ShellViewModel is a 7th of its original size but it still takes 18 parameters – and uses many of them in the constructor so they can’t just be simple mocks. I wrote a testing configuration of the IOC container that is brittle and slow just to be able to do some testing.
Consistent, perhaps even fanatical, application of the Single Responsibility Principle has resulted in much more test flexibility. Melville.PDF uses dependence injection, but uses no DI framework, because a library shouldn’t. All of the classes rely on a modest number of abstract dependencies one level of abstractions below themselves and are easily testable without a DI framework. Reasonable defaults make the library usable without a DI framework even though it uses dependency injection frequently. All the code I want to test is in a public member of some class. Private fields or explicit constructor calls are used to hide public implementations within classes at a higher level of abstraction.
Testability is a major goal of the SOLID principles, and for me they succeeded. Melville.PDF is objectively and subjectively more testable than PhotoDoc.
The next part of this serries will address code reuse.
In the next section of this 4 part serries I look at the effect of SOLID principles on code reuse.
Posted on October 5, 2022
Join Our Newsletter. No Spam, Only the good stuff.
Sign up to receive the latest update from our blog.
Related
November 15, 2024