Skip to content
This repository

ActiveSupport::JSON::Encoding::CircularReferenceError Using doubles #78

Closed
fellix opened this Issue August 26, 2011 · 5 comments

3 participants

Rafael Felix David Chelimsky Andrew Meyer
Rafael Felix

I've a simple controller like that:

  class ListsController < ApplicationController
    respond_to :json

    expose(:lists){ List.select("id, name") }

    def index
      respond_with(lists)
    end

  end

And a simple test:

  require 'spec_helper'

  describe ListsController do
    let(:list){ double("List") }


    describe "GET 'index'" do
      let(:expected){ [{id: "1", name: "test"}] }
      before do
        list.stub(:id){ "1" }
        list.stub(:name){ "test" }
        List.stub(:select){ [ list ] }
      end
      it "should return the list of lists" do
        get :index, format: :json
        response.body.should == expected.to_json
      end
    end
  end

And I'm getting

  Failure/Error: get :index, format: :json
     ActiveSupport::JSON::Encoding::CircularReferenceError:
       object references itself

Using the object instead of the double the error isn't raised.

David Chelimsky
Owner

This is a classic case of when NOT to use stubs. Here's why: there are three incorrect assumptions that are hiding behind the stubs:

  1. select takes an Array: List.select(["id","name"]), but the example stubs it incorrectly
  2. the id is numeric, not a String
  3. the json is wrapped: {"list":{"id":1,"name":"test"}}, but the example doesn't wrap it.

Even if the stubs were properly aligned with reality, the reason you're seeing the error you're seeing is that respond_with(lists) eventually calls as_json on the list object, which, in this example, is an RSpec double that doesn't implement as_json. You need to either use a stub_model (which does implement as_json), or explicitly stub it in the example:

list.stub(:as_json) { { list: {id: 1, name: "test"} } }

But I'd recommend that you avoid stubbing this in this case. Stubbing is great when you stub well defined (and understood) external APIs that are invoked by the code you are specifying. In this case, you'd need to stub a low-level API (as_json), which your code is not invoking directly. That means that if the Rails framework ever changed how it renders json, this example would pass errantly.

Here's how I'd approach this outside-in:

Start with a request spec:

require 'spec_helper'

describe "Lists" do
  describe "GET 'index.json'" do
    it "returns the list of lists" do
      list = List.create!(name: "test")
      get "/lists.json"
      response.body.should == [{list: {id: list.id, name: "test"}}].to_json
    end
  end
end

This shows exactly what to expect, so when you're working on clients you can refer directly to this without having to dig into internals.

Run this and it'll fail with "uninitialized constant List", so generate the list resource:

rails generate resource list name:string
rake db:migrate && db:test:prepare

Run it again and it'll fail with "ActionView::MissingTemplate". Now you have a couple of choices. The purist view says "write a controller spec", but some people say controller specs are unnecessary if you have request specs (or cukes) as they just add duplication. For me, the answer depends upon the complexity of the requirement as it compares to what we get for free from Rails. In this case, the only requirement that is different from what Rails gives us for free is that we constrain the fields to id and name, so I'd just implement this very simple code in the controller and move on:

class ListsController < ApplicationController
  respond_to :json

  def index
    respond_with List.all
  end
end

Now the request spec fails with:

expected: "[{\"list\":{\"id\":1,\"name\":\"test\"}}]"
     got: "[{\"list\":{\"created_at\":\"2011-08-27T14:56:19Z\",\"id\":1,\"name\":\"test\",\"updated_at\":\"2011-08-27T14:56:19Z\"}}]" (u

We're getting more fields than we want. I want the model responsible for constraining the keys in the json (Rails implements json transformations in the context of the model, so why shouldn't we?), so I'd add a model spec:

require 'spec_helper'

describe List do
  describe "#as_json" do
    it "constrains keys to id and name" do
      list = List.new(:name => "things")
      list.as_json['list'].keys.should eq(%w[id name])
    end
  end
end

This fails with:

expected ["id", "name"]
     got ["created_at", "name", "updated_at"]

I expect to see that it includes created_at and updated_at, but I'm surprised to see that the actual keys don't include the id. To get the id to show up in the list of keys, we can use create instead of new, or we can explicitly set the id. I'm going to go with setting the id explicitly to avoid the db hit, but this does feel like a leaky abstraction to me. It's all trade-offs.

it "constrains fields to id and name" do
  list = List.new(:name => "things")
  list.id = 37
  list.as_json['list'].keys.should eq(%w[id name])
end

Now it fails with:

expected ["id", "name"]
     got ["created_at", "id", "name", "updated_at"]

Now we can implement the constraint:

class List < ActiveRecord::Base
  def as_json
    super({ only: %w[id name]})
  end
end

Now the model spec passes, but the request spec fails with:

ArgumentError:
  wrong number of arguments (1 for 0)
# ./app/models/list.rb:2:in `as_json'

This is because the as_json implementation fails to honor the Rails API:

as_json(options = nil)

as_json is called by the rails framework with an options hash. Had we done this without the request spec and weren't aware of this information, we'd have a bunch of passing specs but the app would blow up. Hooray for testing at multiple levels!

So we add a new example to the model spec:

it "honors the submitted options hash" do
  list = List.new(:name => "things")
  list.id = 37
  list.as_json(:only => :name)['list'].keys.should eq(%w[name])
end

This fails with wrong number of arguments (1 for 0) as well, so now we adjust the model implementation:

def as_json(opts={})
  super({ only: %w[id name]}.merge(opts))
end

Now the model spec passes again, and so does the request spec! DONE!

The result is a very nice balance of clarity, speed (in spite of the one db hit) and flexibility. Any new endpoints we add will get the same json representation because it is expressed in the model (heeding the principle of least surprise). The model spec not only specifies how the model should represent itself as json, but it helps to explain how the rails framework uses the model. All of this with no stubbing at all, and especially no stubbing of low level APIs.

HTH, David

David Chelimsky dchelimsky closed this August 27, 2011
Rafael Felix

You're absolutely right,

I'm doing it using the object instead of a mock, you send me some very useful stuff to do that better, thanks a lot.

David Chelimsky
Owner

Mind if I post this on my blog? A lot of people get caught in the same trap of stubbing lower level APIs like this.

Rafael Felix

Thanks great, go on :)

Andrew Meyer
Ajedi32 commented May 31, 2013

Start with a request spec:

require 'spec_helper'

describe "Lists" do
  describe "GET 'index.json'" do
    it "returns the list of lists" do
      list = List.create!(name: "test")
      get "/lists.json"
      response.body.should == [{list: {id: list.id, name: "test"}}].to_json
    end
 end
end

This shows exactly what to expect, so when you're working on clients you can refer directly to this without having to dig into internals.

But wait, isn't that an integration test? That test depends on List being implemented correctly. Isn't that test only supposed to be testing the controller? =/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.