Feelings Erased

An outside-in TDD youtube video series by me :-)

noreply@blogger.com (Grzegorz Gałęzowski) — Sat, 15 Oct 2022 20:35:00 +0000

Hi!

I created and published a set of videos showing me doing outside-in TDD with hexagonal architecture in C#. The videos are very low-budget, which means:

There is no voice commentary (I added subtitles though),
This was mostly done on first take and unedited (so there are some mistakes and screwups, I try to explain them in the subtitles)

Other than that, if you feel like you have ~6 hours to kill by watching me doing TDD of an an asp.net core app with hexagonal architecture and four different levels of tests, have fun!

Robert Pająk's "Lessons Learned" post - a gentle polemic

noreply@blogger.com (Grzegorz Gałęzowski) — Thu, 05 Mar 2020 00:20:00 +0000

One of the people I respect and had very productive discussions with, Robert Pająk, published a blog post about lessons learned from working with Go language. He points out 12 points and I thought it would be fun to comment on them from my own experience in OO design.

Apologies for absolute tone in many places, I tried to keep this post short so I had to resort to some mental shortcuts. Also, it may seem a bit chaotic and general, which is because unpacking all of those thoughts would probably take me a book or two.

Robert suggests to start project from a single assembly and then split into multiple apps or vertical segments. While I agree with the spirit of this point, I find this topic much more nuanced. First of all, my typical initial layout for apps is two assemblies - one for bootstrap/adapters (in the hexagonal architecture sense) and one for application logic. This way I can get the compiler to protect me from dependencies pointing in the wrong direction while keeping everything simple. When the application grows, I find partitioning vertically helps, although to achieve real vertical partitioning, I find that the features must differ significantly from each other. In other case, there's typically some kind of core model containing the code that many features need. Also, I have often some "lib" code growing aside, which fits in no specific place and could later be extracted as separate libraries.
Robert suggests e.g. that "a little copying is better than a little dependency". I strongly disagree. Not that I consider myself a reuse zealot but that I don't think there's a single rule defining all the possible cases. Also, I don't think "little" or "much" has much to do with this. My take on dependencies is:

I try to avoid depending on frameworks. If I do, I keep them away from my application logic,
I always consider whether two pieces of code evolve together and if they do - what is the cost of them falling out of sync. Based on that I decide about dependency vs. copying,
I don't hesitate from pulling dependencies containing code that I could easily write by hand but am too lazy to do so (e.g. a Maybe library) - the reason is, when such dependencies cause issues, I can replace them with hand-written code without lots of effort.

Robert suggests that clear code is better than clever code. I agree 100% but would like to rephrase it into: "if you can achieve the same with clear code or clever code, choose clear code". Sometimes I find writing clever code necessary (e.g. see how Functional.Maybe library checks whether a Maybe is "something" or "nothing").
Robert suggests to avoid interfaces that have one implementation, pointing to Mark Seemaan's blog. I'd like to point several things.

First, Mark says that single-implementation interfaces are a smell. I often see this rephrased into "single implementation interfaces don't make sense" which I strongly disagree with. Single implementation interfaces make sense when they are used to achieve ISP (e.g. separating "mutate interface" from "read interface", see e.g. IReadOnlyList interface) or where they are needed to manage dependencies more cleanly.
Interfaces are also used extensively in the GOOS design/TDD approach which I often use. Still, I am against single implementation interfaces when added as an afterthought only to enable mocking. I find they often typically bad abstractions.
The last comment, in regard to Mark's article is that I myself stopped using the phrase "x is a smell" and I find it unnecessarily offensive and somewhat arrogant.

Robert writes "Do not use patterns, nor do not apply SOLID, DDD, CQRS unless it really helps". Depending on what it means to "really help" I either agree or disagree on this one. About patterns - I don't see using a pattern where it doesn't help as using a pattern at all. About principles - I consider it the whole point of a principle that I adhere to it before I see the "real benefit". The point, I think, it that by following a principle I should not get into a situation where I would see how it really helps. I find that similar to sth. like "be honest" - the whole point is not to feel on my own skin that being dihonest doesn't pay.
This point is about generics. I agree. Generics, at least in C#, cost not only in cognitive load. E.g. applying nullable reference analysis to generic code tends to get somewhat complicated.
Robert suggests to avoid reflection and IoC containers. I agree fully.
is about the cost of fluent API. I agree as well. My rule of thumb is: I treat fluent APIs as UX tool for developers. It pays back when the API has many users and needs to be very flexible while maintaining readability. I also consider fluent API "a language in a language" and effort must be taken to learn that language.
is about SQL and ORMs. I don't know much about this topic, in my cases facades working on DTOs were typically good enough.
is about testing. There are some sentences that I disagree with.

"Tests should show what the system does, instead of how it does it" - if this means "test on the highest level", then I disagree. A system is almost always composed of subsystems. The apps I write are typically only part of much bigger systems that even transcend software. Also, tests on different level have different purposes and properties.
"Avoid mocking whenever possibile" - I think mi agrement or not depends on what "when possible" means. When I use GOOS-style design approach, I use lots of mocks but I would still say I "avoid mocking where possible" because e.g. I don't have to mock value objects and so I don't.
"There is nothing wrong in using real database or file system in your automated tests unless the tests are not taking too much time" - time is not the only factor in play - there are others. One example is isolation.
"Integration tests give more trust, because they work the same way as code in production" - I disagree. Integration tests give more trust about two pieces of whatever being able to work together and functional tests give more trust about the functionality thus I see no superiority of integration tests in that regard.
"Mocking often leads to testing implementation details instead of behavior" - in principle, I disagree. In my opinion, there is nothing inherent of mocks that makes them any special in that regard. Most often when I hear people saying this, it means their testing approach does not match their design approach, thus they end up testing what their design approach considers an implementation detail while their testing approach does not.

Again, about testing and test removal.

"If you have an integration test that already covers 10 of your unit tests, then just remove them" - I disagree with that. First of all, most of my unit tests are functional tests. Functional tests and integration tests have very different goals, so I cannot see how one can be a replacement for another. Also, "integration test" is not an alternative to "unit test" - in fact, there are integration tests that can be written on the unit level (e.g. "my DTO class should work with 3rd party serializer"). Maybe "integration" here means "higher level" - in such a case, I disagree as well. I found that in my approach, different levels of tests have different properties and thus are useful for different things, e.g. I find faster tests very useful during refactoring when ran with a CT tool such as NCrunch.
"Avoid writing tests for a single type" - again, I think that depends on the design approach. I have several open source projects where I use different design approaches and hence my testing strategies are different. One of them is driven solely by end-to-end tests, while another has three-level testing pyramid with single-type tests at the bottom and end-to-end tests at the top.

"There is nothing bad in multiple asserts in a single test as long as it verifies one scenario" - not sure if I see something bad or not about it, but I do multiple "physical" asserts per test as well and find that I never had to regret the decision.

Sandro Mancuso & Uncle Bob comparative design episode 6 notes

noreply@blogger.com (Grzegorz Gałęzowski) — Tue, 23 Oct 2018 13:25:00 +0000

Notes from episode 1
Notes from episode 2
Notes from episode 3/4/5

The episode 6 was the first episode where Uncle Bob's approach is presented. This approach is much less familiar to me than Sandro's so I had a lot of observations about both uncle Bob's testing and his design approach. Not everything is clear to me but more, I hope, will unveil in the next episodes.

As described in the episode summary, he starts with UML diagram to identify a domain model consisting of abstractions and relationship between them. Also, Bob identifies a set of use cases on a whiteboard first.
Bob starts his first test with a test file for a use case (e.g. for "create user" use case, he creates a file called CreateUserTest) and creates a first test called "nothing".
The "nothing" test quickly transforms into creation test (testing that a use case object can be created)
The second test is for the use case logic and is called "canCreateUser". The use case class is called CreateUser - a verb, not a noun, which I am fine with. I don't think all classes names should be nouns and I consider this one a good example.
When writing this test, Uncle Bob introduces a DTO and calls it CreateUserRequest. Also, he puts all the data as public fields, to stress that this is a data structure and not a class with behaviors.
As a refactoring step, Uncle Bob moves all but assertions to the setUp() method, creating essentially what is called "testcase class per fixture" test organization scheme.
A surprising step - Uncle Bob does not inject repository to the use case. Rather, he makes a class called Context and places the repository in a mutable public static field of this class. In each test, he assigns his own in-memory repository to this field. Then the production class accesses this global variable to get the repository. This is, AFAIK, the Ambient Context pattern. The rationale for using it is that Uncle Bob doesn't like passing the reference to repository around. My comments on that choice:

