C# Advent Calendar 2019 – Refactoring RestSharp Sample Tests to Make Them More Maintainable

December 15, 2019 by Hilary

I’m excited to participate in the third C# Advent Calendar (my 2017 post was on combining UI and API automation with RestSharp and Selenium, and my 2018 post was about using the NUnit TestCaseData feature with your RestSharp tests to put a ton of tests in one) – you can check out the rest of the C# Advent Calendar for 2019 here: https://crosscuttingconcerns.com/The-Third-Annual-csharp-Advent.

This time, I wanted to refactor some of the sample tests I have out there, using RestSharp and NUnit (such as these posts). These are great one-off examples of API tests you can write, but when you add a few of them together, you start getting an unmaintainable mess that violates a lot of software design principles. Having test code that doesn’t meet the same design principles and practices as production code makes it easier to dismiss as “not real code”, when in fact it’s all code and it’s all important!

We’ll be working with the same codebase as most of my other examples, more specifically the RestSharp branch of the API tests (here: https://github.com/g33klady/TodoApiSample/blob/RestSharp/TodoApiTests/TodoIntegrationTests.cs). I’ll provide snippets below of a few tests that we’ll refactor so you don’t need to refer to the repo, but for completeness there’s the link!

Let’s look at what we’re starting with (from the link above, minus a few tests to keep it simple):

using NUnit.Framework;
using RestSharp;
using System;
using System.Collections;
using System.Collections.Generic;
using TodoApi.Models;


namespace TodoApiTests
{
    [TestFixture]
    public class TodoIntegrationTests
    {
        private static string _baseUrl;

        [OneTimeSetUp]
        public void TestClassInitialize()
        {
            //make sure this has the correct port!
            _baseUrl = "https://localhost:44350/api/Todo/";
        }

        [Test]
        public void VerifyGetReturns200Ok()
        {
            //Arrange
            var client = new RestClient(_baseUrl);
            var request = new RestRequest(Method.GET);

            //Act
            IRestResponse response = client.Execute(request);

            //Assert
            Assert.IsTrue(response.IsSuccessful, "Get method did not return a success status code; it returned " + response.StatusCode);
        }

        [Test]
        public void VerifyGetTodoItem1ReturnsCorrectName()
        {
            //Arrange
            var expectedName = "Walk the dog"; //we know this is what it should be from the Controller constructor
            var client = new RestClient(_baseUrl);
            var request = new RestRequest("1", Method.GET); //so our URL looks like https://localhost:44350/api/Todo/1

            //Act
            IRestResponse<TodoItem> actualTodo = client.Execute<TodoItem>(request);

            //Assert
            Assert.AreEqual(expectedName, actualTodo.Data.Name, "Expected and actual names are different. Expected " + expectedName + " but was " + actualTodo.Data.Name);
        }

        [Test]
        public void VerifyPostingTodoItemPostsTheItem()
        {
            //Arrange
            TodoItem expItem = new TodoItem
            {
                Name = "mow the lawn",
                DateDue = new DateTime(2019, 12, 31),
                IsComplete = false
            };
            var client = new RestClient(_baseUrl);
            var postRequest = new RestRequest(Method.POST);
            postRequest.RequestFormat = DataFormat.Json;
            postRequest.AddJsonBody(expItem);

            //Act
            //first we need to do the POST action, and get the new object's ID
            IRestResponse<TodoItem> postTodo = client.Execute<TodoItem>(postRequest);
            var newItemId = postTodo.Data.Id; //we need the ID to do the GET request of this item

            //now we need to do the GET action, using the new object's ID
            var getRequest = new RestRequest(newItemId.ToString(), Method.GET);
            IRestResponse<TodoItem> getTodo = client.Execute<TodoItem>(getRequest);

            //Assert
            Assert.AreEqual(expItem.Name, getTodo.Data.Name, "Item Names are not the same, expected " + expItem.Name + " but got " + getTodo.Data.Name);
            Assert.AreEqual(expItem.DateDue, getTodo.Data.DateDue, "Item DateDue are not the same, expected " + expItem.DateDue + " but got " + getTodo.Data.DateDue);
            Assert.AreEqual(expItem.IsComplete, getTodo.Data.IsComplete, "Item IsComplete are not the same, expected " + expItem.IsComplete + " but got " + getTodo.Data.IsComplete);
        }
    }   
}

In just these 3 tests, we’ve got a lot of duplication, and the tests are handling things they shouldn’t be concerned with. I see a few things we can start with to really clean up our code and make it way more maintainable (and so we don’t have to retype a bunch of stuff every time we write a new test!).

Move the RestClient instantiation to the OneTimeSetUp method – each test doesn’t need to instantiate it! When it’s in a OneTimeSetUp method, it’ll be initialized and reused for each test. I don’t know if this will cause a problem with threading or concurrency, or anything else. If you encounter issues, maybe move that to a SetUp method, which will be called before each test is run.
Create a Helper class with methods to make each API call. Each test does not need to handle that – if we have a bunch of test for the same calls that’s a ton of duplication and maintainability is dismal!
Create a Helper method to instantiate a new TodoItem. Our POST and PUT tests are going to need it, and each test shouldn’t have to handle it, either.

Steps 1 and 2 are kind of hand-in-hand so we’ll do those together. Let’s start with moving the RestClient instantiation up.

To do that, we’ll add a field to our class for the RestClient object, then we’ll add that client instantiation to the TestClassInitialize method:

namespace TodoApiTests
{
    [TestFixture]
    public class Refactored_TodoIntegrationTests
    {
        private static string _baseUrl;
        private static RestClient _client;

        [OneTimeSetUp]
        public void TestClassInitialize()
        {
            //make sure this has the correct port!
            _baseUrl = "https://localhost:44350/api/Todo/";
            _client = new RestClient(_baseUrl);
        }
...
   }
}

Before we update our tests to use the new _client object, we’ll create a new Helper class which will generate the requests that the _client uses.

Create a new class file, which I’ll call Helpers.cs. The first method I’ll create will be to refactor our first test – it’s a method to create that simple GET request. The method will return a RestRequest object, that is our basic GET of all TodoItems:

using RestSharp;
using System;
using System.Collections.Generic;
using System.Text;

namespace TodoApiTests
{
    public static class Helpers
    {
        public static RestRequest GetAllTodoItemsRequest()
        {
            var request = new RestRequest(Method.GET);
            //request.AddHeader("CanAccess", "true"); //here's where you might add a header to the request
            return request;
        }
    }
}

With this new method, I can now refactor my first test to use the new _client object, and this new request. I’ll remove the client instantiation from the //Arrange section, and update the request to use the Helper method:

    [TestFixture]
    public class Refactored_TodoIntegrationTests
    {
        private static string _baseUrl;
        private static RestClient _client;

        [OneTimeSetUp]
        public void TestClassInitialize()
        {
            //make sure this has the correct port!
            _baseUrl = "https://localhost:44350/api/Todo/";
            _client = new RestClient(_baseUrl);
        }

        [Test]
        public void VerifyGetReturns200Ok()
        {
            //Arrange
            var request = Helpers.GetAllTodoItemsRequest();

            //Act
            IRestResponse response = _client.Execute(request);

            //Assert
            Assert.IsTrue(response.IsSuccessful, "Get method did not return a success status code; it returned " + response.StatusCode);
        }
...
   }

So in total we’ve removed 1 line of code from that test. ‘After all of that?! We added more code than we removed! What is the point??’ you ask. The point is, it’s now way more maintainable. If we need to add a header to the requests, we don’t have to update every single test – just the Helper method. It’s easier to see with some of the other tests. Let’s add those to the Helper class too!

using RestSharp;
using System;
using System.Collections.Generic;
using System.Text;

namespace TodoApiTests
{
    public static class Helpers
    {
        public static RestRequest GetAllTodoItemsRequest()
        {
            var request = new RestRequest(Method.GET);
            //request.AddHeader("CanAccess", "true"); //here's where you would might a header to the request
            return request;
        }

        public static RestRequest GetSingleTodoItemRequest(long id)
        {
            var request = new RestRequest($"{id}", Method.GET);
            request.AddUrlSegment("id", id);
            return request;
        }

        public static RestRequest PostTodoItemRequest(TodoItem item)
        {
            var request = new RestRequest(Method.POST);
            request.RequestFormat = DataFormat.Json;
            request.AddJsonBody(item);
            return request;
        }
    }
}

So now we’ve got a helper method for each of the requests that our tests are making – a GET of all TodoItems, a GET of a single TodoItem, and a POST of a TodoItem. Obviously we’re missing some of the tests we could have, but I’ll leave that for next time.

Note in the GetSingleTodoItemRequest method, we need to change the URL. That’s because we’ve set the baseUrl already, but to get a single item we need its ID, so we’re appending that to the URL. Also note in the PostTodoItemRequest method, we’re passing in the item, and we’re adding the headers (in the form of RequestFormat), and adding the item in Json format to the request body.

Here’s what our tests look like after refactoring to use the new _client object and the Helper methods:

using NUnit.Framework;
using RestSharp;
using System;
using System.Collections;
using System.Collections.Generic;
using TodoApi.Models;


namespace TodoApiTests
{
    [TestFixture]
    public class Refactored_TodoIntegrationTests
    {
        private static string _baseUrl;
        private static RestClient _client;

        [OneTimeSetUp]
        public void TestClassInitialize()
        {
            //make sure this has the correct port!
            _baseUrl = "https://localhost:44350/api/Todo/";
            _client = new RestClient(_baseUrl);
        }

        [Test]
        public void VerifyGetReturns200Ok()
        {
            //Arrange
            var request = Helpers.GetAllTodoItemsRequest();

            //Act
            IRestResponse response = _client.Execute(request);

            //Assert
            Assert.IsTrue(response.IsSuccessful, "Get method did not return a success status code; it returned " + response.StatusCode);
        }

        [Test]
        public void VerifyGetTodoItem1ReturnsCorrectName()
        {
            //Arrange
            var expectedName = "Walk the dog"; //we know this is what it should be from the Controller constructor
            var request = Helpers.GetSingleTodoItemRequest(1);

            //Act
            IRestResponse<TodoItem> actualTodo = _client.Execute<TodoItem>(request);

            //Assert
            Assert.AreEqual(expectedName, actualTodo.Data.Name, "Expected and actual names are different. Expected " + expectedName + " but was " + actualTodo.Data.Name);
        }

        [Test]
        public void VerifyPostingTodoItemPostsTheItem()
        {
            //Arrange
            TodoItem expItem = new TodoItem
            {
                Name = "mow the lawn",
                DateDue = new DateTime(2019, 12, 31),
                IsComplete = false
            };
            var postRequest = Helpers.PostTodoItemRequest(expItem);


            //Act
            //first we need to do the POST action, and get the new object's ID
            IRestResponse<TodoItem> postTodo = _client.Execute<TodoItem>(postRequest);
            var newItemId = postTodo.Data.Id; //we need the ID to do the GET request of this item

            //now we need to do the GET action, using the new object's ID
            var getRequest = Helpers.GetSingleTodoItemRequest(newItemId);
            IRestResponse<TodoItem> getTodo = _client.Execute<TodoItem>(getRequest);

            //Assert
            Assert.AreEqual(expItem.Name, getTodo.Data.Name, "Item Names are not the same, expected " + expItem.Name + " but got " + getTodo.Data.Name);
            Assert.AreEqual(expItem.DateDue, getTodo.Data.DateDue, "Item DateDue are not the same, expected " + expItem.DateDue + " but got " + getTodo.Data.DateDue);
            Assert.AreEqual(expItem.IsComplete, getTodo.Data.IsComplete, "Item IsComplete are not the same, expected " + expItem.IsComplete + " but got " + getTodo.Data.IsComplete);
        }
    }
}

Again we only removed 1 line of code from the GET Single TodoItem test, but we removed 3 from the POST test. And we can remove even more when we do step 3 of our refactoring – creating a Helper method to instantiate the TodoItem! Let’s do that now.

This Helper method should return a default TodoItem that we can use if the values don’t really matter to the test, but we should also be able to pass in values to use if they do matter to the test. Here’s what I’ve come up with for this Helper method:

public static TodoItem GetTestTodoItem(string name = "mow the lawn", bool isCompleted = false, DateTime dateDue = default(DateTime))
{
    if (dateDue == default(DateTime))
    {
        dateDue = new DateTime(2029, 12, 31);
    }
    return new TodoItem
    {
        Name = name,
        DateDue = dateDue,
        IsComplete = isCompleted
    };
}

So it’s returning a TodoItem object, and the method signature is saying basically “if Name, IsCompleted, or DateDue aren’t provided, we’ll use defaults”. To set our default date, we have to do a bit of trickery. This is a short-term solution, since our DateDue value must be in the future to work. We could write some code to, say, use a date 2 weeks in the future. That’s a better idea than what I’ve got here – which is basically a timebomb. Another fun bug I’m leaving in this test application!

Anyway, now that we can use this default TodoItem whenever we want, we can remove a bunch more code from our POST test:

 [Test]
 public void VerifyPostingTodoItemPostsTheItem()
 {
     //Arrange
     var expItem = Helpers.GetTestTodoItem();
     var postRequest = Helpers.PostTodoItemRequest(expItem);

     //Act
     //first we need to do the POST action, and get the new object's ID
     IRestResponse<TodoItem> postTodo = _client.Execute<TodoItem>(postRequest);
     var newItemId = postTodo.Data.Id; //we need the ID to do the GET request of this item

     //now we need to do the GET action, using the new object's ID
     var getRequest = Helpers.GetSingleTodoItemRequest(newItemId);
     IRestResponse<TodoItem> getTodo = _client.Execute<TodoItem>(getRequest);

     //Assert
     Assert.AreEqual(expItem.Name, getTodo.Data.Name, "Item Names are not the same, expected " + expItem.Name + " but got " + getTodo.Data.Name);
     Assert.AreEqual(expItem.DateDue, getTodo.Data.DateDue, "Item DateDue are not the same, expected " + expItem.DateDue + " but got " + getTodo.Data.DateDue);
     Assert.AreEqual(expItem.IsComplete, getTodo.Data.IsComplete, "Item IsComplete are not the same, expected " + expItem.IsComplete + " but got " + getTodo.Data.IsComplete);
}

We’ve removed that item creation section and replaced it with grabbing an item via the Helper. We left it as it’s own var instantiation because we’re using that item in the assert statements later on.

So we’ve cleaned up a bit of code, moved some out to Helper methods, and made our tests a bit better design-wise. You can see the final product all together in the RestSharp-Refactor branch of the repo here: https://github.com/g33klady/TodoApiSample/tree/RestSharp-Refactor.

Would you approach this differently? What other ways can we refactor these tests?

Hilary

I’m Hilary Weaver, also known as g33klady on the Internets. I’m a Senior Quality Engineer working remotely near Detroit, I tweet a lot (@g33klady), and swear a lot, too.