Month: June 2016

Does TDD “damage” your design?

I recently came across a couple articles that challenged some of my beliefs about best practices.

In this article, Simon Brown makes the case for components tightly coupling a service with its data access implementation and for testing each component as a unit rather than testing the service with mocked-out data access. Brown also cites David Heinemeir Hansson, the creator of Rails, who has written a couple of incendiary articles discouraging isolated tests and even TDD in general. Heinemeir Hansson goes so far as to suggest that TDD results in “code that is warped out of shape solely to accomodate testing objectives.Ouch.

These are thought-provoking articles written by smart, accomplished engineers, but I disagree with them.

For those unfamiliar with the (volatile and sometimes confusing and controversial) terminology, isolated tests are tests which mock out dependencies of the unit under test. This is done both for performance reasons (which Heinemeir Hansson calls into question) and for focus on the unit (if a service calls the database and the test fails, is the problem in the service or the SQL or the database tables or the network connection?). There’s also a question of the difficulty of setting up and maintaining tests with database dependencies. There are tools for that, but there’s a learning curve and some set-up required (which hopefully can be Dockerized to make life easier). And there’s one more very important reason which I’ll get to later…

Both Brown and Heinemeir Hansson argue against adding what they consider unnecessary layers of indirection. If your design is test-driven, the need for unit tests will nudge you to de-couple things that Brown and Heinemeir Hansson think should remain coupled. The real dilemma is where should we put the inevitable complexity in any design? As an extreme example, to avoid all sorts of “unnecessary” code you could just put all your business logic into stored procedures in the database.

“Gang of Four” member Ralph Johnson described a paradox:

There is no theoretical reason that anything is hard to change about software. If you pick any one aspect of software then you can make it easy to change, but we don’t know how to make everything easy to change. Making something easy to change makes the overall system a little more complex, and making everything easy to change makes the entire system very complex. Complexity is what makes software hard to change. That, and duplication.

TDD, especially the “mockist” variety, nudges us to add layers of indirection to separate responsibilities cleanly. Johnson seems to be implying that doing this systematically can add unnecessary complexity to the system, making it harder to change, paradoxically undermining one of TDD’s goals.

I do not think that lots of loose coupling makes things harder to change. It does increase the number of interfaces, but it makes it easier to swap out implementations or to limit behavior changes to a single class.

And what about the complexity of the test code? Brown and Heinemeir Hansson seem to act as if reducing the complexity of the test code does not matter. Or rather, that you don’t need to write tests for code that’s hard to test because you should just expand the scope of the tests to do verification at the level of whole components.

Here’s where I get back to that other important reason why “isolated” tests are necessary: math. J.B. Rainsberger simply destroys the arguments of the kind that Brown and Heinemeir Hansson make and their emphasis on component-level tests. He points out that there’s an explosive multiplicative effect on the number of tests needed when you test classes in combination. For an oversimplified example, if your service class has 10 execution paths and its calls to your storage class have 10 execution paths on average, testing them as a component, you may need to write as may as 100 tests to get full coverage of the component. Testing them as separate units, you only need 20 tests to get the same coverage. Imagine your component has 10 interdependent classes like that… Do you have the developer bandwidth to write all those tests? If you do write them all, how easy is it to change something in your component? How many of those tests will break if you make one simple change?

So I reject the idea that TDD “damages” the design. If you think TDD would damage your design, maybe you just don’t know how bad your design is, because most of your code is not really tested.

As for Heinemeir Hansson’s contention that it’s outdated thinking to isolate tests from database access, he may be right about performance issues (not everyone has expensive development machines with fancy SSD drives, but there should be a way to run a modest number of database tests quickly). If a class’s single responsibility is closely linked to the database, I’m in favor of unit-testing it against a real database, but any other test that hits a real database should be considered an integration test. Brown proposes a re-shaped, “architecturally-aligned” testing pyramid with fewer unit tests and more integrated component tests. Because of the aforementioned combinatorial effect of coupling classes in the component, that approach would seem to require either writing (and frequently running) a lot more tests or releasing components which are not exhaustively tested.

Advertisements

Are you sure you’ve sanitized your inputs?

This boggles the mind. Using an alphabet of just 6 non-alphanumeric characters, anyone can write any javascript code. The problem of how to allow some friendly javascript code while blocking anything unfriendly might be a subject worthy of computer science research.

In the meantime, eBay (and others) really should do something to reduce this vulnerability. I have a quick-and-dirty solution in Java based on detecting significantly long runs of the 6 characters in question. The weakness of the attack in question is of course that you need a lot of characters to do anything evil in the obfuscated javascript, so there should be long runs containing only  the 6 characters. It’s possible to include spaces and line breaks and even comments to break up the runs – I took this into account in my solution.  I chose 10 as the run length threshold for detecting the obfuscation, because I don’t know of something legitimate you can do in javascript using 10 of these characters in a row that you couldn’t do another way using some alphabetic characters, and if I saw code with 10 of those characters in a row, I would suspect it right away.

Here’s some of the code in my solution. First, the implementation of containsSneakyJavascript:

public static boolean containsSneakyJavascriptCode(final String userInput) {
	SneakyJSDetectionContext ctx = new SneakyJSDetectionContext(userInput);
	while (ctx.notDone()) {
		ctx.processCurrentChar();
		ctx.nextChar();
	}
	return ctx.detectedSneakyJS();
}

That’s code at a pretty high level of abstraction, so here’s more detail with the implementation of the processCurrentChar() call that you see in the code above. It ignores whitespace and characters inside comments and otherwise checks whether the current character adds to or ends the current run of suspect characters and whether it starts a comment:

void processCurrentChar() {
	if (insideAComment()) {
		checkForEndOfComment();
	} else if (isNotWhiteSpace()) {
		if (isInSneakyAlphabet(currentChar())) {
			incrementCurrentRunLength();
		} else {
			if (isStartOfComment()) {
				setCommentStart();
			} else {
				resetCurrentRunToEmpty();
			}
		}
	}
}

The full implementation code is here, and for good measure here are the unit tests for it.

You’re welcome, eBay.