Personally, I don't think the reference would be "passed around" - had Uncle Bob used a factory for the use cases, he could inject the repository from composition root into the factory and from there into the use case and that's it. Not sure I would call it "passing around".
Another point would be that probably not every unit testing framework would let him get away with this. For example in xUnit.net, where tests are ran in parallel with each other, I assume he would see some nasty non-deterministic behavior.
By creating the global mutable state, Uncle Bob creates a persistent fixture, which requires a manual teardown (either at the beginning or at the end of a test). I believe him he has enough discipline to keep it torn down at all times, but in team setup, I can see two programmers, each adding new tests and new fields to the Context, each anaware of what the other does and then they merge and it turns out that the changes made by programmer "A" do not include tearing down the things that programmer "B" added and vice versa. In other words, I believe it requires an immense amount of discipline and communication to keep this stuff clean at all times.
This approach, as Sandro suggested, makes dependencies implicit. Uncle Bob argues that although it does make the dependency implicit, it does not hide it. Having looked at several definitions of what "implicit" means (I'm not a native speaker), from my perspective I would call it "hiding" anyway.

When testing whether he can get the user that was put inside the repository by the use case, Uncle Bob makes a test fail by changing the in-memory repository to return a clone of the user instead of the original reference that was put inside the repository. Here I don't know what to think of it. This is because I am not sure how Uncle bob treats the InMemoryRepository class. If it's treated as a fake, a part of the test, then this is a legit TDD step, because by modifying the repository, Bob modifies the test fixture, i.e. he modifies the test. On the other hand, if the in-memory repository is a test fixture, then how did it end up in the org.openchat.repositories production package? This is the part that confuses me.
An interesting part is that Bob tries to write the controller logic without a passing test and only goes to write one when he doesn't know what to write next in the controller logic. Bob doesn't want to mock the framework types (I wouldn't want to as well) and searches for another way, but this will be resolved in episode 7.

Sandro Mancuso & Uncle Bob comparative design episode 3/4/5 notes

noreply@blogger.com (Grzegorz Gałęzowski) — Wed, 26 Sep 2018 14:56:00 +0000

Notes from episode 1
Notes from episode 2
Notes from episode 6

Here are my notes from Comparative Design episode 3,4 and 5 by Sandro Mancuso and Uncle Bob. There were less new things for me in these episodes, more of applying the approach from previous episodes on other use cases (so the notes are going to be short on this one) . Still, I enjoyed it and consider the watching time well-spent. As with previous posts, I will only list stuff that was new or controversial to me as that's what I use the notes for. There's lots of great stuff in these videos that I don't write about because I know them from womewhere else. Also, as these are notes, they are a bit unsolicited and messy - get over it.

I learned IntelliJ IDEA's shortcut ALT+SHIFT+F10 for launching tests from current class. I didn't know that - thanks Sandro & Uncle Bob!
A nice comment from Sandro: In outside-in approach, each test represents a design decision
When doing validation whether a post contains inappropriate words, Sandro delegates the validation (at least the condition) to a separate collaborator and calls it language service instead of "validator" - nice, I never really liked the name "validator". After that, he extracts the condition together with decision to throw exception to a separate method called validate(). Typically, I call methods that throw when a condition is not met assert...() by analogy to how they are called in xUnit frameworks.
When implementing the "adding posts" requirement, Sandro leaves the repository empty since there is no requirement for reading from the repository yet. I like that - there was no point inventing a speculative reading feature for the class only to see it turn out different in the future when driven by a real use case. On the other hand, however, this might be a singnal that the acceptance tests do not revolve around use cases. Is "add a post and receive OK" really a valid customer use case? What use is adding a post if the post cannot be read? Thus, I would probably have a test where I would add a post and then read it, because from the user point of view, only then is the API useful at all. Well, there may be a case where someone writes their own GUI client and wants to integrate with each method as fast as possible (thus having POST only would be beneficial to them as it would allow them check their client compatibility), but I didn't find anything to confirm this is the assumed scenario here. Also, if I only could do one of read/write in an iteration, I would pick reading first as it would already allow showing the customer how posts look like on the GUI.
When testing the language service (a piece of code that rejects inappropriate words), Sandro picks "elephant" for a bad value (because "elephant" is one of the inappropriate words), but uses "OK text" for a good value. I would probably pick some text that's closer to the boundary, e.g. "elephan" to show that the whole word has to match to reject it. Several seconds later, Sandro picks some more examples of bad values, some of which I wouldn't think of (e.g. all uppercase ELEPHANT). Sandro's comment on this: "I don't like baby steps". I can understand that - baby steps are typically for exploratory stuff and this is a CRUD app so no surprises are expected here.
When testing with APIs that accept collections, Sandro uses a single-element collection. I typically prefer 3 elements although I must say that this time Sandro's preference is closer to boundary value testing - interesting :-).
Another interesting observation is that Sandro does sorting of posts (to have the latest post first) in the repository but filtering of logged-in user (when showing all users) in the GUI. I have to admit (and Sandro does as well) these are hard decisions to make and typically an assumption made on these things gets validates as more code appears.
The rest of the episodes 4 and 5 are more or less about the same stuff, so I see no point taking too much notes from them. The only interesting fact is that when Sandro registers a following (a relationship saying that one user follows the other) - and, while there is some validation whether the following already exists, there is no validation whether the users exist at all. I wonder why - does Sandro trust that only the UI will use the API and hence no one will send IDs that don't exist?

And that's it. This concludes my notes on Sandro's part (of course, after Bob's part there will be some dicsussion between both of them). As a final note on this part of episodes, I'd like to comment on the example problem that is used in comparative design case study. It's about writing a backend API for a twitter-like app. The best part about the chosen problem is that it is a realistic one and allows showing TDD with randomness, I/O and some other stuff that usually gets left behind in simple examples, end to end from controller to cache (without a real database though). The worst part of it is that it lacks a significant amount of logic around the use cases. Basically, it's almost a plain CRUD app and if we dropped the layering from the design, each use case would take around 10-15 lines of code to implement. And now we come to my greatest source of confusion. Sandro deciding to introduce the layering means he clearly wanted to show how to design something maintainable. On the other hand, some of the choices he made looked to me like cutting corners because this is a very simple app. No thread safety, no authorization, no authentication. Also, unfortunately, no elaborate requirements like "when you create a post and this is a 10th post created during the last second, we will consider you are a bot and disallow you from posting for 10 minuts". The resulting domain model turned out very anemic and consisting mainly of DTOs. I'd rather see less use cases but more elaborate because interactions between entities and domain concepts is where GOOS authors wrote this style really shines. Still, I enjoyed the episodes and I think this part is something that every senior coder should watch as even without an elaborate domain, it contains lots of great insight, tips, tricks and discussion. I look forward to Uncle Bob's episodes and the concluding discussion!

Sandro Mancuso & Uncle Bob comparative design episode 2 notes

noreply@blogger.com (Grzegorz Gałęzowski) — Tue, 11 Sep 2018 09:50:00 +0000

Notes from episode 1
Notes from episode 3/4/5
Notes from episode 6

After notes from episode 1, I promised some notes after episode 2 and here they are.

As with the previous post (please read the disclaimer that's there if you didn't), I am focusing on the stuff I disagree with, would do in another way or consider controversial. This is because that's what I learn from the most, not because I want to prove to anyone that I am smarter or something. There was also lots of great stuff that I just happen to agree with, so I won't write a lot about that. Also, remember that the episodes show code in evolution, so it may be that some choices Sandro makes are temporary or made with knowledge that no further development will be done in specific code areas.

General Remarks

Stuff I forgot to write down from the first episode and is also in the second.

I noticed that Sandro mixes Mockito with BddMockito, which seems kind of strange to me. Typically what I see is that people prefer one namespace or the other, but Sandro takes stubbing syntax from BddMockito (given(...).willReturn(...)) and verification syntax from plain Mockito (verify(...)). I wonder what he does when he works with his team - whether they all use this convention or someone gives up and agrees to use something different.
Sandro doesn't use a TODO list anywhere in the example. I think one of the reasons is that his UnsupportedException occurences work as TODO marks. The other may be that this is a prepared exercise so there's little exploration and question marks to be investigated. When doing prepared presentations, I also very rarely use a TODO list, so I don't see any issue with that.
He uses TDD together with programming by intention. This is something I've never done before, I will try to practice it a bit in the coming weeks.

Now for the main notes.

Users API

The beginning of the episode is about finishing the user API, where a UserService instance should create a user. Sandro moves the randomness of generating user ids (UUIDs) to a separate class, called IdGenerator and says he will not test it as it is not deterministic. While I agree that it isn't deterministic, I think some of the properties of the ID generator may be. Personally, I think it still is a material for a property-based test. Though there's (almost non-existent) chance of generating a UUID that was once used, I think it is reasonable to expect from the algorithm that no two UUIDs generated in a sequence are the same. Sure, that would lead to a discussion on "why two, not three" etc., but I would still write a small test around that property, especially that the return value of the generator is String, not UUID, so there's nothing in the signature that forces one to use UUIDs in the implementation.
The next test for the UserService is about throwing an exception when a user that we're trying to create is already in the repository. Sandro uses JUnit's expected exception annotation: @Test(expected = UserNameAlreadyInUseException.class). Not sure why he prefers it to AssertJ's built-in assertThatThrownBy(). While this is a recent feature, I think it was already around when the video was recorded. Typically when I read people writing on asserting exceptions, the annotation-oriented solutions are considered the inferior ones to specialized assertions.
In the same test, Sandro wonders whether to return an optional or to create a query method named "isUsernameTaken()". Only if that method returns true does he go on and create the user. I think I can see why he doesn't use an optional - because that would not scale - he would need to establish an implicit assumption that every Optional.empty() returned from repository means that user name is already taken, which could be a bit of stretch. Alternatively, he could throw the exception straight from the repository, but that might mean pushing some of the application logic into what can essentially end as adapter (but currently Sandro puts his repository together with domain logic). On the other hand, I'm wondering whether his solution would hold in a more real-life environment of race conditions and multiple threads accessing the repository. Sandro seems to assume no such requirement for this system since if there was one, the access to the repository would need to be synchronized. Then, he would probably need to solve the dilemma of where to place the synchronization construct. I wonder what would be his answer.
Another thing I am wondering about is whether Sandro considers the repository he created a domain construct or an adapter in the hexagonal architecture sense. He puts it into domain logic for now, but actually how he thinks of it can influence what kind of logic he may be inclined to push inside and what kind of logic he would like to keep away from it. Especially that Sandro says this is going to be replaced by a database at some point.
An interesting fact is that he defers the technology choice for the persistence mechanism. Not sure if he will implement it or not. This is the place where in normal project I would like to see something like a story map to be sure where I am, since the external API description in Swagger does not say anything about persistence, so as a behavioral specification, it is incomplete. If this is the only specification we have, then what should we derive our need for persistence from?
When testing the isUserTaken() behavior in the repository, Sandro writes two tests in a single method - one for true (name is taken), one for false (name is not taken). For me these are two behaviors and I would like to write them separately. When I look at it, the behavior looks to me like a good candidate for a parameterized test, although I must say that JUnit 4 does not make writing parameterized test methods easy. At the very least, if I really felt this is closer to a single behavior (just that each assertion shows one side of the coin), I would employ the soft assertions feature of AssertJ to not stop on the first assertion failure.
There is a nice discussion between Sandro and Bob on why there isn't a getUser() method in the repository that would allow checking whether the user is in the repository or not. I agree with Sandro that adding such method would be premature as no existing use case needs it yet and I think something like this would constrain my internal implementation.

Login API

I noticed that Sandro doesn't use named constants in tests. Even though the production code uses the library-defined OK_200 constant (as funny as this name might be), the test uses plain 200. Not sure what the rationale is as it is never given, so I am going to try and guess that Sandro does not trust the constants and wants the tests independent of them because if tests used the named constants as the production code does, the values of the constants would essentially fall out of scope of a test, so if the value of OK_200 changed at one point, there would be no test to detect that. Personally, I prefer a different way of including constants in the test scope. I use the named constants everywhere in the tests and write an additional small test that says assertThat(OK_200).isEqualTo(200) writing such test takes me literally half a minute and the dilemma is solved for me. Now if the value of the constant changes, I have a single test to maintain.
Just as Sandro created UserApi class, he creates LoginApi. I think not putting both registration and login methods in the same class is a clever move because by doing that, Sandro establishes vetrtical architecture slicing and has a green field for his new feature in a fresh vertical slice.
Interestingly, Sandro goes straight to the repository from the LoginAPI, not introducing any kind of LoginService as he did in case of the UserAPI -> UserService->UserRepository. The rationale is that the login API is so simple that in this case Sandro is willing to do an exception and move away from what he considers a redundant abstraction layer. I think that doing that is a nice demonstration of benefits of vertical slicing, since he can make this decision for login API without affecting the user API. This is one of the reasons why I like vertical slicing so much. If he did put both registration and login methods in the same API class, it would look confusing that for one feature, the API goes to the service which goes to the repository and for another the same API class goes straight to the repository. That being said, I don't understand in what way is the login API simpler than the user API - both are plain "DTO for DTO" cases with "else return error" conditions so I see no significant factor that would lead me to taking a different approach in each case.
I have some (temporary?) concerns with the UserCredentials class - it seems to be a a DTO and a value object in one. While I don't have much against DTOs implementing comparison operations for convenience, the class later seems to get more responsibilities that make it even more similar to a value object (i.e. a method boolean matches(User user)). Maybe this is a beginning of evolution that I will observe further in the next episodes?
In my notes form the last episode, I noticed that Sandro established a relationship between test values where such relationship is not required. He did the exact contrary in the login API test. Here, he created input UserCredentials and did not make data relationship between it and the User object set to be returned by a mock. He only established such relationship between User and its JSON representation, using the User instance as a reference. This is exactly how I like to handle such cases. This way it is clearly shown that it's not the login API's business to say what kind of values will the User object hold - this responsibility is pushed onto the UserRepository class.
Sandro has two identical methods called jsonContaining() but chooses not to remove the duplication. He gives a nice explanation that he is still not sure whether the two occurences of this method will change for the same or different reasons. Also, I can only guess that he is more careful with refactorings that come across vertical slices, but this is only my assumption. I would maybe leave a TODO here if I believed this conflict can be resolved in a foreseeable future. If I believed otherwise, I would probably eliminate this duplication because it would probably be easier for me to later copy-paste than find all the ocurrences of a piece of code.
In the controller test, Sandro has the following login() method:

String login(Request request, Response response) {
UserCredentials credentials = credentialsFrom(request);
Optional<User> user = userRepository.userFor(credentials);
if(user.isPresent()) {
response.status(OK_200);
response.type("application/json");
return jsonFor(user.get());
}
response.status(NOT_FOUND_404);
return "Invalid credentials";
}
One thing that bothers be is that in the test for the case where a user is NOT present, there is no assertion on the content type of the response, which means that if I move the line response.type("application/json") above the if statement, all the tests are going to compile and pass but the response is going to be different. I assume that from Sandro's perspective, this doesn't matter as he has only one client application and that application doesn't care (or maybe acceptance tests check that?). Typically in what I do for a living, I am told to be strict about what I offer through my API and generous about what I accept. This is because where I work, the API is the product and I don't know all the client applications up-front.
In the same method, a choice between response.status(OK_200) and response.status(NOT_FOUND_400) is what I call a situation of exclusive choice. In other words, this is not like an "add" method that can be called multiple times, but only one call to the status() method makes sense and it must be exclusively chosen. In such situation, I typically use Mockito's VerifyNoMoreInteractions() to make sure only one choice is made. In this case, however, there are several calls to the response object so VerifyNoMoreInteractions() would be too coarse-grained because it works for the whole mock, not for a single method. The code can be refactored (I will show how in a second) to make the use of VerifyNoMoreInteractions easier or something like this could work:

verify(response).status(OK_200);
verify(response, never()).status(intThat(status -> !status.equals(OK_200)));
This could be also refactored to something like:

verifyStatusSetExclusivelyTo(OK_200, response);

The whole rationale for me doing it is that presence of two different calls to response.status() in one method move a potential error of calling them both at some point to the Inherently possible (can be made impossible) group from the inherently impossible (can be made possible) group.
Sandro refactors the login() method to use two more methods and ternary operator (condition ? a : b) to transform the code into something like this:

String login(Request request, Response response) {
UserCredentials credentials = credentialsFrom(request);
Optional<User> user = userRepository.userFor(credentials);
return user.isPresent()
? prepareOkResponse(response, user)
: prepareErrorResponse(response);}

Bob and Sandro briefly discuss this choice, Sandro commenting that he likes ternary operator and that the real problem with ternary operators is when you chain them. Personally, though I have nothing against ternary operator as such I see one more issue with the use of ternary operator in this case and the refactoring that was done here. To demonstrate it on a simpler example, I find the following use of ternary operator confusing as well:

int a = getA();
int b = getB();
int c = getC();
int y = (a = b) == 3 ? c = 1 : 4;The part that confuses me in the code above (besides the cryptic variable names :-)) is that there are side-effects in ternary operator - two assignment operators which, in C-like languages, happen to return value as well. A ternary operator turns "if" from statement to an expression and typically when I see someone make expression out of a statement like this, it is to stress that the whole code in scope of the operator is just a composition of pure expressions or pure functions. This is where I think ternary operator is even clearer in conveying intention than the if statement. But this is not what happens in the login() method as both prepareOkResponse() and prepareErrorResponse() are impure functions with side effects, similarly to the assignment operator I presented above. For example, the prepareErrorResponse() is defined as:

private String prepareErrorResponse(Response response) {
response.status(NOT_FOUND_404); //side effect
return "Invalid Credentials";
}
When looking at the login() method and the ternary operator, I wouldn't expect to find side effect in a function like this, so this is not my favorite solution. What would I do instead? Probably either I would not do the refactoring or (in more complex use case) I would refactor further to something like custom result observer or collecting parameter, which would on the other hand cost me at least one or two new classes (and would probably require moving the login() method logic to a service class from the controller). The code could then look like this (note that the Response type is changed to my own custom class):

void login(Request request, MyOwnResponse response) {
UserCredentials credentials = credentialsFrom(request);
Optional<User> user = userRepository.userFor(credentials);
if(user.isPresent()) {
response.foundCredentialsMatching(user.get());
} else {
response.InvalidCredentials();
}
}

This would have the added benefit that I could now use VerifyNoMoreInteractions on the response object as now there is only one exclusive call to this object in each case and the situation of exclusive choice becomes clearer.

Note that in my example, the login() method is void. This would be a result of pushing the responsibility of holding the return value to the MyOwnResponse class. Someone that calls the login() method (in my case - a controller) could then do something like this:

loginService.login(request, response);
return response.text();
Sandro claims his acceptance tests only test the sunny day scenarios because of test pyramid, so he pushes all the error conditions into the unit tests. Personally, I typically don't have as dramatic dilemma as I write bigger systems where there are three or four levels of tests. And while I would not test error conditions in the end-to-end tests, I would not leave them for unit tests alone. I would have maybe one test at the lowest test level where I still have real network and HTTP, one test for each error category in tests that test the whole component logic in isolation from external resources and leave the rest to unit tests. My experience is that errors and validation is something that sometimes can be more tricky than the sunny day logic. Moreover, I remember writing some code where error detection and reporting is the only logic there is (e.g. assertion libraries) so (even though the openchat app is not like that) I personally believe that validation and error reporting is often underrated part of the system, especially when it is a big system.
An interesting observation is that, while Sandro tends to prefer vertical architecture slicing, his package structure does not seem reflect it (yet?). For example, he has domain.posts package where I would expect posts.domain with posts, the vertical feature, as the main package differentiator. That may be a temporary choice so I think I would need to watch more and see for myself.

And there it is. I suspect I will not be doing as lengthy notes onward as there will be less things I will see for the first time, but I can be wrong :-).

Sandro Mancuso & Uncle Bob comparative design episode 1 notes

noreply@blogger.com (Grzegorz Gałęzowski) — Thu, 06 Sep 2018 10:33:00 +0000

Update: Sandro commented on my notes in a twitter thread.

Notes from episode 2
Notes from episode 3/4/5
Notes from episode 6

I just finished watching episode 1 of Uncle Bob and Sandro Mancuso's London vs Chicago Comparative Design Case Study and enjoyed it. I really recommend this case study to anyone who wants to learn more about TDD.

This episode 1 was about Sandro beginning writing a twitter-like application and the first tests. Sandro claims to use the London Style TDD school, which is heavily based on mocking and outside-in approach. The school originated (AFAIK) from the Growing Object Oriented Software Guided By Tests book. Because of that and because I prefer this approach to design as well, I thought it could be interesting to write down, even if only for the "future me", the notes on the differences I think I can see between what Sandro did and what GOOS would do and what I would do. Of course, this is only based on episode 1, so some things may change during the next episodes.

Disclaimer

This is a set of notes, not a full blown article, so its structure might be a bit too dense and unsolicited. I welcome discussion and requests to clarify stuff. If you didn't see the episode, there is a high chance some of what I write will not make sense to you.
This post is not a critique of Sandro's style. I am not claiming that I am better in TDD than Sandro or better in this particular style of TDD. While I may sometimes sound as if I find some ideas silly, it's just for showing this stuff from the perspective of my biases and default choices (this is why sometimes I will describe my feelings, not only rational judgement). I am by no means suggesting that my choices are/would be better. I am sharing this because I think that sharing thoughts (and feelings) leads to interesting discussions. If you find me being disrespectful, please point it out so that I can correct it.
While I point some of the differences I think I see between GOOS and Sandro's TDD, it's not that I do EVERYthing like in GOOS. I am not including the differences between my TDD and GOOS TDD because I figured nobody would be interested in that :-).
I am not an authority on what is and is not compliant with GOOS style. I just write my subjective opinion based on my observations that may be wrong. If you see me missing the point, please point it out so that I can correct it and learn from the exercise.

