12.19.2009

Testing tips (That is testability, not TDD part 2)

This is a continuation of my last post That is testability, not Test Driven Development! In it, I analyzed an article on the ADO.Net team blog Walkthrough: Test-Driven Development with the Entity Framework 4.0. I wanted to take that analysis a step further and share some insight from the testing perspective itself. There are two tests to review to produce a list of testing tips.

Update: Some of these test refactorings won’t work in the existing sample as they take dependencies to NUnit, you will need to add that in if you want to run these tests.

  1. Don’t setup what we don’t need. Before the first test case was even outlined, a large amount of unnecessary setup was created, some of which was never used. Tip: only write setup specifically for the given test case, in other words write the test case first and then the setup.
    namespace BlogTests
    {
        [TestClass]
        public class CommentTests
        {
            private IBloggingEntities _context;

        [TestInitialize]
        public void TestSetup()
        {
            Person p = new Person()
            {
                fn = "Jonathan",
                ln = "Aneja",
                email = "jonaneja@microsoft.com",
                User = new User() { Password = "password123" }
            };
    
    
            Blog b = new Blog()
            {
                Name = "Entity Framework Team Blog",
                Url = "http://blogs.msdn.com/adonet",
                User = p.User
            };
    
            Post post = new Post()
            {
                ID = 1,
                Blog = b,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Title = "Walkthrough: Test-Driven Development in Entity Framework 4.0",
                User = p.User,
                User1 = p.User,
                PermaUrl = b.Url + "/walkthrough-test-driven-development-in-Entity-Framework-4.0.aspx",
                Body = "This walkthrough will demonstrate how to..."
            };
    
            _context = new FakeBloggingEntities();
    
            var repository = new BlogRepository(_context);
    
            repository.AddPerson(p);
            repository.AddBlog(b);
            repository.AddPost(post);
    
            repository.SaveChanges();
        }
    }
    

    }

  2. Focus on the unit that is being tested, do not test it indirectly. In this example, the first test is to validate that the same comment isn’t added twice, however I see no call to any validate method in the test case!
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void AttemptedDoublePostedComment()
            {
                var repository = new BlogRepository(_context);

            Comment c = new Comment()
            {
                ID = 45,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Body = "EF4 is cool!",
                Title = "comment #1",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            Comment c2 = new Comment()
            {
                ID = 46,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Body = "EF4 is cool!",
                Title = "comment #1",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            repository.AddComment(c);
            repository.AddComment(c2);
    
            repository.SaveChanges();
        }
    
    public class BlogRepository
    {
        private IBloggingEntities _context;
        ...
        public void SaveChanges()
        {
            _context.SaveChanges();
        }
    }
    
    public class FakeBloggingEntities : IBloggingEntities, IDisposable
    {
        public int SaveChanges()
        {
            foreach (var comment in Comments)
            {
                ((IValidate)comment).Validate(ChangeAction.Insert);
            }
            return 1;
        }
    }
    
    public partial class Comment : IValidate
    {
      ...
      void IValidate.Validate(ChangeAction action) {
            if (action == ChangeAction.Insert)
            {
                //prevent double-posting of Comments
                if (this.Post.Comments.Count(c => c.Body == this.Body && c.Person.User == this.Person.User) > 1)
                    throw new InvalidOperationException("A comment with this exact text has already been posted to this entry");
            }
        }
    }</pre>The call to validate is indirectly tested through BlogRepository.SaveChanges which calls IBloggingEntities.SaveChanges which is tested with FakeBloggingEntities. FakeBloggingEntities is a stub class and not real code, it’s faking the infrastructure of the EF, when we should be testing our validate method directly. We should rewrite this test as follows and avoid the need for a repository expectation: <pre class="brush: c#">        [TestMethod]
        [ExpectedException(typeof(InvalidOperationException))]
        public void AttemptedDoublePostedComment()
        {
            var repository = new BlogRepository(_context);
    
            Comment c = new Comment()
            {
                ID = 45,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Body = "EF4 is cool!",
                Title = "comment #1",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            Comment c2 = new Comment()
            {
                ID = 46,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Body = "EF4 is cool!",
                Title = "comment #1",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            c.Validate(ChangeAction.Insert);
        }</pre>
    

  3. Within a test, avoid unnecessary setup in the scenario (arrange portion). We do not need to set ID,Created,Posted and title for this test case.
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void AttemptedDoublePostedComment()
            {
                var repository = new BlogRepository(_context);

            Comment c = new Comment()
            {
                Body = "EF4 is cool!",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            Comment c2 = new Comment()
            {
                Body = "EF4 is cool!",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            c.Validate(ChangeAction.Insert);
        }</pre>
    

  4. If there are other dependencies for a test case, we should avoid setting them up as global variables. Instead, create them as needed with helper methods. This increases readability and maintainability as we can quickly look at how the person and post are created without going to the setup method. It also decouples the test from the test class so it can be moved if needed when refactoring. Notice we no longer need the repository to pass setup from the setup method to the test case! This is production code that isn’t needed but was added without even testing it! (BlogRepository.GetPersonByEmail & BlogRepository.GetPostByID)
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void AttemptedDoublePostedComment()
            {
                var person = GetNewPerson();
                var post = GetNewPost();
                Comment c = GetNewComment();
                c.Body = "EF4 is cool!";
                c.Person = person;
                c.Post = post;
                Comment c2 = GetNewComment();
                c2.Body = "EF4 is cool!";
                c2.Person = person;
                c2.Post = post;

            c.Validate(ChangeAction.Insert);
        }
    
        private Comment GetNewComment()
        {
            return new Comment();
        } 
    
        private Person GetNewPerson()
        {
            return new Person { User = new User() };
        } 
    
        private Post GetNewPost()
        {
            return new Post();
        } </pre>
    

  5. Avoid duplicated strings. If they have meaning, then share that with a common variable. In this example the body is duplicated for testing validation, so duplicatedBody helps convey that in the name of the variable. Also, avoid meaningful variable values unless they are specific to the test case, to avoid distracting future readers. In this case we should replace duplicatedBody with a valid and simple value to imply that it isn’t important to the test case (“EF4 is cool!” now becomes “A”).
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void AttemptedDoublePostedComment()
            {
                var person = GetNewPerson();
                var post = GetNewPost();
                var duplicatedBody = "A";
                Comment c = GetNewComment();
                c.Body = duplicatedBody;
                c.Person = person;
                c.Post = post;
                Comment c2 = GetNewComment();
                c2.Body = duplicatedBody;
                c2.Person = person;
                c2.Post = post;

            c.Validate(ChangeAction.Insert);
        }</pre>
    

  6. Use AAA test naming to help convey what you are testing and give all variables meaningful names according to the scenario they represent. This method is testing the Validate operation (Act in AAA), the scenario (Arrange in AAA) is a duplicated comment, and the expectation (Assert in AAA) is an InvalidOperationException. The Act_Arrange_Assert naming style from Roy Osherove's The Art of Unit Testing allows test case comprehension without looking at the test code, much like an interface should tell the behavior regardless of implementation! We should rename the comment variables from c and c2 to firstComment and duplicatedComment (Note: if you cannot use var for your type and still understand what the variable represents then you have a misnamed variable).
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void Validate_DuplicatedComment_ThrowsInvalidOperationException()
            {
                var person = GetNewPerson();
                var post = GetNewPost();
                var duplicatedBody = "A";
                var firstComment = GetNewComment();
                firstComment.Body = duplicatedBody;
                firstComment.Person = person;
                firstComment.Post = post;
                var duplicatedComment = GetNewComment();
                duplicatedComment.Body = duplicatedBody;
                duplicatedComment.Person = person;
                duplicatedComment.Post = post;

            firstComment.Validate(ChangeAction.Insert);
        }</pre>
    

  7. Avoid attributed exception assertions as any code in the test could fulfill the assertion. Instead, use inline assertions to test this. In the example below we can setup an action (delegate) that will execute the method under test, then pass that to a method that runs it and will return a failing test if the exception is not thrown (Note: inline exception assertions are a feature in NUnit and several other test frameworks but not in MSTest that I know of).
            [TestMethod]
            public void Validate_DuplicatedComment_ThrowsInvalidOperationException()
            {
                var person = GetNewPerson();
                var post = GetNewPost();
                var duplicatedBody = "A";
                var firstComment = GetNewComment();
                firstComment.Body = duplicatedBody;
                firstComment.Person = person;
                firstComment.Post = post;
                var duplicatedComment = GetNewComment();
                duplicatedComment.Body = duplicatedBody;
                duplicatedComment.Person = person;
                duplicatedComment.Post = post;

            Action validate = () =&gt; firstComment.Validate(ChangeAction.Insert);
    
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }</pre>
    

  8. Arrange test code into three sections, Arrange, Act and Assert (AAA syntax). You will notice we already refactored to this when applying the other tips above. This way the first block of code is always the scenario (arrange), the second block is the action, or deferred action when testing exceptions. Finally the last block is the assertion. Notice how these flow naturally from the AAA test case naming convention. One of the things Roy points out in his book is that if you cannot name a method with AAA syntax then you have not thought through what the method is actually testing. If the authors of the blog post would have named the method with A_A_A syntax and organized it into AAA blocks, they would have noticed the operation under test (Validate) was not mirrored in the test code!
            [TestMethod]
            public void Validate_DuplicatedComment_ThrowsInvalidOperationException()
            {
                // Arrange
                var person = GetNewPerson();
                var post = GetNewPost();
                var duplicatedBody = "A";
                var firstComment = GetNewComment();
                firstComment.Body = duplicatedBody;
                firstComment.Person = person;
                firstComment.Post = post;
                var duplicatedComment = GetNewComment();
                duplicatedComment.Body = duplicatedBody;
                duplicatedComment.Person = person;
                duplicatedComment.Post = post;

            // Act
            Action validate = () =&gt; firstComment.Validate(ChangeAction.Insert);
    
            // Assert
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }</pre>
    

  9. Here is their other test, before applying any of the tips above:
            [TestMethod]
            [ExpectedException(typeof(InvalidOperationException))]
            public void AttemptBlankComment()
            {
                var repository = new BlogRepository(_context);

            Comment c = new Comment()
            {
                ID = 123,
                Created = DateTime.Now,
                Posted = DateTime.Now,
                Body = "",
                Title = "some thoughts",
                Person = repository.GetPersonByEmail("jonaneja@microsoft.com"),
                Post = repository.GetPostByID(1),
            };
    
            repository.AddComment(c);
            repository.SaveChanges();
        }</pre>Here is the test after. The Person and Post properties aren't needed for this test but they are required as the Validate tests with these things for our other test case. If these are part of a “default” test object for a comment we could move them to GetNewComment() and have them available for any test case. <pre class="brush: c#">        [TestMethod]
        public void Validate_BlankComment_ThrowsInvalidOperationException()
        {
            var blankComment = GetNewComment();
            blankComment.Body = string.Empty;
            blankComment.Person = GetNewPerson();
            blankComment.Post = GetNewPost();
    
            Action validate = () =&gt; blankComment.Validate(ChangeAction.Insert);
    
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }</pre>
    

  10. Going back to focusing on the unit itself, needing a post to validate duplicated comments is a smell. This highlights that the concern for validating a duplicated comment should be moved to the post class.
    public partial class Post : IValidate 
    { 
      ... 
      void IValidate.Validate(ChangeAction action) 
      { 
        //prevent double-posting of Comments 
        if (this.Comments.Any(c => this.Comments.Any(other => other != c && other.Body == c.Body && other.Person.User == c.Person.User))) 
          throw new InvalidOperationException("A comment with this exact text has already been posted to this entry");
       } 
    } 

    (Note: this is going to validate existing comments that are duplicated in addition to new ones, but it points out the issue of SRP for validation and that it really belongs here, maybe someone else can point out how we can work with change sets to determine if the duplication was already in place or is new)

    The first test would be moved to PostTests test class. We no longer need the relationship from Comment->Post in our domain, this is domain distillation! Notice how easy it would be to move our first test to a new PostTests class as we don't have a dependency to the setup method! If we needed to share the GetNewXyz() methods we could even move those to a base test fixture.
            // PostTests.cs
            [TestMethod]
            public void Validate_DuplicatedComment_ThrowsInvalidOperationException()
            {
                var person = GetNewPerson();
                var post = GetNewPost();
                var duplicatedBody = "A";
                var firstComment = GetNewComment();
                firstComment.Body = duplicatedBody;
                firstComment.Person = person;
                post.AddComment(firstComment);
                var duplicatedComment = GetNewComment();
                duplicatedComment.Body = duplicatedBody;
                duplicatedComment.Person = person;
                post.AddComment(duplicatedComment);

            Action validate = () =&gt; post.Validate(ChangeAction.Insert);
    
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }
    
        // CommentTests.cs
        [TestMethod]
        public void Validate_BlankComment_ThrowsInvalidOperationException()
        {
            var blankComment = GetNewComment();
            blankComment.Body = string.Empty;
    
            Action validate = () =&gt; blankComment.Validate(ChangeAction.Insert);
    
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }</pre>
    

    Look how simple the second test case is now! Beautiful, the intent is so clear!

  11. Don't write code beyond what a test case expects. In the sample above, we have ChangeAction.Insert as an input to validate but the test doesn’t dictate that in the scenario. We might want this test case to cover both insert and update. We could parameterize the test for both ChangeAction.Insert and ChangeAction.Update. This is using the NUnit framework for parameterized tests. I like to give a distinct name to each of the scenarios when I parameterize a test, so they are explicit when a test runner reports the output of a failure. If we wanted different behavior, we could create two tests but in this case we don't. Be very cautious not to go over board with parameterized tests, they should only be used in the very simplest of test case duplication.
            // CommentTests.cs
            [Test]
            [TestCase(ChangeAction.Insert, TestName="Validate_InsertBlankComment_ThrowsInvalidOperationException")]
            [TestCase(ChangeAction.Update, TestName="Validate_UpdateBlankComment_ThrowsInvalidOperationException")]
            public void Validate_BlankComment_ThrowsInvalidOperationException(ChangeAction changeAction)
            {
                var blankComment = GetNewComment();
                blankComment.Body = string.Empty;

            Action validate = () =&gt; blankComment.Validate(changeAction);
    
            Assert.Throws&lt;InvalidOperationException&gt;(validate);
        }</pre></li></ol>
    

    These are a few testing tips that I use to write and maintain tests every day.

    Happy Testing!





comments powered by Disqus