Background
Recently I have been working on projects that use the approach of Acceptance Test Driven Development (ATDD) along with Test Driven Development (TDD) . Fitnesse is the tool used for writing Acceptance tests. For those who are unaware of Acceptance tests, in layman terms these are the tests which validate the business functionality. Usually they are used to compare the output of Service calls. These tests are different from the normal unit tests written by developers. Acceptance tests are mainly written by business people. In most cases business folks are represented by Business Analysts (BA) and it is BA’s who are involved in writing these tests.
In software industry we have patterns and Anti Patterns. With all technologies and programming languages, we always find good patterns and the other ones which are not used in the proper sense. Such patterns are commonly called as Anti Patterns. I have come across few anti patterns while working with ATDD. Let me concentrate on one such anti pattern here.
A case of Acceptance Test Anti Pattern
Imagine we have a user in the system and need to display certain details related to the user on screen. Lets imagine we need to display the user ID which uniquely identifies the user, his first name and last name. The Business Analyst working on this user story will come up with an acceptance test which verifies the details pertaining to the user which would be returned by the service call. Everything seems very simple. But business users have a habit of making easier things difficult or we the developer community has made it a habit of complicating simple user requirements.
Imagine in the above scenario, the business users want to see the user name in concatenated form as first name + last name. In the fitnesse test the BA would set an expectation in the same manner. Here is a screenshot of the test business analyst would write in Fitnesse.
We can develop the required functionality in multiple ways.
Approach 1 : The simplest way to fulfil this expectation is to return the result from the service in the concatenated form and the fitnesse tests will be all green.
Approach 2 : Use an extension method in the client side.
Approach 3 : Use separation of concerns principle and make use of ViewModel from MVVM pattern.
I would say more than 90% of the developers would follow the first approach. Technically there is nothing wrong with that approach. But from an architectural point of view as well as from the point of implementing best practices it has some serious flaws. Lets dive into the code to see what and how. Lets look at the service implementation.
namespace UserManagementServices
{
public class UserManagementService : IUserManagementService
{
public UserDto GetUserdetails(int userId)
{
User domainUser = new User
{
Id = userId,
FirstName = "James",
LastName = "Bond"
};
return new UserDto
{
Id = domainUser.Id,
UserName = string.Format("{0} {1}", domainUser.FirstName, domainUser.LastName)
};
}
}
}
The service exposes a method named GetUserDetails which takes the userId as the input parameter. Then we instantiate a domain object which would ideally be returned from the persistent layer using some sort of ORM like Entity Framework or NHibernate or at least a hand coded Data Access Layer (DAL). These are outside the scope of this demo and hence I am instantiating a domain object directly. We then transform this domain object into a data transfer object (DTO) to return to the caller of the service. Note how the domain object has separate properties for Firstname and Last but the DTO has only one property called UserName.
Lets look at the code which acts as the glue between the Fitnesse test and the service call. We call these as Fixtures. Here is the implementation of UserFixture.
namespace AntiPattern1.FitnesseFixtures
{
public class UserFixture
{
public List<Object> Query()
{
UserManagementServices.UserManagementService service = new UserManagementServices.UserManagementService();
UserDto userdetails = service.GetUserdetails(1);
return new List<Object>
{
new List<object>
{
new List<string> { "Id", userdetails.Id.ToString() },
new List<object> { "UserName", userdetails.UserName }
}
};
}
}
}
The code is self explanatory. We create an instance of the service and call the GetUserDetails with userId as 1. The result is then transformed into the format which is required for the Fitnesse to parse the results. Lets run the fitnesse test and see the result.
The service works as expected. So lets make use of this service in a client. Lets build a Silverlight UI to display these results.
<Grid>
<Grid.RowDefinitions>
<RowDefinition />
<RowDefinition />
<RowDefinition />
</Grid.RowDefinitions>
<Grid.ColumnDefinitions>
<ColumnDefinition />
<ColumnDefinition />
</Grid.ColumnDefinitions>
<TextBlock Text="Id"
Grid.Row="0"
Grid.Column="0" />
<TextBlock Text="{Binding Id}"
Grid.Row="0"
Grid.Column="1" />
<TextBlock Text="User Name"
Grid.Row="1"
Grid.Column="0" />
<TextBlock Text="{Binding UserName}"
Grid.Row="1"
Grid.Column="1" />
</Grid>
We have a bare minimal UI controls to display two labels and two properties retrieved from the service. Lets look at the output.
I haven’t paid much attention to the layout of fields here, as can be seen from above screenshot the user name is displayed as expected in the concatenated form “James Bond”. Everything is working as expected.
But I personally have a problem with this approach. I believe we are mixing two concerns here. In every article related to Domain Driven Design (DDD), we’ll find that the object model that we define for our application should be as close to the domain model of the application. In that sense we have ensured that the User domain object in our application defines two properties for individually storing the first and the last name. But the other aspect is related to the presentation layer. The user wants to see concatenated data. And we are doing the concatenation in the service layer. This is where the separation of concerns should address our needs.
If we use MVVM pattern, the concatenation part should be taken care by the view model and the service should return two separate fields for first and the last name. Many people take a shortcut of using an ExtensionMethod or a partial class which exposes an additional property which does the same. To me it looks like an overkill to have a partial class. Its confusing in the first place and also a maintenance headache. I strongly recommend managing this through the view model.
To better understand my viewpoint, lets consider a use case where the user management service is accessed by another client and we are using WPF for this. In this application the business users need the same data but the representation is different. Lets look at the UI for this WPF application.
Oops, the users want to see first name and last name separately. What do we do now? Add another method to the service which returns first name and last name separately? In future we might have another client who wants an altogether different representation of the same data. The user details could be displayed in an desktop application or inside a desktop browser, it could be displayed on a smart phone or it could be a tablet. The possibilities are many. With each permutation and combination we can have different form factors and screen resolutions. Not all places would be best suited to display the users full name. There would be situations where we might need to display the names separately.
Worst case, even if there are no multiple tenants to the service, the same business users might decide to represent the details separately. If we concatenate the names in the service layer, it would force us to change the service implementation when the client needs changes. This I feel is against the principles of Service orientation and should be strictly avoided by the people practicing Service Oriented Architecture (SOA).
Every time we think of refactoring the service code, first and foremost question that should come to our mind is what will be the impact of refactoring if the same service is to be used with more that one client. That should give us a clear indication whether the changes needs to be done in the service layer or on the client side. The thumb rule is that it the change needs to be consistent across all clients it should be done in the service layer. If only one or two clients are impacted its better to do it in the client side.
My preferred way of solving this issue is to refactor the service and remove the concatenation logic from the service layer and handle it in the client side. Here is how I would refactor the service implementation.
public UserDto GetUserdetails(int userId)
{
User domainUser = new User
{
Id = userId,
FirstName = "James",
LastName = "Bond"
};
return new UserDto
{
Id = domainUser.Id,
FirstName = domainUser.FirstName,
LastName = domainUser.LastName
};
}
The UserDto now has two separate properties. Lets use this in the WPF client.
<StackPanel>
<Label Content="Id" />
<TextBlock Text="{Binding Id}" />
<Label Content="First Name" />
<TextBlock Text="{Binding FirstName}" />
<Label Content="Last Name" />
<TextBlock Text="{Binding LastName}" />
</StackPanel>
The service refactoring has fixed the issue with the WPF client but what about the Silverlight client which needs to display the name in the original concatenated form. Currently this is what we are doing to bind the results of the service call to the UI
private void Home_Loaded(object sender, System.Windows.RoutedEventArgs e)
{
UserManagementExposedServiceClient client = new UserManagementExposedServiceClient();
client.GetUserdetailsCompleted += (o, args) => client_GetUserdetailsCompleted(o, args);
client.GetUserdetailsAsync(10);
}
private void client_GetUserdetailsCompleted(object sender, GetUserdetailsCompletedEventArgs args)
{
UserDto userDto = args.Result;
this.DataContext = userDto;
}
We cannot continue doing this because there is no UserName property on the DTO which can be databound to the Textblock. So we introduce a view model which mediates between the view and the model. The view model is set as the data context of the view as
private void client_GetUserdetailsCompleted(object sender, GetUserdetailsCompletedEventArgs args)
{
UserDto userDto = args.Result;
UserViewModel viewModel = new UserViewModel(userDto);
DataContext = viewModel;
}
As we can see rom the above change, the view model takes the UserDto as constructor argument. The view is not even aware of the change that has happened. We can use the view model to format the data in whatever way we like. Here is the view model implementation.
public class UserViewModel
{
private readonly UserDto _userDto;
public UserViewModel(UserDto userDto)
{
_userDto = userDto;
}
public int Id
{
get
{
return _userDto.Id;
}
}
public string UserName
{
get
{
return string.Format("{0} {1}", _userDto.FirstName, _userDto.LastName);
}
}
}
There is nothing special in this code. It is all self explanatory. So we have overcome the problem in Silverlight application using the ViewModel approach. What about the Ftnesse from where we all started. If we run the test in its current state it would fail because the Dto no longer has the username property. How do we make the fitnesse test pass?
We can do the concatenation in the fixture code and it will work as expected the test would be green. But that is not recommended as it duplicates the code and the fixture has a logic built into it. The fixture should be used as a glue between the test and the service. There should not be any logic inside the fixture. If we relook at the purpose of Fitnesse, it is to validate the output of the service. So in its current state the service is returning FirstName and LastName but the fitnesse test is expecting the concatenated text. The Fitnesse test should really be validating that the service returns the first and last names as expected. I updated the fitnesse test and verify the output as follows
Conclusion
With latest advances in client side computing the views are becoming more and more dumb. One common approach used is to follow an architectural pattern like Model View Presenter (MVP) or Model View ViewModel (MVVM) which relies more on the data binding capabilities. These patterns clearly help us in separating the UI concerns and the business logic. As a result we can safely say that if we validate the output of the service we are at least sure that the binding will take care of the presentation concerns and the functional data would be represented correctly.
I fully understand and support the need for making the software more and more testable. The more ways we can test the software, the better understanding we can have about the system that we are building. Also different perspectives can help us discover different issues related to the quality of the software. The more amount of testing and specially the automated testing we can build around the software will help us in reducing regression issues and enhances the quality of the product or system being developed. All this is possible only if we use the tools available at our disposal in the right manner.
As one of my colleague said having a unit test doesn’t mean that you don’t write bad code, I would say having an acceptance test doesn’t mean we have tested the business functionality in the right manner. In my opinion there has to be the right balance. A tool like Fitnesse should be used for testing the service interfaces. It is not a tool to be used for doing GUI based testing. If you are using fitnesse for doing that I am sure you’ll have to violate many of the best practices and industry standards. And if you manage to still follow the best practices and industry standards, you might have to write a lot of boilerplate code to make the fitnesse tests pass. One way or the other it can lead to maintenance problems in the long run. My suggestion is to keep things simple and efficient. GUI based testing can be done using a browser plugin called Silenium. We need to use the right tool for the right purpose to get the best results possible.
As always I have uploaded the complete working solution which can be downloaded for further reference. Note that I have not included the fitnesses binaries and the fitnesse test code in this zip file. The test is only three lines and I preferred to just copy those 3 lines here because you’ll need to setup infrastructure for running Fitnesse test which is out of scope of this post. So here are the lines from the Fitnesse test
!|import|
|AntiPattern1.FitnesseFixtures|
!|query:UserFixture|
|Id|FirstName|LastName|
|1|James|Bond|
Until next time Happy Programming
Further Reading
Here are some books I recommend related to the topics discussed in this blog post.
Good stuff, thanks. Agree on the conclusion.
ReplyDeletetypo : Silenium/Selenium
A very useful and qualitative post on User Acceptance Testing.
ReplyDeleteGreat job & good work for sharing! :)