Differences between Sandro's TDD and GOOS:

GOOS worked example started with a single acceptance test, then some code was written (without unit tests) just to make this acceptance test pass, resulting with a walking skeleton. The next step would be to refactor the existing code based only on the acceptance test and then introduce some first abstractions and place to insert mocks for future behaviors. Sandro does two things differently:

He has all the acceptance tests written up-front (Update: this was explained by Sandro as being an input condition for the whole exercise)
He drives even the very first feature with unit tests already.

GOOS advises writing the acceptance tests in a domain language. For that, it recommends building a testing DSL that isolate a test from the implementation details like delivery mechanism. Sandro uses plain RestAssured library calls without (almost) any abstractions on top of it. This may be because Sandro's tests are not really end-to-end, but they are tests for HTTP contracts between the front-end and the back-end (Update: this was explained by Sandro as being an input condition for the whole exercise). Also, Sandro calls his tests "integration tests", which is a term that is used for a different kind of tests in GOOS, but that might not be an issue since, as I believe, the "integration" is more about the goal of a test, not about a level or kind.
GOOS advises usage of strict mocking framework (JMock), while Sandro uses a more-loose-by-default mocking framework (Mockito). Usage of strict mocking framework by GOOS was not merely a choice dictated by the availability of tools at the time the book was written. On the GOOS mailing list, there is a comment by Steve Freeman that he considers strictness-by-default part of his style (cannot find it now though).
GOOS advises mocking interfaces, while Sandro up to now has only mocked classes. As above, the choice to introduce and mock interfaces in GOOS was not dictated by the limitation of tools at that time. It was a deliberate choice to think of interfaces as roles that objects play and GOOS authors claim they are aggressive in refactoring interfaces to make them smaller and more abstract.

Now time for the differences between Sandro's TDD and my TDD:

Sandro starts all his tests from a name and then an assertion. As this is one of my ways to start a failing tests, it's not the only one that I use. In my book, I describe 4 different approaches to starting with a failing test, starting from assertion being one of them.
Sandro uses underscores in test names and lower camel case in production code method names. It's just a matter of preference, but I find that confusing - I find it better to stick to a single convention, which has the added value of simplifying the configuration of static analysis tools that enforce naming convention and takes one dilemma away. Still, this is not a big deal, just a matter of style.
Sandro tends to prefer realistic test data even where this is not needed or required. In the first test, where he passes data for user registration, he chooses a realistic name, (semi)realistic password and the ID is generated as UUID and converted to string. This is not needed at all, because the object under test does not handle any validation and, as we find out later, the only class that knows that the user ID is actually a UUID is the IdGenerator - in the rest of the cases, any string would do. Personally I use dummy data wherever validating this data is not the responsibility of the object under test because I find that by doing it, I describe the contract of my class with its collaborators more precisely. Whenever I see a specific/realistic data, I interpret it as if it was required by the tested object. Also, I am confused about one more thing - if I use another ID generator or change the implementation of existing one, should I change the "realistic data" in the tests where it doesn't matter?
Also, I wouldn't establish relationship between test data fragments where it is clearly not required. For example, Sandro creates a registration object with "name", "password" and "about" fields and sets up a mock of a service object to return a User object with the same three values. Personally, I don't see why. The responsibility for establishing relationship between input and output values is clearly pushed to another class that is mocked (a service class), so in my mind, this is leaking responsibilities of another class to tests of class that clearly does not care. And as I understand GOOS compositional approach to design and its principle of context independence, a class and its tests should not assume in what context the class is used.
Sandro moves parts of the code to the setup method and parts of the data to fields of class with tests, arguing that this is cleaner. I wrote a blog post long ago explaining why I don't like this idea and why I find such tests harder to read and maintain with the "Testcase class per class" organization approach.. Looking at the video, the setup method is called "initialise", which I find a meaningless name and usually read it as "bag for everything I don't want to look at". Thus I consider it a smell and I also feel it takes away a bit of the pressure from tests to improve the design. When I see some logic that I strongly feel could be moved to a setup method and fields, this leads me to searching for a way to improve the design. I find setup methods hiding these issues for me. I understand that most probably, Sandro has a way of compensating for all the issues I described and maybe even making them irrelevant.
The method of UserAPI class that Sandro test-drives clearly violates the principle of command-query separation (it is a command that returns a value as well). This is often what frameworks require us to do, unfortunately. Sandro chooses to push this violation further into the application/domain layer (specifically - the UserService class). Personally, I tend to cut away from this kind of violation on controller level using a Collecting Parameter pattern and then I only deal with commands, which I find makes the rest of my tests easier.
In the controller test, Sandro includes parsing in the scope. I would probably push the parsing responsibility to a collaborator. I can see that Sandro can succeed with his approach with a simple, non-nested JSON with three fields, but I wonder if his approach would be the same if it was a bigger structure with several nesting levels, optional sections etc.

What's happening with my book and future plans

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 02 Jul 2017 20:07:00 +0000

Hi, I haven't posted in a while, so a quick update:

I am currently focusing on my book, that's why the updates on this blog are extremely rare.
I have some nice topics to write blog posts about, but I'm putting them on hold for a bit. The steady increase pace of my book's content (it's already over 250 pages A4!) should make it up for you.
Speaking about book's content, I recently finished an overhaul of part 1, which took me a long time and lots of effort.
I'm currently fixing some bugs submitted on the book's github page for some portions of part 2
When I finish this, I plan on continuing part 2, review the value objects chapters and probably move TDD with mocks to part 3.
After that, I want to write something about hexagonal architecture (which will probably be part 4) and TDDing on a level of a whole component, using mentioned hexagonal approach (maybe part 5?).

There is still a lot of work ahead of me, so thank you all for your support and I hope you like the new book content. I not, well, you can still contact me with your thoughts and opinions!

New release of my book

noreply@blogger.com (Grzegorz Gałęzowski) — Mon, 09 May 2016 21:36:00 +0000

Release notes are on the books' github page. Enjoy!

Is test isolation always a good thing?

noreply@blogger.com (Grzegorz Gałęzowski) — Sat, 13 Feb 2016 20:35:00 +0000

TDD is not only about unit tests. Steve Freeman and Nat Pryce argue in their book that a TDD cycle is kickstarted using an end-to-end automated test. These tests exercise a system as a black box. This often implies difficulties and issues that well written unit tests don’t have. On of such fields of difficulty is test fixture management, especially fixture teardown.

Gerard Meszaros in his book titled XUnit test patterns provides an extensive discussion of different fixture management strategies and their implications. Using Meszaros’ terminology, what I have seen many end-to-end tests using is a combination of:

Persistent shared immutable fixture - this is the data that’s shared by many tests - usually some initial environment setup and plumbing, uploading licenses and so on. The “immutable” part of the fixture name means that no test is allowed to modify any data that’s part of this fixture. The reason is that each test relies on this data being intact. If any test modifies the fixture, then other tests’ assumptions are broken and we cannot expect them to work reliably.
Persistent fresh fixture - this is the data that’s needed for a single test. This may be, for example, adding a specific employee required by the test scenario to a database. As database is a persistent storage, this data must be cleaned up either at the end of the same test that created it or at the beginning of any new test (there are some ways to achieve that safely, but that’s a topic for another post). The cleanup is needed to avoid test interactions - many tests may add an employee with the same ID number, but with other data being different. Thus we need to prepare a clean bed for each test so that leftover data from previous tests doesn’t get in the way (by the way, there are some other ways of dealing with test interactions than cleaning up - although they’re quite difficult to apply).
Transient fresh fixture - this part of data is created inside a test and removed without our intervention when the test finishes running, usually by garbage collector or other memory management mechanism. The simplest example of such fixture would be an integer variable defined in a test method - we don’t have to worry about it impacting the other tests, because its lifetime is automatically associated with the lifetime of a test.

After reading XUnit test patterns, my conclusion was obvious - life would be beautiful if we could only use transient fresh fixture. When writing unit tests for classes or small clusters of classes, this kind of fixture is the only one we ever need to use. Things change, on the other hand, when e.g. our tests require installation of the application and a database - in such situation, using only transient fixture would probably mean that each test should perform a fresh installation of an operating system. This would take ages, so we use the other fixture types as a compromise. We accept the problems they bring and accept the fact that they can introduce interactions between tests (and thus, instability) and trade these things for shorter test execution time.

I met with an opinion that this is not actually bad. What’s more, the opinion states, this can be seen as an advantage, because when a single instance of an application is exercised by many different tests, it resembles closer the way the user exercises the system when it’s deployed and, by working longer with an existing instance of a system, it may uncover some defects that would otherwise not become visible.

I have several issues with this point of view on fixture management. In order to explain why is that, let’s recap the differences between how user exercises a system and how automated end-to-end tests do it.

What are the differences between usage by automated tests and by user?

There are three pretty big differences that I can think of:

An automated test has a scope, i.e. it has a beginning

When defining a test, we usually define the context in which a test should be executed, a series of steps and an expected outcome. For example, we might say:

Given there are two employees: Ken and Tom //the context
When I choose to remove Ken's data         //step 1
And list all my employees                  //step 2
Then I should only see Tom's data          //expected outcome

Here, the explicitly stated context is that we have two employees - if there is only one, executing the two steps would most probably create different results. Of course, there is also an implicit context that’s not written here, for example that the application is installed and turned on and probably that I’m logged into the application. The test relies on both kinds of context to be the same for every execution. In other words, in order to bring stable outcomes, a test requires a context to be defined. Users usually work in the context they are currently given and are able to adapt to it. For example, I want to create an e-mail account with an address "buziaczek@mymail.com", but this address already exists. No problem, I’ll just create "buziaczek-1984@mymail.com" and I’m ready to go. Most of the time, an automated test doesn’t make such decisions (although we can do something to bypass such situations, e.g. generating a portion of the e-mail address randomly, but note that it’s still not making a decision).

Thus, although it may happen for a user to follow a strict sequence of steps in a strictly defined context, this is not what their work with the application is about. If it was, we could probably automate these sequences in the software itself.

Anyway, a demand for strictly defined context for each automated test means that this context would usually have to be cleaned after or before each test (Meszaros writes about other ways, like generating distinct test data for each test, but I didn’t see them in practice all that often). This is somehow contradictory to wanting to achieve testing “as the user would” by executing tests in partially the same context (i.e. on the same running or installed application instance). The contradiction for me is that we seem to value test isolation, except that we don’t.

An automated test has a scope, i.e. it has an end

A user uses a product in a continuous manner, usually taking many, many steps during each session with an application. On the other hand, when reading on good practices for automated testing, one can spot that such tutorials hardly ever advise writing long, automated scenarios. It is more common to see several test cases for several different behaviors or variants of a single behavior rather than a single test case exercising all of these conditions. So, it looks like we deliberately try to run away from testing the system the way a user would in automated tests. Why is that?

This is for several reasons. One of them is that most of the time, a failed assertion ends an automated test, so the longer the test, the harder it is to get feedback from the later parts of that test, because there are more things that can fail along the way. Two other reasons that I’d like to talk about are: splitting work into manageable chunks and defect localization.

Splitting into manageable chunks means that we prefer shorter scenarios to longer ones because it is easier to maintain and modify each scenario independently when it’s smaller. This is because building up context becomes increasingly harder to grasp, understand and manage. Users, on the other hand, don’t need to manage scenarios - they dynamically choose their goals and steps based on the current situation.

Defect localization means that when a test fails, I need to search for the failure root cause only among what’s in the scope of this test. When a scope of a test is smaller, I have fewer places to search for such root cause. This is also one of the reasons why we set descriptive names for tests, i.e. we’d rather say “the user should be able to delete an employee” than “the user should be able to play with the system” - because in the first case, a failure points us closer to the root cause. The usual thing I do when I see an automated test failing is to try and run it in isolation (i.e. not as part of a suite) and in clean environment. This is because it gives me a better feedback on whether the failure is caused by something that’s in the scope. So, the broader the scope of a test, the lower defect localization.

In his wonderful book, Lean-Agile Acceptance Test-Driven Development, Ken Pugh outlines three conditions we strive to achieve for automated tests:

A test should fail for a well-defined reason.
No other test should fail for the same reason.
A test should not fail for any other reason.

And, while Ken admits this is often very hard (or impossible) to achieve, this is the ideal we should be targeting. Managing a persistent fixture introduces more complexity and possible root causes of test failures, driving us away from these three conditions.

An automated test has a scope, i.e. it has a purpose

As I said above, a test should fail for a well-defined reason. This well-defined reason for failure is a purpose of the test. In other words, every test failure should relate to its purpose. This purpose is usually conveyed in test name, for example “An added employee should appear on an employee report”. Note that very rarely do we see tests like “previous tests runs should not have led the application into an illegal or crashing state”. This is because the tests aren’t usually written to check whether an application crashes if we do some random things with it. Sharing parts of the fixture between tests doesn’t help them achieving their purpose.

I also said that it is probably impossible to get rid of all the things that are beyond the purpose of a test but still can impact the outcome of the test. For example, if a test loads anything from a disk, it can fail because of (unlikely but possible) disk drivers failure. These things are outside the scope of a test, but still influence its result.

The point is, the more things exist outside the scope of an automated test that can affect its outcome, the more the test becomes incompetent at testing what’s inside its scope. Do we want the tests to be incompetent in fulfilling their purpose? I hope not. Thus, my preference is to maximize the competency of a test by reducing to a minimum other things that can impact its outcome. This is how I believe I get the most value out of automated tests. This is in line with Meszaros’ write-up on the goals of test automation, where he stresses the ease of maintainability as one of such goals.

Automated tests teardown is often very error-prone and adds a lot of instability to the test suite, thus making it less competent in doing the thing it written for. For some time, For test execution time-related reasons, I’m afraid we’ll have to live with sharing some parts of persistent fixtures, however, If I could get rid of the necessity of teardowns in tests without sacrificing their run time, I’d do so in a blink of an eye.

Summary

My opinion on test isolation is that the more the better. For me, saying that partial lack of isolation is better (because it would uncover some defects only a user otherwise could) is really taking the biggest disadvantage of automated black-box tests and pretending it’s an advantage. It is relying on undeliberate potential effect of this disadvantage. Creating a test approach targeted towards uncovering such defects (i.e. relying on deliberate effort to uncover them instead of hoping they will come up from other tests) would in my opinion give a better return of investment.

Two new chapters on value obejcts

noreply@blogger.com (Grzegorz Gałęzowski) — Fri, 25 Sep 2015 07:13:00 +0000

Hi!

It has been long since I last wrote about the progress of my book. Two chapters were added since, and both on value objects:

Happy reading and, as always, I welcome any feedback!

I recorded a video: FizzBuzz To Chain of Responsibility to DSL using TDD in Java

noreply@blogger.com (Grzegorz Gałęzowski) — Tue, 01 Sep 2015 04:25:00 +0000

Sorry for bad narration - I did very minor preparation to record this video. Also sorry for saying "dividable" instead of divisible.

This movie demonstrates implementation a variation of FizzBuzz kata without error checking, but with refactoring towards the Chain of Responsibility pattern. The first part is done using TDD and triangulation.

New chapter: Viewing object composition as a language

noreply@blogger.com (Grzegorz Gałęzowski) — Fri, 12 Jun 2015 10:27:00 +0000

Hi!

New chapter on viewing object composition as a language was added. This chapter is about how the idea of having a system of composable objects leads to inventing a set of mini domain-specific languages.

Happy reading and I really welcome any feedback!

New Chapter: Protocols

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 30 Nov 2014 22:29:00 +0000

Hi!

New chapter on protocols was added. This chapter deals with the idea of creating a stable communication patterns between object and its collaborators. Understanding this idea is crucial for working effectively with mock objects.

Happy reading!

Three new chapters!

noreply@blogger.com (Grzegorz Gałęzowski) — Sat, 29 Nov 2014 22:11:00 +0000

Hi!

Just to let you know - three new chapters were added since the last time I blogged:

Happy reading and I hope this makes up for long absence of posts here!

Why do we need composability?

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 10 Aug 2014 12:45:00 +0000

Hi,

I moved my book to leanpub (as you may have guessed from the sidebar of this blog). The online version looks very good and is available for all, so I decided there is no point reproducing it on the blog.

Instead, you can read the new chapter called Why do we need composability? online on leanpub and maybe decide to download the current state of the book (which is an always will be free).

This chapter is a next step in explaining the object oriented design knowledge necessary to understand mock objects well.

Have fun!

Road To Mock Objects, pt. 2: Telling, not asking

noreply@blogger.com (Grzegorz Gałęzowski) — Fri, 16 May 2014 19:26:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

In this post, we'll get back to Johnny and Benjamin as they introduce another change in the code they are working on. In the process, they discover the impact that return values and getters have on composability of objects.

Contractors

Johnny: G'morning. Ready for another task?

Benjamin: Of course! What's next?

Johnny: Remember the code we worked on yesterday? It contains a policy for regular employees of the company. But the company wants to start hiring contractors as well and needs to include a policy for them in the application.

Benjamin: So this is what we will be doing today?

Johnny: That's right. The policy is going to be different for contractors. While, just as regular employees, they will be receiving raises and bonuses, the rules will be different. I made a small table to allow comparing what we have for regular employees and what we want to add for contractors:

	Raise	Bonus
Regular Employee	+10% of current salary if not reached maximum on a given pay grade	+200% of current salary one time after five years
Contractor	+5% of average salary calculated for last 3 years of service (or all previous years of service if they have worked for less than 3 years)	+10% of current salary when a contractor receives score more than 100 for the previous year

So while the workflow is going to be the same for both a regular employee and a contractor:

Load from repository
Evaluate raise
Evaluate bonus
Save

the implementation of some of the steps will be different for each type of employee.

Benjamin: Correct me if I am wrong, but these "load" and "save" steps do not look like they belong with the remaining two - they describe something technical, while the other steps describe something strictly related to how the company operates...

Johnny: Good catch, however, this is something we'll deal with later. Remember the scout rule - just don't make it worse. Still, we're going to fix some of the design flaws today.

Benjamin: Aww... I'd just fix all of it right away.

Johnny: Ha ha, patience, Luke. For now, let's look at the code we have now before we plan further steps.

Benjamin: Let me just open my IDE... OK, here it is:

public class CompanyPolicies 
{
  readonly Repository _repository; 

  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    var employees = _repository.CurrentEmployees();

    foreach(var employee in employees)
    {
      var payGrade = employee.GetPayGrade();

      //evaluate raise
      if(employee.GetSalary() < payGrade.Maximum)
      {
        var newSalary 
          = employee.GetSalary() 
          + employee.GetSalary() 
          * 0.1;
        employee.SetSalary(newSalary);
      }
      
      //evaluate one-time bonus
      if(employee.GetYearsOfService() == 5)
      {
        var oneTimeBonus = employee.GetSalary() * 2;
        employee.SetBonusForYear(2014, oneTimeBonus);
      }
      
      employee.Save();
    }
  }
}

