My first task on the first day of my new job at MONK Software was to refactor a web application that had grown into a monolith rails application.
Testing and refactoring are my favourite activities, so I started refactoring the pieces of code covered by tests.
Then I moved to the pieces of code not covered by tests. Having to write tests for code you didn’t write in the first place is a really fun experience. It helps you understand the business logic of the application and the reason behind every line of code.
A part of the application where tests were missing was the JSON API. And that’s what I will cover in this post.

JSON API Structure

The application implements a standard RESTful JSON API.
Let’s take, for example, the Continent resource of our application. A Continent has some basic fields – ID, name, priority, image – and the api call to get all the continents is

/api/v1/roaming/continents.json

While our backend implements a RESTful API, we have a quite strange JSON response, due to backward compatibility with some of the clients.
The json response of the previous call is:

{
  "version": "1.0",
    "response": {
      "status": 0,
      "reason": "Request successfully processed"
    },
    "continents": [{
      "id": "528ce3d151f3fc3ef3000002",
      "name": "Europe",
      "image": "http://domain.tdl/path/to/image.png"
    }, {
      "id": "528ce3d151f3fc3ef3000012",
      "name": "Asia",
      "image": "http://domain.tdl/path/to/image.png"
    }]
}

Also, every response has an HTTP status of 200, no matter the response succeded or failed. The status is passed in the response body itself, by the status key in the response hash.

The specs

Anyway, here is the first spec for this API call.

require 'spec_helper'
describe 'Continents API' do
  describe 'continents.json' do
    it "returns a 200 HTTP status" do
      get '/api/v1/roaming/continents.json'
      expect(response).to be_success
    end
  end
end

This first spec passed right away. Every time a spec passes without a failure first, I break the code just to see the spec fail for the right reason. Then I fix the code back.
The next spec tests that the response is a valid JSON response. In our case, a valid JSON response means that JSON contains:

  1. the version key set to “1.0”
  2. the response hash, which in turns contains a status set to 0 and a reason.

Here is the spec

...
it "returns a valid JSON response" do
  get '/api/v1/roaming/continents.json'
  json = JSON.parse(response.body)
  expect(json['version']).to eq("1.0")
  expect(json['response']['status']).to eq(0)
  expect(json['response']['reason']).to eq("Request successfully processed")
end
...

Again, the spec passed at the first run, so I broke the code to see it fail and than fix it back again.
The spec is really simple. First I make a request to the API endpoint. Then I parse the JSON response body. At that point I just check that the JSON response has the right value set for the version, status and reason keys.
I have two specs passing at this point. The natural next step is refactor! There is some duplication in our specs:

get '/api/v1/roaming/continents.json'

To remove the duplication I moved the line in a before block

...
before do
  get '/api/v1/roaming/continents.json'
end
...

Every time I need to test the JSON response, I know I will have to parse the response body with

JSON.parse(response.body)

so, I created a spec helper to take care of it for me

module Request
  module JsonHelpers
    def json
      @json ||= JSON.parse(response.body)
      end
    end
end
RSpec.configure do |config|
  config.include Requests::JsonHelpers, type: :request
end

Now, everytime I need to check the JSON response I can access the json method directly.
The three expects in our last spec serve to ensure that the response is a valid JSON response. Since I will use it all over the place in these API specs, I extracted them into a custom matcher

RSpec::Matchers.define :be_a_valid_api_response do
  match do |json_response|
    json_response['version'] == '1.0' &&
      json_response['response']['status'] == 0 &&
      json_response['response']['reason'] == 'Request successfully processed'
  end
end

After this round of refactoring the specs looked like this:

require 'spec_helper'
describe 'Continents API' do
  describe 'continents.json' do
    before do
      get '/api/v1/roaming/continents.json'
    end
    it "returns a 200 HTTP status" do
      expect(response).to be_success
    end
    it "returns a valid JSON response" do
      expect(json).to be_a_valid_api_response
    end
  end
end

The next spec tested the actual data. First I wrote a spec to check the number of continents returned

...
before do
  %w[Europe Asia America Africa Oceania].each do |name|
    FactoryGirl.create :continent, name: name
  end
  get '/api/v1/roaming/continents.json'
end
...
it "returns the right number of continents" do
  expect(json['continents'].lenght).to eq(5)
end
...

In the before block I added the five continent using FactoryGirl, and in the spec I checked the length of the continents array.
The last thing needed to be test was the format of the continent resource in the JSON response. Every continent needs an id, a name and an image.
Here is the spec:

...
it "returns the continents with the right format" do
  json['continents'].each do |continent|
    expect(continent.keys).to eq(["id", "name", "image"])
  end
end
...

For each continent, I check it contains the id, name and image keys. I used the keys method of the Array class to extract an array of keys. Then I check that array agains the expect array of keys I know the response needs to have.
This code works but it has a problem. If, for whatever reason the order of the keys changes in the response, the spec will fail. Here I’m not checking for the keys to be in the right order, but just their existence.
I solved the problem using the Set class.

require 'spec_helper'
require 'set'
...
it "returns the continents with the right format" do
  json['continents'].each do |continent|
    expect(continent.keys.to_set).to eq(
      ["id", "name", "image"].to_set
    )
  end
end
...

Now the spec will keep on passing even if the order of the keys changes.
I still don’t like this spec because it requires a lot of thinking to understand what it is really doing. It would be a lot easier to have a line like this

expect(continent).to match_the_format("id", "name", "image")

So I wrote another custom matcher

RSpec::Matchers.define :match_the_format do |*expected_format|
  require 'set'
    match do |json_resource|
      json_resource.keys.to_set == expected_format.to_set
    end
end

After this last refactor my whole spec file looked like this:

require 'spec_helper'
describe 'Continents API' do
  describe 'continents.json' do
    before do
      %w[Europe Asia America Africa Oceania].each do |name|
        FactoryGirl.create :continent, name: name
      end
      get '/api/v1/roaming/continents.json'
    end
    it "returns a 200 HTTP status" do
      expect(response).to be_success
    end
    it "returns a valid JSON response" do
      expect(json).to be_a_valid_api_response
    end
    it "returns the right number of continents" do
      expect(json['continents'].lenght).to eq(5)
    end
    it "returns the continents with the right format" do
      json['continents'].each do |continent|
        expect(continent).to match_the_format(
          "id", "name", "image")
        end
      end
    end
end

Conclusions

I’m quite happy with the specs so far. There are a few things I’d like to change, like moving, for example, the specs from the requests group to a custom api group.
Another possible refactor would be to group the common behavior of the apis into a shared examples group. These two specs

it "returns a 200 HTTP status" do
  expect(response).to be_success
end
it "returns a valid JSON response" do
  expect(json).to be_a_valid_api_response
end

are common to every successful api request, so they could be grouped into a shared example like this

shared_example "a successful api request" do
  it "returns a 200 HTTP status" do
    expect(response).to be_success
  end
  it "returns a valid JSON response" do
    expect(json).to be_a_valid_api_response
  end
end

and then used this way

require 'spec_helper'
describe 'Continents API' do
  describe 'continents.json' do
    ...
    it_behaves_like "a successful api request"
    ...
  end
end

This way you reduce duplication between specs without loosing clarity.
Anyway, I now have the version 1 of the api covered and I can move on refactoring them without the fear of breaking them 🙂

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes:

<a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <s> <strike> <strong>