Raba - Defend your code RSS 2.0
# Saturday, August 01, 2009

Test methods must be written as production code.
While writing your tests you must act the same methods you write for your production code which means:

  1. human-readable (code standard, pattern etc.)
  2. code re-use (copy paste is never acceptable)
  3. etc...

This is a code you will probably find in your test projects

   1: [Test]
   2: public void GetCommentsByQuery_LookFormHmmmText_FindAtLeaseOne()
   3: {
   4:     SvnLogParser parser = new SvnLogParser();
   5:     string textToSearch = "Hmmm...";
   6:     var results = parser.GetCommentsByQuery(comment => comment.Contains(textToSearch));
   7:     CollectionAssertExtensions.IsNotEmpty(results);
   8: }
   9:  
  10: [Test]
  11: public void GetCommentsByQuery_LookFormBugText_FindAtLeastOne()
  12: {
  13:     SvnLogParser parser = new SvnLogParser();
  14:     string textToSearch = "bug";
  15:     var results = parser.GetCommentsByQuery(comment => comment.Contains(textToSearch));
  16:     CollectionAssertExtensions.IsNotEmpty(results);
  17: }

You can see that the two methods are pretty much the same, the only change is the textToSearch variable.
There are some excuses for such duplication:

  • "We want two different names... to easily understand the failiure reason"
  • "we want it to be easy to read, without context switches..." (method calls and un-needed inheritance complexity)

We, of course, can refactor this code, like this:

   1: [Test]
   2: public void GetCommentsByQuery_LookFormBugText_FindAtLeastOne()
   3: {
   4:     string textToSearch = "bug";
   5:     GetCommentsByQuery_AssertAllItemsStartsWith(textToSearch);
   6: }
   7:  
   8:  
   9: [Test]
  10: public void GetCommnentsByQuery_LookForHmmmText_VerifyInsideComment()
  11: {
  12:     string textToSearch = "Hmmm...";
  13:     GetCommentsByQuery_AssertAllItemsStartsWith(textToSearch);
  14: }
  15:  
  16: public void GetCommentsByQuery_AssertAllItemsStartsWith(string textToSearch)
  17: {
  18:     SvnLogParser parser = new SvnLogParser();
  19:     var results = parser.GetCommentsByQuery(comment => comment.StartsWith(textToSearch));
  20:  
  21:     CollectionAssertExtensions.AllItemsSatisfy(results, res => res.Comment.StartsWith(textToSearch));
  22: }

This is better when you think of clone detection: less identical rows and the logic was extracted to one method.
But people might say that they can't see the AAA (Arrange, Act, Assert) and they want it in one place,
Moreover, in such code sample, it is harder to find all the input parameters, cause they will be spread all over the TestFixture methods.

 

NUnit 2.5 added TestCaseAttribute, this can help us write tests that are:

  • more readable
  • shorter test-fixture
  • all-in-one-place tests
   1: [TestCase("Hm...")]
   2: [TestCase("bug")]
2: [TestCase("bug")]
   3: public void GetCommentsByQuery_LookForText_FindAtLeastOne(string textToSearch)
   4: {
   5:     SvnLogParser parser = new SvnLogParser();
   6:     var results = parser.GetCommentsByQuery(comment => comment.Contains(textToSearch));
   7:     CollectionAssertExtensions.IsNotEmpty(results);
   8: }

Here you can avoid the duplication and you won't need to refactor your Tests to more than one method, while you still see them running using the given parameter,
for example when the second parameter will fail you will see this output:

------ Test started: Assembly: App.Tests.dll ------

TestCase 'App.Tests.NewFolder1.SvnLogParserTests.GetCommentsByQuery_LookForText_FindAtLeastOne("Hm...")'
failed: 
  Expected: True
  But was:  False
    C:\ShaniData\ProjectsByTitle\Delver\App.Tests\NewFolder1\SvnLogParserTests.cs(75,0): at App.Tests.NewFolder1.CollectionAssertExtensions.IsNotEmpty[T]
    C:\ShaniData\ProjectsByTitle\Delver\App.Tests\NewFolder1\SvnLogParserTests.cs(33,0): at App.Tests.NewFolder1.SvnLogParserTests.GetCommentsByQuery_LookForText_Find...


4 passed, 1 failed, 0 skipped, took 0.78 seconds (NUnit 2.5).


The output will point the given method and the parameter used - which will ease finding the error.
You can extend reading about the usage here

Saturday, August 01, 2009 5:20:12 AM (GMT Daylight Time, UTC+01:00)  #    Comments [2] - Trackback
.Net 3.5 | C#3.0 | TDD | NUnit

Sunday, August 02, 2009 8:46:19 PM (GMT Daylight Time, UTC+01:00)
There's a problem with that solution - it violates you first rule - "readable" code.
A method named "GetCommentHelper" - has no meaning what so ever. When you read the following code, you have no idea what it does:
string textToSearch = "bug";

5: GetCommentsHelper(textToSearch);

You wonder to you self "GetCommentHelper" - hmmm, what does it do? It forces you to read that method code to understand it.

I would revise the naming of that helper to a name that signifies its meaning.

asaf mesika
Sunday, August 02, 2009 9:19:09 PM (GMT Daylight Time, UTC+01:00)
Hey Asaf,
You are right the name: GetCommentsHelper is bad - i will fix it.

Anyway, pay attention that:
1) this is not the best solution, the best solution is the TestCaseAttribute
2) even with a better naming, I am not sure that the second solution is more readable than the first solution.

Cheers,
Shani.
Comments are closed.
Archive
<February 2012>
SunMonTueWedThuFriSat
2930311234
567891011
12131415161718
19202122232425
26272829123
45678910
Disclaimer

Disclaimer
The opinions expressed herein are my own personal opinions and do not represent my employer's view in any way.

© Copyright 2012
Shani Raba
Sign In
Statistics
Total Posts: 145
This Year: 0
This Month: 0
This Week: 0
Comments: 97
Cool Stuff
Add to Technorati Favorites
Themes
Pick a theme:
All Content © 2012, Shani Raba
DasBlog theme 'Business' created by Christoph De Baene (delarou)