Benjamin: Look, Johnny, the class in fact contains all the four steps you mentioned, but they are not named explicitly - instead, their internal implementation for regular employees is just inserted in here. How are we supposed to add the variation of the employee type?

Johnny: Time to consider our options. We have few of them. Well?

Benjamin: For now, I can see two. The first one would be to create another class similar to CompanyPolicies, called something like CompanyPoliciesForContractors and implement the new logic there. This would let us leave the original class as is, but we would have to change the places that use CompanyPolicies to use both classes and choose which one to use somehow. Also, we would have to add a separate method to repository for retrieving the contractors.

Johnny: In addition, we would miss our chance to communicate through the code that the sequence of steps is intentionally similar in both cases. Others who read this code in the future will see that the implementation for regular employees follows the steps: load, evaluate raise, evaluate bonus, save. When they look at the implementation for contractors, they will see the same order of steps, but they will be unable to tell whether the similarity is intentional, or is it there by pure accident.

Benjamin: So our second option is to put an if statement into the differing steps inside the CompanyPolicies class, to distinguish between regular employees and contractors. The Employee class would have an isContractor() method and depending on what it would return, we would either execute the logic for regular employees or for contractors. Assuming that the current structure of the code looks like this:

foreach(var employee in employees)
{
  //evaluate raise
  ...
      
  //evaluate one-time bonus
  ...
  
  //save employee
}

the new structure would look like this:

foreach(var employee in employees)
{
  if(employee.IsContractor())
  {
    //evaluate raise for contractor
    ...
  }
  else
  {
    //evaluate raise for regular
    ...
  }

  if(employee.IsContractor())
  {
    //evaluate one-time bonus for contractor
    ...
  }
  else
  {
    //evaluate one-time bonus for regular
    ...
  }
  
  //save employee
  ...
}

this way we would show that the steps are the same, but the implementation is different. Also, this would mostly require us to add code and not move the existing code around.

Johnny: The downside is that we would make the class even uglier than it was when we started. So despite initial easiness, we'll be doing a huge disservice to future maintainers. We have at least one another option. What would that be?

Benjamin: Let's see... we could move all the details concerning the implementation of the steps from CompanyPolicies class into the Employee class itself, leaving only the names and the order of steps in CompanyPolicies:

foreach(var employee in employees)
{
  employee.EvaluateRaise();
  employee.EvaluateOneTimeBonus();
  employee.Save();
}

Then, we could change the Employee into an interface, so that it could be either a RegularEmployee or ContractorEmployee - both classes would have different implementations of the steps, but the CompanyPolicies would not notice, since it would not be coupled to the implementation of the steps anymore - just the names and the order.

Johnny: This solution would have one downside - we would need to significantly change the current code, but you know what? I'm willing to do it, especially that I was told today that the logic is covered by some tests which we can run to see if a regression was introduced.

Benjamin: Cool, what do we start with?

Johnny: The first thing that is between us and our goal are these getters on the Employee class:

GetSalary();
GetGrade();
GetYearsOfService();

They just expose too much information specific to the regular employees. It would be impossible to use different implementations when these are around. These setters don't help much:

SetSalary(newSalary)
SetBonusForYear(year, amount);

While these are not as bad, we'd better give ourselves more flexibility. Thus, let's hide all of this behind more abstract methods that hide what actually happens, but reveal our intention.

First, take a look at this code:

//evaluate raise
if(employee.GetSalary() < payGrade.Maximum)
{
  var newSalary 
    = employee.GetSalary() 
    + employee.GetSalary() 
    * 0.1;
  employee.SetSalary(newSalary);
}

Each time you see a block of code separated from the rest with blank lines and starting with a comment, you see something screaming "I want to be a separate method that contains this code and has a name after the comment!". Let's grant this wish and make it a separate method on the Employee class.

Benjamin: Ok, wait a minute... here:

employee.EvaluateRaise();

Johnny: Great! Now, we've got another example of this species here:

//evaluate one-time bonus
if(employee.GetYearsOfService() == 5)
{
  var oneTimeBonus = employee.GetSalary() * 2;
  employee.SetBonusForYear(2014, oneTimeBonus);
}

Benjamin: This one should be even easier... Ok, take a look:

employee.EvaluateOneTimeBonus();

Johnny: Almost good. I'd only leave out the information that the bonus is one-time from the name.

Benjamin: Why? Don't we want to include what happens in the method name?

Johnny: Actually, no. What we want to include is our intention. The bonus being one-time is something specific to the regular employees and we want to abstract away the details about this or that kind of employee, so that we can plug in different implementations without making the method name lie. The names should reflect that we want to evaluate a bonus, whatever that means for a particular type of employee. Thus, let's make it:

employee.EvaluateBonus();

Benjamin: Ok, I get it. No problem.

Johnny: Now let's take a look at the full code of the EvaluateIncentivePlan method to see whether it is still coupled to details specific to regular employees. Here's the code:

public void ApplyYearlyIncentivePlan()
{
  var employees = _repository.CurrentEmployees();

  foreach(var employee in employees)
  {
    employee.EvaluateRaise();
    employee.EvaluateBonus();
    employee.Save();
  }
}

Benjamin: It seems that there is no coupling to the details about regular employees anymore. Thus, we can safely make the repository return a combination of regulars and contractors without this code noticing anything. Now I think I understand what you were trying to achieve. If we make interactions between objects happen on a more abstract level, then we can put in different implementations with less effort.

Johnny: True. Can you see another thing related to the lack of return values on all of employee's methods in the current implementation?

Benjamin: Actually, no. Does it matter?

Johnny: Well, if Employee methods had return values and this code depended on them, all subclasses of Employee would be forced to supply return values as well and these return values would need to match the expectations of the code that calls these methods, whatever these expectations were. This would make introducing other kinds of employees harder. But now that there are no return values, we can, for example:

introduce a TemporaryEmployee that has no raises, by leaving its EvaluateRaise() method empty, and the code that uses employees will not notice.
introduce a ProbationEmployee that has no bonus policy, by leaving its EvaluateBonus() method empty, and the code that uses employees will not notice.
introduce an InMemoryEmployee that has empty Save() method, and the code that uses employees will not notice.

As you see, by asking the objects less, and telling it more, we get greater flexibility to create alternative implementations and the composability, which we talked about yesterday, increases!

Benjamin: I see... So telling objects what to do instead of asking them for their data makes the interactions between objects more abstract, and so, more stable, increasing composability of interacting objects. This is a valuable lesson - it is the first time I hear this and it seems a pretty powerful concept.

A Quick Retrospective

In this post, Benjamin learned that the composability of objects (not to mention clarity) is reinforced when interactions between them are: abstract, logical and stable. Also, he discovered, with Johnny's help, that it is further strengthened by following a design style where objects are told what to do instead of asked to give away information to somebody who the makes the decision on their behalf. This is because if an API of an abstraction is built around answering to specific questions, the clients of the abstraction tend to ask it a lot of questions an are coupled to both those questions and some aspects of the answers (i.e. what it in the return values). This makes creating another implementation of abstraction harder, because each new implementation of the abstraction needs to not only provide answers to all those questions, but the answers are constrained to what the client expects. When abstraction is merely told what its client wants it to achieve, the clients is decoupled from most of the details of how this happens. This makes introducing new implementations of abstraction easier - it often even lets us define implementations with all methods empty without the client noticing at all.

These are all important conclusions that will lead us towards TDD with mock objects.

Time to leave Johnny and Benjamin for now. In the next posts, I'm going to reiterate on their discoveries and put them in a broader context.

Road To Mock Objects, pt. 1: On Objects Composability

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 13 Apr 2014 11:40:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

In this post, I will try to outline briefly why objects' composability is a goal worth achieving and how it can be achieved. I am going to start with an example of unmaintainable code and will gradually fix its flaws in the next posts. For now, we are going to fix just one of the flaws, so the code we will end up will not be perfect by any means, still, it will be better by one quality.

In the coming posts, we will learn more valuable lessons resulting from changing this little piece of code and slowly arrive at justification for using mock objects.

Another task for Johnny and Benjamin

Remember Johnny and Benjamin? Looks like they managed their previous task and are up to something else. Let us listen to their conversation as they are working on another project...

Benjamin: So, what's this assignment about?

Johnny: Actually, it's nothing exciting - we'll have to add two features to a legacy application that's not prepared for the changes.

Benjamin: What is the code for?

