Rspec, there's gotta be a better way to write this

in my system, there are 4 classes of users: Non-signed-in, Consumer, Producer and Admin.

I am currently using Cancan for ACL.

While writing rspec, I am seeing the following:

describe DealsController do 

  describe "non-signed-in users" do
    before(:each) do 
      @deal = Factory(:deal)
    end

    describe "should be able to" do 
      it "access index" do get :index end
      it "show deal" do get :show, :id => @deal end

      after(:each) do
        response.should be_success
      end
    end

    describe "should not be able to" do 
      it "redeem" do get :redeem end
      it "new" do get :new end
      it "edit" do get :edit, :id => @deal end
      it "update" do get :update, :id => @deal end
      it "destroy" do get :destroy, :id => @deal end

      after(:each) do
        response.should_not be_success
        response.should redirect_to(root_path)
        flash[:error].should == "Permission denied."
      end
    end
  end

  describe "consumers" do 
    before(:each) do 
      @user = test_sign_in(Factory(:user, :role => "consumer"))
      @deal = Factory(:deal)
    end

    describe "should开发者_如何学C be able to" do 
      it "access index" do get :index end
      it "show deal" do get :show, :id => @deal end

      after(:each) do
        response.should be_success
      end
    end

    describe "should not be able to" do 
      it "redeem" do get :redeem end
      it "new" do get :new end
      ...


      after(:each) do
        response.should_not be_success
        response.should redirect_to(root_path)
        flash[:error].should == "Permission denied."
      end
    end
 end

  describe "producer" do 
    before(:each) do 
      @user = test_sign_in(Factory(:user, :role => "producer"))
      @business = Factory(:business, :user_id => @user.id)
      @deal = Factory(:deal, :business_id => @business.id)
    end

    it "should be able to access index" do 
      get :index
      response.should be_success
    end

    describe "in show deals" do 
      it "should be able to see merchant controls for his deal" do 
        get :show, :id => @deal
        response.should have_selector('h3', :content => "Merchant Controls")
      end

      it "should not be able to see merchant controls for other's deal" do 
        @user2 = Factory(:user, :role => "producer")
        @business2 = Factory(:business, :user_id => @user2.id)
        @deal2 = Factory(:deal, :business_id => @business2.id)
        get :show, :id => @deal2

        response.should_not have_selector('h3', :content => "Merchant Controls")
      end
    end

    describe "should not be able to" do 
      it "new" do get :new end
      ...

      after(:each) do
        response.should_not be_success
        response.should redirect_to(root_path)
        flash[:error].should == "Permission denied."
      end
    end
 end

end

I haven't even filled in the admin section but I am pretty confident that this is not the recommended way to go about doing this.

What's a better way?

---

You should consider adopting a new style for one-liners. E.g. do something like this

describe "should be able to" do
  it "access index" { get :index }
  it "show deal"    { get :show, :id => @deal }
  after(:each)      { response.should be_success }
end

Also, you might consider creating a custom matcher for repeated multi-line expectations. For example, the following:

after(:each) do
  response.should_not be_success
  response.should redirect_to(root_path)
  flash[:error].should == flash
end

could be replaced with

after(:each) { response.should fail_redirect_and_flash(root_path, 'Permission denied.') }

using the following custom matcher code

RSpec::Matchers.define :fail_redirect_and_flash do |path,flash|
  match do |response|
    response.should_not be_success
    response.should redirect_to(path)
    flash[:error].should == flash
  end
end

Also, many people do not even bother writing controller unit tests because well designed controllers generally have very little code in them (they usually just set some variables using methods on a model and render/redirect, hence most of the testing actually happens in the models). Instead, they wrap both controller and view tests together and use cucumber. You still wind up with an equally ugly mess of code, but some people find it easier to manage.

On that note, you'll see that your spec "should not be able to see merchant controls for other's deal" is actually testing your view and not so much your controller. You should probably rip out whatever logic you use in your controller to show merchant controls and stick it in a helper and test it in isolation. This will help keep your controllers thin. E.g. you could have the following helper that you use in your views

def merchant_controls(deal, business)
  if business.can? :update, deal
    # render html
  end
end

And you could have a spec just for this helper method...

Describe "merchant_controls(deal, business)" do
  before(:all) do
    @business_a = create(:business)
    @deal_a = create(:deal, :business_id => @business_a)
    @business_b = create(:business)
    @deal_b = create(:deal, :business_id => @business_b)
  end

  it "a business should see merchant controls next to its own deals" do
    merchant_controls(@business_a, @deal_a).should eq("blahblah")
  end
  it "a business should not see merchant controls next to other business' deals" do
    merchant_controls(@business_a, @deal_b).should eq("")
  end
end

Hope this helps.

---

Honestly, if it's an application you care about - especially if it involves user interactions - having a ton of acceptance tests is more than okay. Obviously, don't get carried away, and start testing things like certain colours are visible or something. But if you're not thoroughly testing, you're not being the programmer you could be.

You may want to break your tests down in to more manageable tests though. Maybe make a separate physical file for each of the four user types. I can't remember, but I don't think this causes any sort of issue with RSpec.

Personally, I think using RSpec for acceptance tests is too terrible. Too gritty. I prefer, by a large margin, to use cucumber (http://www.cukes.info). It makes acceptance tests much easier, especially if you also want to test javascript. They're faster to write and cleaner, in my opinion. I'd look in to it a bit if I were you to see if it's right for you.