Async Process functionality – take 2

In my previous post, I showed a simple way to add a few Task-based async methods for dealing with System.Diagnostics.Process. The code I showed, while simple, has some flaws:

1. It has low cohesion. In violation of the single responsibility principle, the ProcessEx class is doing at least three things: acting as a factory for starting processes, watching for process exit events, and providing access to a more full-featured underlying Process object. (The relatively weak “Ex” name is a good clue that this class doesn’t have a clear purpose.)
2. The wait operation cannot be canceled. Perhaps a somewhat minor point, but it is a good idea for any “WaitXxxAsync” API to support cancellation.
3. It is not testable. I can’t write “pure” unit tests against a class that requires me to spawn a real, live external process and waste time waiting for a real, live exit condition.

The last point (testability) was what initially bothered me. I was able to convince myself of the correctness of the code via integration testing, but since TDD is primarily about design, I couldn’t vouch for the quality of the implementation. And when I looked more closely, some of problems in the first point (cohesion) jumped out at me.

So… I dutifully tried again! You can see the evolution of what came to be known as ProcessExitWatcher in the ProcessSample project on GitHub.

I’ll highlight some interesting checkpoints of this effort.

In the beginning, I started with the same weak name (ProcessEx) and a single unit test to demonstrate the constructor behavior (subscribing to an Exited event). At that point, I actually needed something to provide this event — hence my test stub ProcessStub implementing IProcess.

A few steps later, I began to implement the WaitForExitAsync functionality. Eventually I got to the point where I knew there was a race condition which could possibly complete the exited task twice. However, instead of trying something crazy like resorting to a nondeterministic, multithreaded integration test, I wrote a unit test to expose the race and solved it. In this case, I was able to extend my test stub and take advantage of code that I could inject via an event add accessor to sneakily raise the Exited event.

Shortly after I added an Inner property I started to have some misgivings. Why should this class need to provide outside access to the full process object? After thinking on it a bit, I recognized that choosing the more specific class name ProcessExitWatcher and making appropriate cascading naming changes would provide better clarity.

Eventually I had to write the real code that wired up my interface with a System.Diagnostics.Process. This ended up being as mundane as you could imagine, a pass-through implementation. That’s a good thing as it means no unit tests need to be written.

I wrote a few more unit tests but still sensed a code smell in returning an IProcessExit instance. The most obvious problem was the naming; a property called Inner has encapsulation-breaking connotations. But the practical issue was that a caller would now have access to surface area that should not be needed or used (e.g. one could now set EnableRaisingEvents to false and mess things up). It was now apparent that the interface needed to be broken up (interface segregation). Thus, I created IProcessExitEvents and IProcessExitStatus to separate concerns between the watcher and its clients. After all, the (conveniently read-only) status of the process is the only thing the caller should care about.

By the end of this, I had a lean and mean ProcessExitWatcher and a clean, testable design I felt good about. Here it is in action:

int exitCode;
DateTime exitTime;

using (Process process = await Task.Factory.StartNew(() => Process.Start("notepad.exe")))
using (ProcessExitWatcher watcher = new ProcessExitWatcher(new ProcessExit(process)))
{
    Console.WriteLine("Waiting for Notepad to exit...");
    await watcher.WaitForExitAsync(token);
    exitCode = watcher.Status.ExitCode;
    exitTime = watcher.Status.ExitTime;
}

Console.WriteLine("Done, exited with code {0} at {1}.", exitCode, exitTime);