Johnny: It is a C# class that implements company policies. As the company has just started using this automated system and it was started recently, there is only one policy implemented: yearly incentive plan. Many corporations have what they call incentive plans. These plans are used to promote good behaviors and exceeding expectations by employees of a company.

Benjamin: You mean, the project has just started and is already in a bad shape?

Johnny: Yep. The guys writing it wanted to "keep it simple", whatever that means, and now it looks pretty bad.

Benjamin: I see...

Johnny: By the way, do you like riddles?

Benjamin: Always!

Johnny: So here's one: how do you call a development phase when you ensure high code quality?

Benjamin: ... ... No clue... So what is it called?

Johnny: It's called "now".

Benjamin: Oh!

Johnny: Getting back to the topic, here's the company incentive plan.

Every employee has a pay grade. An employee can be promoted to a higher pay grade, but the mechanics of how that works is something we will not need to deal with.

Normally, every year, everyone gets a raise by 10%. But to encourage behaviors that give an employee a higher pay grade, such employee cannot get raises indefinitely on a given pay grade. Each grade has its associated maximum pay. If this amount of money is reached, an employee does not get a raise anymore until they reach a higher pay grade.

Additionally, every employee on their 5th anniversary of working for the company, gets a special, one-time bonus which is twice their current payment.

Benjamin: Looks like the source code repository just finished synchronizing. Let's take a bite at the code!

Johnny: Sure, here you go:

public class CompanyPolicies : IDisposable
{
  readonly SqlRepository _repository
    = new SqlRepository();
  
  public void ApplyYearlyIncentivePlan()
  {
    var employees = _repository.CurrentEmployees();

    foreach(var employee in employees)
    {
      var payGrade = employee.GetPayGrade();
      //evaluate raise
      if(employee.GetSalary() < payGrade.Maximum)
      {
        var newSalary 
          = employee.GetSalary() 
          + employee.GetSalary() 
          * 0.1;
        employee.SetSalary(newSalary);
      }
      
      //evaluate one-time bonus
      if(employee.GetYearseOfService() == 5)
      {
        var oneTimeBonus = employee.GetSalary() * 2;
        employee.SetBonusForYear(2014, oneTimeBonus);
      }
      
      employee.Save();
    }
  }
  
  public void Dispose()
  {
    _repository.Dispose();
  }
}

Benjamin: Wow, there is a lot of literal constants all around and functional decomposition is barely done!

Johnny: Yeah. We won't be fixing all of that today. Still, we will follow the scout rule and "leave the campground cleaner than we found it".

Benjamin: What's our assignment?

Johnny: First of all, we need to provide our users a choice between an SQL database and a NoSQL one. To achieve our goal, we need to be somehow able to make the CompanyPolicies class database type-agnostic. For now, as you can see, the implementation is coupled to the specific SqlRepository, because it creates a specific instance itself:

public class CompanyPolicies : IDisposable
{
  readonly SqlRepository _repository
    = new SqlRepository();

Now, we need to evaluate the options we have to pick the best one. What options do you see, Benjamin?

Benjamin: Well, we could certainly extract an interface from SqlRepository and introduce an if statement to the constructor like this:

public class CompanyPolicies : IDisposable
{
  readonly Repository _repository;

  public CompanyPolicies()
  {
    if(...)
    {
      _repository = new SqlRepository();
    }
    else
    {
      _repository = new NoSqlRepository();
    }
  }

Johnny: True, but this option has few deficiencies. First of all, remember we're trying to follow the scout rule and by using this option we introduce more complexity to the CommonPolicies class. Also, let's say tommorow someone writes another class for, say, reporting and this class will also need to access the repository - they will need to make the same decision on repositories in their code as we do in ours. This effectively means duplicating code. Thus, I'd rather evaluate further options and check if we can come up with something better. What's our next option?

Benjamin: Another option would be to change the SqlRepository itself to be just a wrapper around the actual database access, like this:

public class SqlRepository : IDisposable
{
  readonly Repository _repository;

  public SqlRepository()
  {
    if(...)
    {
      _repository = new RealSqlRepository();
    }
    else
    {
      _repository = new RealNoSqlRepository();
    }
  }

  IList<Employee> CurrentEmployees()
  {
    return _repository.CurrentEmployees();
  }

Johnny: Sure, this is an approach that could work and would be worth considering for very serious legacy code, as it does not force us to change the CompanyPolicies class at all. However, there are some issues with it. First of all, the SqlRepository name would be misleading. Second, look at the CurrentEmployees() method - all it does is delegating a call to the implementation chosen in the constructor. With every new method required of the repository, we'll need to add new delegating methods. In reality, it isn't such a big deal, but maybe we can do better than that?

Benjamin: Let me think, let me think... I evaluated the option where CompanyPolicies class makes the choice between repositories. I also evaluated the option where we hack the SqlRepository to makes this choice. The last option I can think of is leaving this choice to another, "3rd party" code, that would choose the repository to use and pass it to the CompanyPolicies via constructor, like this:

public class CompanyPolicies : IDisposable
{
  private readonly Repository _repository;

  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }

This way, the CompanyPolicies won't know what exactly is passed to it via constructor and we can pass whatever we like - either an SQL repository or a NoSQL one!

Johnny: Great! This is the option we're looking for! For now, just believe me that this approach will lead us to many good things - you'll see why later.

Benjamin: OK, so let me just pull the SqlRepository instance outside the CompanyPolicies class and make it an implementation of Repository interface, then create a constructor and pass the real instance through it...

Johnny: Sure, I'll go get some coffee.

... 10 minutes later

Benjamin: Ha ha! Look at this! I am SUPREME!

public class CompanyPolicies : IDisposable
{
  //_repository is now an interface
  readonly Repository _repository; 

  // repository is passed from outside.
  // We don't know what exact implementation it is.
  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    //... body of the method. Unchanged.
  }
  
  public void Dispose()
  {
    _repository.Dispose();
  }
}

Johnny: Hey, hey, hold your horses! There is one thing wrong with this code.

Benjamin: Huh? I thought this is what we were aiming at.

Johnny: Yes, with the exception of the Dispose() method. Look closely at the CompanyPolicies class. it is changed so that it is not responsible for creating a repository for itself, but it still disposes of it. This is wrong because CompanyPolicies instance does not have any right to assume it is the only object that is using the repository. If so, then it cannot determine the moment when the repository becomes unnecessary and can be safely disposed of.

Benjamin: Ok, I get the theory, but why is this bad in practice? Can you give me an example?

Johnny: Sure, let me sketch a quick example. As soon as you have two instances of CompanyPolicies class, both sharing the same instance of Repository, you're cooked. This is because one instance of CompanyPolicies may dispose of the repository while the other one may still want to use it.

Benjamin: So who is going to dispose of the repository?

Johnny: The same part of the code that creates it, for example the Main method. Let me show you an example of how this may look like:

public static void Main(string[] args)
{
  using(var repo = new SqlRepository())
  {
    var policies = new CompanyPolicies(repo);

    //use policies for anything you like
  }
}

This way the repository is created at the start of the program and disposed of at the end. Thanks to this, the CompanyPolicies has no disposable fields and it itself does not have to be disposable - we can just delete the Dispose() method:

//not implementing IDisposable anymore:
public class CompanyPolicies 
{
  //_repository is now an interface
  readonly Repository _repository; 

  //New constructor
  public CompanyPolicies(Repository repository)
  {
    _repository = repository;
  }
  
  public void ApplyYearlyIncentivePlan()
  {
    //... body of the method. No changes
  }

  //no Dispose() method anymore
}

Benjamin: Cool. So, what now? Seems we have the CompanyPolicies class depending on repository abstraction instead of an actual implementation, like SQL repository. My guess is that we will be able to make another class implementing the interface for NoSQL data access and just pass it through the constructor instead of the original one.

Johnny: Yes. For example, look at CompanyPolicies component. We can compose it with a repository like this:

var policies 
  = new CompanyPolicies(new SqlRepository());

or like this:

var policies 
  = new CompanyPolicies(new NoSqlRepository());

without changing the code of CompanyPolicies. This means that CompanyPolicies does not need to know what Repository exactly it is composed with, as long as this Repository follows the required interface and meets expectations of CompanyPolicies (e.g. does not throw exceptions when it is not supposed to do so). An implementation of Repository may be itself a very complex and composed of another set of classes, for example something like this:

new SqlRepository(
  new ConnectionString("..."), 
  new AccessPrivileges(
    new Role("Admin"), 
    new Role("Auditor")),
  new InMemoryCache());

but the CompanyPolicies neither knows or cares about this, as long as it can use our new Repository implementation just like other repositories.

Benjamin: I see... So, getting back to our task, shall we proceed with making a NoSQL implementation of the Repository interface?

Johnny: First, show me the interface that you extracted while I was looking for the coffee.

Benjamin: Here:

public interface Repository
{
  IList<Employee> CurrentEmployees();
}

Johnny: Ok, so what we need is to create just another implementation and pass it through the constructor depending on what data source is chosen and we're finished with this part of the task.

Benjamin: You mean there's more?

Johnny: Yeah, but that's something for tomorrow. I'm exhausted today.

Quick Retrospective

In this post, Benjamin learned to appreciate the composability of objects, i.e. the ability to compose object with other objects into more complex structures with more complex behaviors without changing the implementation of the object's class. This means that potentially, a composable object might trigger different behaviors depending on what it is composed with. We'll talk more about this property in the coming posts.

As I said, the code Johnny and Benjamin worked on in this posts still has some serious flaws. For now, our characters did not encounter a desperate need to address those flaws yet, but this is going to change in the next posts.

Also, after we part again with Johnny and Benjamin, we are going to reiterate the ideas they stumble upon in a more disciplined manner.

Mocks "break encapsulation"? Here's why I disagree...

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 05 Jan 2014 19:34:00 +0000

note that when I say "statement" in this blog post, I mean "unit test".

For some time, I am witnessing discussions whether using mocks is something that "breaks encapsulation" and thus should be used sparingly. The time has come for me to voice my opinion.

Is "breaking encapsulation" even the right charge?

First of all, we must decrypt what "breaking encapsulation" means. It's not so simple actually, for two reasons.

The first reason is that encapsulation is understood differently by different people and is sometimes used synonymously with information hiding and sometimes a distinction is made between the two.

If we decide to simplify things and treat encapsulation as "information hiding", it is still unclear what it means to "break" it. Does it mean to "reveal information"? In this case, almost any object breaks encapsulation, because any object reveals some information. In other words, there's not such thing as "full encapsulation", because if everything was hidden, objects would not be able to communicate at all.

So, what does it mean to "break encapsulation" in case of charges made against mocks?

What is the actual charge?

The thing many people have problem with is that unit-level specification makes statements on how an object uses (or more precisely, communicates with) its collaborators. For example, let's assume that I have a class that sends a packet through a connection and write the following unit-level statement:

[Test] public void
ShouldOpenConnectionThenSendAndThenCloseItWhenAskedToSendPacket()
{
  //GIVEN
  var connection = Substitute.For<Connection>();
  var sending = new Sending(connection);
  var packet = Any.Instance<Packet>();

  //WHEN
  sending.PerformFor(packet);

  //THEN
  Received.InOrder(() =>
  {
    connection.Open();
    connection.Send(packet);
    connection.Close();
  });
}

Note that this statement is coupled not only to the public methods of a class, but also to the way it uses objects of another class. The usual charge made against such statements is based on the following deduction chain:

How an object uses its collaborators is not part of the object's public interface, hence it's an implementation detail
Mocks are all about how an object uses its collaborators
So it seems that statements using mocks are coupled to the implementation details of specified class
If we change the class so that it uses different collaborators or uses them differently, it may still do its job from the perspective of the clients of those objects (who just call the methods and expect return values). But, the statements will be failing in such case, because they're coupled to how object uses its collaborators.
The statements using mocks are brittle
Mocks should be avoided
Well... unless they're necessary to get high code coverage in places where just invoking methods and checking results is not enough

In the next section, I'll try to point why I think this kind of thinking is flawed.

Dependency Injection changes the game

The first point of this deduction chain we just talked about tells that it's object's private business who and how it calls to achieve this goal.

This may even be true for an object that is instantiated like this:

var sending = new Sending();

But is utterly false when the object is instantiates like this:

Connection c = ...; //get object that implements Connection interface
var sending = new Sending(connection);

What is changed, you may ask - it's just adding a constructor parameter! Well, no it's not just this. The point is, the Sending class now can have its dependency injected. This is very important, because the whole point of dependency injection is that the object is not the owner of its dependencies. Thus, such object effectively says: "Listen up, my client, it's your problem what you give me, as long as I am able to use it". In other words, each client of the Sending class can have their own class implementing the Connection interface and each new user of the class can write their own implementation if they wish so.

So now the real question kicks in:

How can anyone provide a custom implementation to work with a class without knowing how the custom implementation will be used?

Remember, you can't just implement all methods of an Connection interface anyway you want. By doing so, you have a nice chance to violate the Liskov Substitution Principle. Thus, the custom implementation of Connection a client provides must not only implement the interface, it must also adhere to a certain protocol required by the Sending class that will be using it.

This leads us to one conclusion: if a custom implementation of Connection must be coded with a knowledge of the protocol between Sending and Connection, this protocol is not a private business of Sending class. While it may not be part of the public API, it's still part of the public contract.

Other examples

There are many examples out there that what I just wrote is true. For example, let's look at JEE Servlet lifecycle documentation and see what we can read there:

The lifecycle of a servlet is controlled by the container in which the servlet has been deployed. When a request is mapped to a servlet, the container performs the following steps.

If an instance of the servlet does not exist, the web container

Loads the servlet class.

Creates an instance of the servlet class.

Initializes the servlet instance by calling the init method. Initialization is covered in Creating and Initializing a Servlet.

Invokes the service method, passing request and response objects. Service methods are discussed in Writing Service Methods.

If it needs to remove the servlet, the container finalizes the servlet by calling the servlet’s destroy method. For more information, see Finalizing a Servlet.

For comparison, let's take a look at what we can read at ASP.NET web page life cycle documentation. Again, there's a table where we can find the names of the methods called at each stage, the order of the methods called and what we can expect when implementing each method.

These aren't necessarily examples of dependency injection, but are examples of inversion of control, which DI is a particular application of.

So, ekhem... would you really say that putting knowledge on what methods and in what order are performed breaks encapsulation of JSP container or ASP.NET runtime? Or, in better words, is it exposing implementation details? No, it's just another part of the class public contract. Another example might be unit-testing frameworks, where you write so called "test classes" knowing, that "Setup" method is called before a test and "teardown" is called after.

And this is my view on the whole encapsulation dillema - as soon as you make the dependencies of the class injectable (or use any other forms of inversion of control), how this class uses those dependencies becomes a part of public contract of a class, not private implementation detail. This public contract should be specified and thus we write unit level statements unit mock objects.

Of course, nothing, never, prevents you from exposing private implementation details while using any technique, it's just that mocks don't seem different in this regard in any particular way.

What do YOU think?

Moving to NUnit from MsTest - experience report

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 08 Dec 2013 21:33:00 +0000

Recently, I was a part of migration of some tests from MsTest to NUnit. I was in a situation where some of the tests were written in NUnit and some (legacy ones) in MsTest and it was too costly to maintain both tools, so the money were put on NUnit. The goal was to convert existing tests as fast as possible, leaving refactoring and cleanup of test and production code for later. I didn't know much about MsTest before, and here's what I learned about it and the transition:

Assertions

Much to my astonishment, the number of assertions MsTest supports is rather small, compared to NUnit. You cannot assert something is greater than something else, cannot assert a value is in a certain range etc. This way, a lot of assertions looked like this (remember this was ugly legacy test code :-)):

Assert.True(numberOfMessages <= 1000 
  && numberOfMessages >= 950, "numberOfMessages")

This, at failure, gives you the a message like: "Expected true, got false". Pretty helpful, isn't it (sarcasm)? Sure, you can make the message better, but you have to work hard on it yourself. The NUnit's (better) equivalent is:

Assert.That(numberOfMessages, Is.InRange(950, 1000));

Usually, when writing good unit tests, I don't really need this kind of assertions, but this was legacy code and it tested many kinds of weird situations over a large set of objects put together.

On the other hand, MsTest has a strongly typed assertions in form of Assert.AreEqual<T>() which are missing from NUnit. This, however, is not a big deal, because creating a custom wrapping assertions which will give you this benefit in NUnit is trivial. For example:

public class XAssert
{
  public static void AreEqual<T>(T expected, T actual)
  {
    NUnit.Framework.Assert.AreEqual(expected, actual);
  }
}

Lessons learned:

If you ever use MsTest, do yourself a favor and pick an external assertion library.
If you use NUnit, writing strongly typed wrappers over common assertions may be a good idea.

Deployment Items in .testsettings file

MsTest has this little feature of deployment items that can be configured in solution-wide .testsettings file to be copied to the output directory before a test is run (usually these are some extra files etc.).

But wait! Why on earth would someone be willing to use that kind of feature in unit tests at all?

Unfortunately, this was my case - the deployment items were used to load an XML configuration :-(. The problem with Deployment Items is that some 3rd party runners support it, and some don't (namely: NCrunch and Gallio, which makes running tools like Sonar over such tests pretty difficult). Also, some 3rd party unit testing tools do not run tests from the build output directory, but create their owne custom directories somewhere on the hard drive (e.g. NCrunch does), but the paths coded in Deployment Items are not adjusted and get gopied to the same directories as always, leading to failed runs.

Lessons learned:

If you're using Deployment Items in MsTest for unit tests, you don't even give me a chance to think you're serious.
When Deployment Items are used to copy some additional files to the build output, this mechanism can be replaced with including these config files in test project as links and setting their "Copy Local" property to copy them to the build output directory.
Do not write or load files in unit tests!!!

PrivateType and PrivateObject

I was shocked when I discovered that MsTest includes helpers for accessing private instance/static methods: PrivateType and PrivateObject. Even for legacy code, there are better strategies to apply. Anyway, what shocks me more is that anyone can even think that using these may be a good idea (it is not). Thus, when converting to NUnit, I found such constructs in the code. They lead to brittle tests that are accessing members using strings and reflection. It seems like MsTest allows generating accessors that are strongly typed and use strings and reflection under the hood, but this is still coupling to implementation details, plus I saw PrivateType and PrivateObject being used directly many times.

Lessons Learned:

When writing new tests, forget that PrivateType and PrivateObject exist. Look for the real problem.
PrivateType and PrivateObject are just helpers over a reflection API, so they don't require being executed under MsTest to work properly. During transition to NUnit, you can leave references to MsTest assembly and change all code references to include namespace e.g. from PrivateObject to Microsoft.VisualStudio.TestTools.UnitTesting.PrivateObject. The code will still work.

Cost of Transition

In general, it's a lot simpler to migrate from MsTest to NUnit than the other way round. This is because NUnit requires only that an assembly references nunit.framework.dll and marks certain methods with [Test] attribute. On the other hand, MsTest requires a special type of project (a Test Project) to hold the tests. Also, as I said, MsTest's assertions are for the most part a subset of NUnit's. Anyway, to convert a project from MsTest to NUnit, the following steps are usually sufficient:

Reference nunit.framework.dll from the test project.
Convert Deployment Items into links in project copied to the build output folder.
Search-Replace the following:
1. [TestClass] -> [TestFixture]
2. [TestMethod] -> [Test]
3. [TestInitialize] -> [SetUp]
4. [TestCleanup] -> [TearDown]
5. [TestClassInitialize] -> [TestFixtureSetUp]
6. [TestClassCleanup] -> [TestFixtureTearDown]
7. using Microsoft.VisualStudio.TestTools.UnitTesting; -> using NUnit.Framework;
Change Assert.IsInstanceOfType(x, typeof(X)) to Assert.IsInstanceOf<X>(x);
Change Assert.AreEqual<X>(x, y) to Assert.AreEqual(x, y)
Fix references to some MsTest-specific classes (like PrivateObject)

Summary

And that's it. These were the lessons learned and tips for dealing with the transition as fast as possible. I advise to do a real cleanup of the test and production code sooner than later, but the tips I outlined here let you defer this cost.

Developing TDD style by example

noreply@blogger.com (Grzegorz Gałęzowski) — Tue, 08 Oct 2013 07:09:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

(Note that I use a term "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post)

Throughout this blog, you have seen me using quite extensively a utility class called Any. The time has come to explain a little bit more carefully what principles lie under this technique and tool. I'll also use this technique as a case study to show you how one develops a style of Test-Driven Development.

A style?

Yep. Why am I wasting your time writing about style instead of giving you the hardcore technical details? The answer is simple. Before I started writing this tutorial, I read four or five books solely on TDD and maybe two others that contain chapters on TDD. All of this sums up to about two or three thousands of paper pages, plus numerous posts on many blogs. And you know what I noticed? No two authors use exactly the same sets of techniques for test-driving their code! I mean, sometimes, when you look techniques they're suggesting, the suggestions from two authorities contradict each other. As each authority has their followers, it's not uncommon to observe and take part in discussions about whether this or that technique is better than a competing one or which one leads to trouble in the long run.

I did this, too. I also tried to understand how come people praise techniques I KNEW were wrong and led to disaster. Then, Finally, I got it. I understood that it's not a "technique A vs. technique B" debate. There are certain sets of techniques that work together and choosing one technique leaves us with issues we have to resolve by adopting other techniques. This is how a style is created.

Developing a style starts with underlying set of principles. These principles lead us to adopt our first technique, which makes us adopt another one and, ultimately, a coherent style emerges. Using Constrained Non-Determinism as an example, I'll try to show you how part of a style gets derived from a technique that is derived from a principle.

Principle: Tests As Specification

As I already stressed, I strongly believe that unit tests constitute an executable specification. Thus, they should not only pass input values to an object and assert on the output, they should also convey to their reader the rules according to which objects and functions work. The following oversimplified example shows a Statement where it's not explicitly stated what's the relationship between input and output:

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostName()
{
  //GIVEN
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo("MY_HOST_NAME");
  
  //THEN
  Assert.Equal("backup_MY_HOST_NAME.zip");
}

Although the relationship can be guessed quite easily, remember it's just an example. Also, seeing code like that makes me ask questions like: is the "backup_" prefix always applied? What if I actually pass "backup_" instead of "MY_HOST_NAME"? Will the name be "backup_backup_.zip", or "backup_.zip"? Also, is this object responsible for any validation of passed string?

This makes me invent a first technique to provide my Statements with better support for the principle I believe in.

First technique: Anonymous Input

I can wrap the actual value "MY_HOST_NAME" with a method and give it a name that better documents the constraints imposed on it by the specified functionality. In our case, we can pass whatever string we want (the object is not responsible for input validation), so we'll name our method AnyString():

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostName()
{
  //GIVEN
  var hostName = AnyString();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName);
  
  //THEN
  Assert.Equal("backup_MY_HOST_NAME.zip");
}

public string AnyString()
{
  return "MY_HOST_NAME";
}

By using anonymous input, we provide a better documentation of the input value. Here, I wrote AnyString(), but of course, there can be a situation where I use more constrained data, e.g. AnyAlphaNumericString() when I need a string that does not contain any characters other than letters and digits. Note that this technique is applicable only when the particular value of the variable is not important, but rather its "trait". Taking authorization as an example, when a certain behavior occurs only when the input value is Users.Admin, there's no sense making it anonymous. On the other hand, for a behavior that occurs for all values other than Users.Admin, it makes sense to use a method like AnyUserOtherThan(Users.Admin) or even AnyNonAdminUser().

Now that the Statement itself is freed from the knowledge of the concrete value of hostName variable, the concrete value of "backup_MY_HOST_NAME.zip" looks kinda weird. There's no clear indication of the kind of relationship between input and output and whether there is any at all (one may reason whether the output is always the same string or maybe it depends on the string length). It is unclear which part is added by the production code and which part depends on the input we pass to the method. This leads us to another technique.

Second Technique: Derived Values

To better document the relationship between input and output, we have to simply derive the expected value we assert on from the input value. Here's the same Statement with the assertion changed:

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostName()
{
  //GIVEN
  var hostName = AnyString();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName);
  
  //THEN
  Assert.Equal(string.Format("backup_{0}.zip", hostName);
}
public string AnyString()
{
  return "MY_HOST_NAME";
}

This looks more like a part of specification, because we're documenting the format of the backup file name and show which part of the format is variable and which part is fixed. This is something you'd probably find documented in a paper specification for the application you're writing - it would probably contain a sentence saying: "The format of a backup file should be backup_H.zip, where H is the current local host name".

Derived values are about defining expected output in terms of the input that was passed to provide a clear indication on what is the "transformation" of the input required of the specified production code.

Third technique: Distinct Generated Values

Let's assume that some time after our initial version is shipped, we're asked to make the backup feature applied locally per user only for this user's data. As the customer doesn't want to confuse files from different users, we're asked to add name of the user doing backup to the backup file name. Thus, the new format is "backup_H_U.zip", where H is still the host name and U is the user name. Our Statement for the pattern must change as well to include this information. Of course, we're trying to use the anonymous input again as a proven technique and we end up with:

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostNameAndUserName()
{
  //GIVEN
  var hostName = AnyString();
  var userName = AnyString();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName, userName);
  
  //THEN
  Assert.Equal(string.Format(
    "backup_{0}_{1}.zip", hostName, userName);
}

public string AnyString()
{
  return "MY_HOST_NAME";
}

Now, we can clearly see that there is something wrong with this Statement. AnyString() is used twice and each time it returns the same value, which means that evaluating the Statement does not give us any guarantee, that both values are applied and that they're applied in the correct places. For example, the Statement will be evaluated to true when user name is used instead of host name in specified production code. This means that if we still want to use the anonymous input effectively, we have to make the two values distinct, e.g. like this:

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostNameAndUserName()
{
  //GIVEN
  var hostName = AnyString();
  var userName = AnyString2();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName, userName);
  
  //THEN
  Assert.Equal(string.Format(
    "backup_{0}_{1}.zip", hostName, userName);
}

public string AnyString()
{
  return "MY_HOST_NAME";
}

public string AnyString2()
{
  return "MY_USER_NAME";
}

We solved the problem (for now) by introducing another helper method. However, this, as you can see, isn't a very scalable solution. Thus, let's try to reduce the amount of helper methods for string generation to one and make it return a different value each time:

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostNameAndUserName()
{
  //GIVEN
  var hostName = AnyString();
  var userName = AnyString();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName, userName);
  
  //THEN
  Assert.Equal(string.Format(
    "backup_{0}_{1}.zip", hostName, userName);
}

public string AnyString()
{
  return Guid.NewGuid.ToString();
}

This time, we're not returning an understandable string, but rather a guid, which gives us the fairly strong guarantee of generating distinct value each time. The string not being understandable (contrary to something like "MY_HOST_NAME") may leave you worried that maybe we're losing something, but hey, didn't we say AnyString()?

Distinct generated values means that each time we need a value of a particular type, we get something different (if possible) than the last time and each value is generated automatically using some kind of heuristics.

Fourth technique: Constant Specification

Let us consider another modification that we're requested to make - this time, the backup file name needs to contain version number of our application as well. Remembering that we want to use Derived Values, we won't hardcode the version number into our Statement. Instead, we're going to use a constant that's already defined somewhere else in the application (this way we also avoid duplication of this version number across the application):

[Fact] public void 
ShouldCreateBackupFileNameContainingPassedHostNameAndUserNameAndVersion()
{
  //GIVEN
  var hostName = AnyString();
  var userName = AnyString();
  var fileNamePattern = new BackupFileNamePattern();
  
  //WHEN
  var name = fileNamePattern.ApplyTo(hostName, userName);
  
  //THEN
  Assert.Equal(string.Format(
    "backup_{0}_{1}_{2}.zip", 
    hostName, userName, Version.Number);
}

public string AnyString()
{
  return Guid.NewGuid.ToString();
}

Note that I didn't use the literal constant value, but rather, the value inside the Version.Number constant. This allows us to use derived value, but leaves us a little worried about whether the value of the constant is correct - after all, we're using it for creation of our expected value, but it's a part of production code - i.e. is something that should be specified itself!

To keep everyone happy, we write a single Statement just for the constant to specify what the value should be:

[Fact] public void 
ShouldContainNumberEqualTo1_0()
{
  Assert.Equal("1.0", Version.Number);
}

By doing so, we make the value in the production code just echo what's in our executable Specification, which we can fully trust.

Summary of the example

In this example, I tried to show you how a style can evolve from the principles you value when doing TDD. I did so for two reasons:

To introduce to you a set of techniques I personally use and recommend and to do it in a fluent and logical way.
To help you better communicate with people that are using different styles. Instead of just throwing "you're doing it wrong" at them, try to understand their principles and how their techniques of choice support those principles.

Now, let's take a quick summary of all the techniques introduced in example:

Anonymous Input: moving the output out of the Statement code and hide it behind a method that to emphasize the constrain on the data used rather than what's its value
Derived Values: defining expected output in terms of the input in order to document the relationship between input and output
Distinct Generated Values: When using Anonymous Input, generate a distinct value each time (in case of types that have very few values, like boolean, try at least not to generate the same value twice in a row) in order to make the Statement more reliable.
Constant Specification: Write a separate Statement for a constant and use the constant instead of its literal value in all other Statements to create a Derived Value.

Constrained non-determinism

When we combine anonymous input together with distinct generated values, we get something that's called Constrained Non-Determinism. This is a term coined by Mark Seemann and basically means three things:

Values are anonymous i.e. we don't know the actual value we're using
The values are generated in as distinct as possible sequence (which means that, whenever possible, no two values generated one after another hold the same value)
The non-determinism in generation of the values is constrained, which means that the algorithms for generating values are carefully picked in order to provide values that are not special in any way (e.g. when generating integers, we don't allow generating '0' as it's usually a special-case-value)) and that are not "evil" (e.g. for integers, we generate small positive values first and go with bigger numbers only when we run out of those small ones).

There are multiple ways to implement constrained non-determinism. Mark Seemann himself invented the AutoFixture library for C# that's freely available to download by anyone. Here's a shortest possible snippet to generate an anonymous integer using AutoFixture:

Fixture fixture = new Fixture();
var anonymousInteger = fixture.Create<int>();

I, after Amir Kolsky and Scott Bain, like to use Any class. Any takes a slightly different approach than AutoFixture (although it shamelessly uses AutoFixture internally and actually obscures its impressive abilities). My implementation of Any class is available to download as well.

Summary

That was a long ride, wasn't it? I hope that this post gave you some understanding of how different TDD styles came into existence and why I use some of the techniques I do (and how these techniques are not just a series of random choices). In the next posts, I'll try to introduce some more techniques to help you grow a bag of neat tricks - a coherent style.

How is TDD about analysis and what's with the GIVEN-WHEN-THEN structure?

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 11 Aug 2013 19:50:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

(Note that I use a term "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post)

Is there really a commonality between analysis and TDD?

From Wikipedia:

Analysis is the process of breaking a complex topic or substance into smaller parts to gain a better understanding of it.

Thus, in order for TDD to be about analysis, it has to fulfill two conditions:

Be a process of breaking a complex topic into smaller parts
Allow gaining a better understanding of such broken topics

In the story about Johnny, Benjamin and Jane, I included a part where they analyze requirements using concrete examples. Johnny explained that this is a part of a technique called Acceptance Test-Driven Development. The process followed by the three characters fulfilled both mentioned conditions for a process to be analytical. But what about TDD itself?

Actually, I used parts of ATDD process in the story to make the analysis part more obvious, but similar things happen at pure technical levels. For example, when starting development with a failing application-wide Statement (we'll talk about levels of granularity of Statements some time later. For now the only thing you need to know is that the so called "unit tests level" is not the only level of granularity we write Statements on), we may encounter a situation where we need to call a web method and assert its result. This makes us think: what should this method be called? What are the scenarios supported? What do I need to get out of it? How should its user be notified about errors? Many times, this leads us to either a conversation (if there's another stakeholder that needs to be involved in the decision) or rethinking our assumptions. This is how we gain a better understanding of the topic we're analyzing, which makes TDD fulfill the first of the two requirements for it to be an analysis method.

But what about the first requirement? What about breaking a complex logic into smaller parts?

If you go back to our example, you'll note that both when talking to a customer and when writing code, Johnny and Benjamin used a TODO list. This list was first filled with whatever scenarios they came up with, but later, they would add a smaller unit of work. This is one way complex topics are decomposed into smaller items that all land on the TODO list (the other way is mocking, but we won't get into that yet). Thanks to this, we can focus on one thing at a time, crossing off item after item from the list after it's done. If we learn something new or encounter new issue that needs our attention, we can add it to the TODO list and get back to it later, for now continuing our work on the current point of focus.

An example TODO list from the middle of implementation may look like this (don't read trough it, I put it here only to give you a glimpse):

~~Create entry point to the module (top-level abstraction)~~
~~Implement main workflow of the module~~
~~Implement Message interface~~
~~Implement MessageFactory interface~~
Implement ValidationRules interface
~~Implement behavior required from Wrap method in LocationMessageFactory class~~
Implement behavior required from ValidateWith method in LocationMessage class for Speed field
Implement behavior required from ValidateWith method in LocationMessage class for Age field
Implement behavior required from ValidateWith method in LocationMessage class for Sender field

Note that some items are already crossed out as done, while other remain undone and waiting to be addressed.

Ok, That's it for the discussion. Now that we're sure that TDD is about analysis, let's focus on the tools can use to aid and inform it.

Gherkin

Hungry? Too bad, because the Gkerkin I'm gonna talk about is not edible. It's a notation and a way of thinking about behaviors of the specified piece of code. It can be applied on different levels of granularity - any behavior, whether of a whole system or a single class, may be described using it.

I actually talked about Gherkin before on this blog, just did not name it. It's the GIVEN-WHEN-THEN structure that you can see everywhere, even in code samples as comments. This time, we're stamping a name on it and analyzing it further.

In Gherkin, a behavior description consists of three parts:

Given - a context
When - a cause
Then - a effect

In other words, the emphasis is on causality in a given context.

As I said, there are different levels you can apply this. Here's an example for such a behavior description from the perspective of its end user (this is called acceptance-level Statement):

Given a bag of tea costs $20
When I buy two of them
Then I pay 30$ due to promotion

And here's one for unit-level (note the line starting with "And" that adds to the context):

Given a list with 2 items
When I add another item
And check count of items
Then the count should be 3

While on acceptance level we put such behavior descriptions together with code as the same artifact (If this doesn't ring a bell, look at tools like SpecFlow or Cucumber or FIT to get some examples), on the unit level the description is usually not written down literally, but in form of code only. Still, this structure is useful when thinking about behaviors required from an object or objects, as we saw when we talked about starting from Statement rather than code. I like to put the structure explicitly in my Statements - I find that it makes them more readable. So most of my unit-level Statements follow this template:

[Fact]
public void Should__BEHAVIOR__()
{
  //GIVEN
  ...context...

  //WHEN
  ...trigger...

  //THEN
  ...assertions etc....
}

Sometimes the WHEN and THEN sections are not so easily separable - then I join them, like in case of the following Statement specifying that an object throws an exception when asked to store null:

[Fact]
public void ShouldThrowExceptionWhenAskedToStoreNull()
{
  //GIVEN
  var safeList = new SafeList();

  //WHEN - THEN
  Assert.Throws<Exception>(
    () => safeList.Store(null)
  );
}

By thinking in terms of these three parts of behavior, we may arrive at different circumstances (GIVEN) at which the behavior takes place, or additional ones that are needed. The same goes for triggers (WHEN) and effects (THEN). If anything like this comes to our mind, we add it to the TODO list.

TODO list... again!

As we said previously, a TODO list is a repository for anything that comes to our mind when writing or thinking about a Statement, but is not a part o the current Statement we're writing. We don't want to forget it, neither do we want it to haunt us and distract us from our current task, so we write it down as soon as possible and continue with our current task.

Suppose you're writing a piece of small logic that allows user access when he's an employee of a zoo, but denies access if he's a guest of the zoo. Then, after starting writing a Statement it gets to you that actually any employee can be a guest as well - for example, he might choose to visit the zoo with his family during his vacation. Still, the two previous rules hold, so not to allow this case to distract you, you quickly add an item to the TODO list (like "TODO: what if someone is an employee, but comes to the zoo as a guest?") and finish the current Statement. When you're finished, you can always come back to the list and pick item to do next.

There are two important questions related to TODO lists: "what exactly should we add as a TODO list item?" and "How to efficiently manage the TODO list?". We'll take care of these two questions now.

What to put on the TODO list?

Everything that you need addressed but is not part of the current Statement. Those items may be related to implementing unimplemented methods, to add whole functionalities (such items are usually followed my more fine-grained sub tasks as soon as you start implementing the item), there might be reminders to take a better look at something (e.g. "investigate what is this component's policy for logging errors") or questions about the domain that need to get answered. If you get carried away too much in coding that you forget to eat, you can even add a reminder ("TODO: get something to eat!"). The list is yours!

How to pick items from TODO list?

Which item to choose from the TODO list when you have several? I have no clear rule, although I tend to take into account the following factors:

Risk - if what I learn by implementing or discussing a particular item from the list can have big impact on design or behavior of the system, I tend to pick such items first. An example of such item is that you start implementing validation of a request that arrives to your application and want to return different error depending on which part of the request is wrong. Then, during the development, you discover that more than one part of the request can be wrong at a time and you have to answer yourself a question: which error code should be returned in such case? Or maybe the return codes should be accumulated for all validations and then returned as a list?
Difficulty - depending on my mental condition (how tired I am, how much noise is currently around my desk etc.), I tend to pick items with difficulty that best matches this condition. For example, after finishing an item that requires a lot of thinking and figuring out things, I tend to take on some small and easy items to feel wind blowing in my sails and to rest a little bit.
Completeness - in simplest words, when I finish test-driving an "if" case, I usually pick up the "else" next. For example, after I finish implementing a Statement saying that something should return true for values less than 50, then the next item to pick up is the "greater or equal to 50" case. Usually, when I start test-driving a class, I take items related to this class until I run out of them, then go on to another one.

Where to put a TODO list?

There are two ways of maintaining a TODO list. The first one is on a sheet of paper, which is nice, but requires you to take your hands off the keyboard, grab a pen or pencil and then get back to coding every time you think of something. Also, the only way a TODO item written on a sheet of paper can tell you which place in code it is related to, is (obviously) by its text.

Another alternative is to use a TODO list functionality built-in into an IDE. Most IDEs, such as Visual Studio (and Resharper plugin has its own enhanced version), Xamarin Studio or eclipse-based IDEs have such functionality. The rules are simple - you put special comments in the code and a special view in your IDE aggregates them for you, allowing you to navigate to each. Such lists are great because:

They don't force you to take your hands off keyboard to add an item to the list.
You can put such item in a certain place in the code where is makes sense and then navigate back to it with a click of a mouse. This, apart from other advantages, lets you write shorter notes than if you had to do it on paper. For example, a TODO item saying "TODO: what if it throws an exception?" looks out of place on the paper, but when added as a comment to your code in the right place, it's mighty sufficient.
Many TODO lists automatically add items for certain things. E.g. in C#, when you are yet to implement a method that was automatically generated by a tool, its body usually consists of a line that throws a NotImplementedException exception. Guess what - NotImplementedException occurences are added to the TODO list automatically, so don't have to manually add notes to implement the methods where they occur.

There are also some downsides. The biggest is that people often add TODO items for other means than to support TDD and they never go back to such items. Some people joke that a TODO left in the code means "Once, I wanted to...". Anyway, such items may pollute your TDD-related TODO list with so much cruft that your own items are barely findable. To work around it, I tend to use a different tag than TODO (many IDEs let you define your own tags, or support multiple tag types out of the box. E.g. with Resharper, I like to use "bug" tag, because this is something no one would leave in the code) and filter by it. Another options is, of course, getting rid of the leftover TODO items - if no one addressed it for a year, then probably no one ever will.

Another downside is that when you work with multiple workspaces/solutions, your IDE will gather TODO items only from current solution/workspace, so you'll have few TODO lists - one per workspace or solution. Fortunately, this isn't usually a big deal.

How to start with a test, not implementation - part 4

noreply@blogger.com (Grzegorz Gałęzowski) — Wed, 10 Jul 2013 19:14:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

(Note that I use "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post)

This is the last installment of "how to start with a test" series. This time, we're gonna take on the last of the typical techniques, useful when there's already some existing code to fit in.

Start by invoking a method when you have one

Sometimes, we have to add a new class that implements an existing interface required by another class. The fact of implementing an interface imposes what methods should the new class support. If this point is already decided, we can start our Statement by first calling the method and then discovering what we need to supply.

A simple example

Suppose we have an application holding a lot of data that, among other things, handles importing am existing database from another instance of the application. As importing a database can be a lengthy process, a message box is displayed each time when user chooses to perform the import and this message box displays the following message: "Johnny, please sit down and enjoy your coffee for a few minutes as we take time to import your database" (given user name is Johnny). The class that implements it looks like this:

public class FriendlyMessages
{
 public string 
 HoldOnASecondWhileWeImportYourDatabase(string userName)
 {
   return string.Format("{0}, "
     + "please sit down and enjoy your coffee "
     + "for a few minutes as we take time "
     + "to import your database",
     userName);
 }
}

Now, imagine that our management told us that they need to ship a trial version with some features disabled, including importing an existing database. Among all the things we need to do to make it happen, we also need to display a different string with message saying that this is a trial version and the feature is locked. We can do it by extracting an interface from the FriendlyMessages class and using it to put in an instance of another class implementing this interface when the application discovers that it is being run as a trial version. The extracted interface looks like this:

public interface Messages
{
  string HoldOnASecondWhileWeImportYourDatabase(string userName);
}

So our new implementation is forced to support the HoldOnASecondWhileWeImportYourDatabase method. Thus, we when implementing the class, we start with the following:

public class TrialVersionMessages : Messages
{
 public string HoldOnASecondWhileWeImportYourDatabase(string userName)
 {
   throw new NotImplementedException();
 }
}

Now, we're ready to start writing a Statement. Assuming we don't know where to start, we just start with creating an object and invoking the method that needs to be implemented:

[Fact] 
public void TODO()
{
 //GIVEN
 var trialMessages = new TrialVersionMessages();
 
 //WHEN
 trialMessages.HoldOnASecondWhileWeImportYourDatabase();

 //THEN
 Assert.True(false); //to remember about it
}

As you can see, we added an assertion that always fails at the end because we don't have any assertions yet and the Statement would otherwise be already evaluated as true and we'd rather have it remind ourselves that it is not finished. Other than this, the Statement does not compile anyway, because the method HoldOnASecondWhileWeImportYourDatabase takes a string argument and we passed none. This makes us ask the question what is this argument and what's its role in the behavior triggered by the HoldOnASecondWhileWeImportYourDatabase method. Seems like it is a user name and we want it to be somewhere in the result of the method. Thus, we can add it to the Statement like this:

[Fact] 
public void TODO()
{
 //GIVEN
 var trialMessages = new TrialVersionMessages();
 var userName = Any.String();
 
 //WHEN
 trialMessages.
  HoldOnASecondWhileWeImportYourDatabase(userName);

 //THEN
 Assert.True(false); //to remember about it
}

Now, this compiles but is evaluated as false because of the guard assertion that we put at the end. Our goal is to substitute it with a real assertion for a real expected result. The return value of the HoldOnASecondWhileWeImportYourDatabase is a string message, so all we need to do is to come up with the message that we expect in case of trial version:

[Fact] 
public void TODO()
{
 //GIVEN
 var trialMessages = new TrialVersionMessages();
 var userName = Any.String();
 
 //WHEN
 var message = trialMessages.
  HoldOnASecondWhileWeImportYourDatabase(userName);

 //THEN
 var expectedMessage = 
  string.Format("{0}, better get some pocket money!", userName);

 Assert.Equal(expectedMessage, message);
}

and all that is left is to find a good name for the Statement. This isn't an issue since we already specified the desired behavior in the code, so we can just summarize it as something like ShouldYieldAMessageSayingThatFeatureIsLockedWhenAskedForImportDatabaseMessage.

How to start with a test, not implementation - part 3

noreply@blogger.com (Grzegorz Gałęzowski) — Thu, 04 Jul 2013 07:42:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

(Note that I use "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post)

In the first part, I discussed how a good name is a great start for writing a Statement when there's no production code to invoke. In the second part, I elaborated on usefulness of thinking in terms of GIVEN-WHEN-THEN structure and translating it almost literally to code. Today, I'd like to introduce to you another technique - one that may appear awkward, but is actually very useful.

Start from the end

This is a technique that I suggest to people that seem to have absolutely no idea how to start. I got it from Kent Beck's book Test Driven Development by Example. It seems funny at start, but is quite powerful. The trick is to write the Statement 'backwards', i.e. starting with what the Statement asserts to be true (in terms of the GIVEN-WHEN-THEN structure, we'd say that we start with our THEN).

This works because, while we're many times quite sure of our goal (i.e. what the outcome of the behavior should be), but are unsure of how to get there.

A simple example

Imagine we're writing a class for granting access to a reporting functionality based on roles. We don't have any idea what the API should look like and how to write our Statement, but we know one thing: in our domain the access can be either granted or denied. Let's take the successful case (just because it's the first one we can think of) and, starting backwards, start with the following assertion:

//THEN
Assert.True(accessGranted);

Ok, that part was easy, but did we make any progress with that? Of course we did - we now have a non-compiling code and the compilation error is because of the accessGranted variable. Now, in contrast to the previous approach (with translating our GIVEN-WHEN-THEN structure into a Statement), our goal is not to make this compile as soon as possible. The goal is to answer ourselves a question: how do I know whether the access is granted or not? The answer: it is the result of authorization of the allowed role. Ok, so let's just write it down, ignoring everything that stands in our way (I know that most of us have a habit to add a class or a variable as soon as we find out that we need it. If you're like that, then please turn off this habit while writing Statements - it will only throw you off the track and steal your focus from what's important. The key to doing TDD successfully is to learn to use something that does not exist yet like it existed):

//WHEN
var accessGranted 
 = authorization.IsAccessToReportingGrantedTo(
  roleAllowedToUseReporting);

Note that we do not know what roleAllowedToUseReporting is, neither do we know what's authorization, but that didn't stop us from writing this line. Also, the IsAccessToReportingGrantedTo() method is just taken from the top of our head. It's not defined anywhere, it just made sense to write it like this, because it's the most direct translation of what we had in mind.

Anyway, this new line answers the question on where do we take the accessGranted from, but makes us ask further questions:

Where does the authorization variable come from?
Where does the roleAllowedToUseReporting variable come from?

As for authorization, we don't have anything specific to say about it other than that it is an object of a class that we don't have yet. In order to proceed, let's pretend that we have such a class. How do we call it? The instance name is authorization, so it's quite straightforward to name the class Authorization and instantiate it in the simplest way we can think of:

//GIVEN
var authorization = new Authorization();

Now for the roleAllowedToUseReporting. The first question that comes to mind when looking at this is: which roles are allowed to use reporting? Let's assume that in our domain, this is either an Administrator or an Auditor. Thus, we know what's going to be the value of this variable. As for the type, there are various ways we can model a role, but the most obvious one for a type that has few possible values is an enum. So:

//GIVEN
var roleAllowedToUseReporting = Any.Of(Roles.Admin, Roles.Auditor);

And so, working our way backwards, we have arrived at the final solution (we still need to give this Statement a name, but, provided what we already know, this is an easy task):

[Fact] public void
ShouldAllowAccessToReportingWhenAskedForEitherAdministratorOrAuditor()
{
 //GIVEN
 var roleAllowedToUseReporting = Any.Of(Roles.Admin, Roles.Auditor);
 var authorization = new Authorization();

 //WHEN
 var accessGranted = authorization
  .IsAccessToReportingGrantedTo(roleAllowedToUseReporting);

 //THEN
 Assert.True(accessGranted);
}

How to start with a test, not implementation - part 2

noreply@blogger.com (Grzegorz Gałęzowski) — Tue, 25 Jun 2013 21:18:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

(Note that I use "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post)

In the previous part, I discussed how a good name is a great start for writing a Statement when there's no production code to invoke. Today, I'll introduce to you another technique.

Start by filling the GIVEN-WHEN-THEN structure with the obvious

This is applicable when you come up with a GIVEN-WHEN-THEN structure for the Statement or a good name for it (GIVEN-WHEN-THEN structure can be easily derived from a good name and vice versa). Anyway, this method is about taking the GIVEN-WHEN-THEN parts and translating them almost literally into code, then add all the missing pieces that are required for the code to compile and run.

An example speaks a thousand words

Let's take a simple example of comparing two users. We assume that a user should be equal to another when it has the same name as the other one:

GIVEN a user with any name
WHEN I compare it to another user with the same name
THEN it should appear equal to this other user

Let's start with the translation

The first line:

GIVEN a user with any name

can be translated literally to code like this:

var user = new User(anyName);

Then the second line:

WHEN I compare it to another user with the same name

can be written as:

user.Equals(anotherUserWithTheSameName);

Great! Now the last line:

THEN it should appear equal to this other user

and its translation into the code:

Assert.True(areUsersEqual);

Ok, so we've made the translation, now let's summarize this and see what's missing to make this code compile:

[Fact] public void 
ShouldAppearEqualToAnotherUserWithTheSameName()
{
  //GIVEN
  var user = new User(anyName);

  //WHEN
  user.Equals(anotherUserWithTheSameName);

  //THEN
  Assert.True(areUsersEqual);
}

As we expected, this will not compile. Notably, our compiler might point us towards the following gaps:

A declaration of anyName variable.
A declaration of anotherUserWithTheSameName object.
The variable areUsersEqual is both not declared and it does not hold the comparison result.
If this is our first Statement, we might not even have the User class defined at all.

The compiler created a kind of a small TODO list for us, which is nice. Note that while we don't have full compiling code, filling the gaps boils down to making a few trivial declarations and assignments:

anyName can be declared as var anyName = Any.String();
anotherUserWithTheSameName can be declared as var anotherUserWithTheSameName = new User(anyName);
areUsersEqual can be declared and assigned this way: var areUsersEqual = user.Equals(anotherUserWithTheSameName);
If User class does not exist, we can add it by simply stating: public class User {}

Putting it all together:

[Fact] public void 
ShouldAppearEqualToAnotherUserWithTheSameName()
{
  //GIVEN
  var anyName = Any.String();
  var user = new User(anyName);
  var anotherUserWithTheSameName = new User(anyName);

  //WHEN
  var areUsersEqual = user.Equals(anotherUserWithTheSameName);

  //THEN
  Assert.True(areUsersEqual);
}

And that's it - the Statement is complete!

How to start with a test, not implementation - part 1

noreply@blogger.com (Grzegorz Gałęzowski) — Sun, 23 Jun 2013 09:29:00 +0000

(this post is adapted from my work-in-progress open source TDD tutorial)

Note that I use "Statement" instead of "test" and "Specification" instead of "Test Suite" in this post

Part 2 of this series is ready!

Whenever I sat down with a person that was about to write their first code in a Statement-first manner, the person would first stare at the screen, then at me, then would say: "what now?". It's easy to say: "You know how to write code, you know how to write a unit test for it, just this time start with the latter rather than the first", but for many people, this is something that blocks them completely. If you're one of them, don't worry - you're not alone. I decided to dedicate this series of posts solely to techniques for starting to write a Statement when there's no code. I do not use mocks in this series on purpose, to keep it simple. However, all of these techniques are proven to work when using mocks.

Start with a good name

It may sound obvious, but really some people are having trouble describing the behavior they expect from their code. If you can name such behavior, it's a great starting point.

I know some people don't pay attention to naming their Statements, mainly because they're considered as tests and second-level citizens - as long as they run and "prove the code does not contain defects", they're considered sufficient. We'll take a look at some examples of bad names and then I'd like to introduce to you some rules of good naming.

Consequences of bad naming

As I said, many people don't really care how their Statements are named. This is a symptom of treating the Specification as garbage or leftovers - something that just "runs through your code". Such situation is dangerous, because as soon as this kind of thinking is established, it leads to bad, unmaintainable Specification that looks more like lumps of accidental code put together in a haste than a living documentation. Imagine that your Specification consists of names like this:

TrySendPacket()
TrySendPacket2()
testSendingManyPackets()
testWrongPacketOrder1()
testWrongPacketOrder2()

and try for yourself how difficult it is to answer the following questions:

How do you know what situation each Statement describes?
How do you know whether the Statement describes a single situation, or few of them at the same time?
How do you know whether the assertions inside those Statements are really the right ones assuming each Statement was written by someone else or was written a long time ago?
How do you know whether the Statement should stay or be removed when you modify the functionality it specifies?
If your changes in production code make a Statement evaluate to false, how do you know whether the Statement is no longer correct or the production code is wrong?
How do you know whether you will not introduce a duplicate Statement for a behavior that's already specified by another Statement when adding to Specification originally created by another team member?
How do you estimate, by looking at the runner tool report, whether the fix for failing Statement will be easy or not?
How do you answer a new developer in your team when they ask you "what is this Statement for?"
How can you keep track of the Statements already made about the specified class vs those that still need to be made?

What's in a good name?

For the name of the Statement to be of any use, it has to describe the expected behavior. At minimum, it should describe what happens at what circumstances. Let's take a look at one of the names Steve freeman and Nat Pryce came up in their great book Growing Object Oriented Software Guided By Tests:

notifiesListenersThatServerIsUnavailableWhenCannotConnectToItsMonitoringPort()

Note few things about the name of the Statement:

It describes a behavior of an instance of a specified class. Note that it does not contain method name, because what is specified is not a method, but a behavior that has its entry point and expected result. The name simply tells that what an instance does (notifies listeners that server is unavailable) under certain circumstances (when cannot connect to its monitoring port). It's important because such description is what you can derive from thinking about responsibilities of a class, so you don't need to know any of its methods signatures or the code that's inside of the class. Hence, this is something you can come up without before implementing - you just need to know why you created this class and feed on it.
The name is long. Really, really, really don't worry about it. As long as you're describing a single behavior, it's alright. I know usually people are hesitant to give long names to the Statements, because they try to apply the same rules to those names as method names in production code. Let me make it clear - these two cases are different. In case of Statements, they're not invoked by anyone besides the automatic runner applications, so they won't obfuscate any code that would need to call them. Sure, we could put all the information in the comment instead of Statement name and leave the name short, like this:
```
[Fact]
//Notifies listeners that server 
//is unavailable when cannot connect
//to its monitoring port
public void Statement_002()
{
 //...
}
```
There are two downsides of this solution: one is that we now have to put extra information (Statement_002) which is required only by compiler, because every method needs to have a name - there's usually no value a human could derive from such a name. The second downside is that when the Statement is evaluated to false, the automated runner shows you the following line: Statement_002: FAILED - note that all the information included in the comment isn't present in the failure report. It's really better to receive a report such as: notifiesListenersThatServerIsUnavailableWhenCannotConnectToItsMonitoringPort: FAILED, because all the information about the Statement that fails is present in the runner window.
Using a name that describes a (single) behavior allows you to track quickly why the Statement is false when it is. Suppose a Statement is true when you start refactoring, but in the meantime it turns out to be false and the report in the runner looks like this: TrySendingHttpRequest: FAILED - it doesn't really tell you anything more than an attempt is made to send a HTTP request, but, for instance, does not tell you whether your specified object is the sender (that should try to send this request under some circumstances) or the receiver (that should handle such request properly). To actually know what went wrong, you have to go to the code to scan its source code. Now compare it to the following name: ShouldRespondWithAnAckWheneverItReceivedAHttpRequest. Now when it evaluates to false, you can tell that what is broken is that the object no longer responds with an ACK to HTTP request. Sometimes this is enough to deduct which part of the code is in fault of this evaluation failure.

My favourite convention

There are many conventions for naming Statements appropriately. My favorite is the one developed by Dan North, which makes each Statement name begin with the word Should. So for example, I'd name a Statement: ShouldReportAllErrorsSortedAlphabeticallyWhenItEncountersErrorsDuringSearch(). The name of the Specification (i.e. class name) answers the question "who should do it?", i.e. when I have a class named SortingOperation and want to say that it "should sort all items in ascending order when performed", I say it like this:

public class SortingOperationSpecification
{
 [Fact] public void
 ShouldSortAllItemsInAscendingOrderWhenPerformed()
 {
 }
}

It's important to focus on what result is expected from an object when writing names along this convention. If you don't, you quickly find it troublesome. As an example, one of my colleague was specifying a class UserId and wrote the following name for the Statement about comparison of two identifiers: EqualOperationShouldPassForTwoInstancesWithTheSameUserName(). Note that this is not from the perspective of a single object, but rather from the perspective of an operation that's executed on it, which means that we stopped thinking in terms of object responsibilities and started thinking in terms of operation correctness, which is farther away from our assumption that we're writing a Specification consisting o Statements. This name should be changed to: ShouldReportThatItIsEqualToAnotherObjectWhenItHasTheSameUserName().

And this is the end of part 1 of the series. In the next installment, we'll take a look at another technique - brute force translation of prose description written with GIVEN-WHEN-THEN structure to code.