What tests should I write to test user validations for an admin-only feature?

Question

I created an RSpec spec to test if a POST #create action works properly:

describe "POST #create" do
  it "creates a career" do
    expect {
      post "/careers/", career:  attributes_for(:career)
    }.to change(Career, :count).by 1
  end
end

The above code works correctly. The issue happens when I create another test to allow only users with roles of "admin". Do I need to create a new user, log them in, and then run the above test? Do I need to do this for all future tests which have a restriction based on the user's role?

Is there another way to do this type of testing? 1) Just test if the create method works, and 2) only allow Users with "admin" role access the GET #new and POST #create methods?

Can anyone create a career, or only admins? – Dave Schweisguth Feb 09 '16 at 02:53 — Dave Schweisguth, Feb 09 '16 at 02:53
I'd like to have only admins create careers. – Ankit Patel Feb 09 '16 at 07:03 — Ankit Patel, Feb 09 '16 at 07:03
@Ankit - did you work this out? – jvillian Feb 19 '16 at 15:10 — jvillian, Feb 19 '16 at 15:10

score 1 · Answer 1 · answered Feb 09 '16 at 13:41

When your feature is fully developed you'll want to have the following tests:

one happy-path test in which an admin creates a career
one in which a non-admin tries to create a career but is prevented from doing so
possibly another in which a not-logged-in user tries to create a career but is prevented from doing so (whether you want this depends on whether you have to write different code to handle non-logged-in and logged-in non-admin users), and
possibly other tests of different scenarios in which an admin creates a career.

This idea of having one complete, happy-path test is one of the most fundamental patterns in testing, but I'm not aware that it has a name, other than being implied by the term "happy path".

It looks like you're doing TDD. Great! To get from where you are now to the above list of tests, the next test to write is the one where the non-logged-in user is prevented from creating a career. To make both tests pass at the same time you'll need to change the first test to log in an admin. And if you need more tests of successfully creating a career (bullet 4), yes, you'll need to log in an admin in those too.

Side notes:

Unless you already have it, I'd write your happy-path spec not as a controller spec but as a feature spec (an acceptance test), so that you specify the important parts of the UI and integration-test the entire stack. Your failed-authentication specs might work as controller specs, although you might decide you need to acceptance-test the UI when a user doesn't have permissions for at least one of those scenarios.
I really don't like that expect {}.to change syntax. It prevents you from making any other expectations on the result of posting. In your example I would want to expect that the HTTP response status is 200 (response.should be_success). As I said, though, my first spec would be a feature spec, not a controller spec.

Great comments on testing approach! I was intrigued by your `expect{}.to change` comment. I try to have only one expectation per `it` statement. So, if an `it` statement is failing it is very clear which expectation is not being met. — jvillian, Feb 18 '16 at 16:12
That's a common practice, but [in my opinion](http://stackoverflow.com/a/23550938/634576) a detrimental one. — Dave Schweisguth, Feb 19 '16 at 13:36
Thank you for the comment! It's great to have different perspectives. And, it's a good reminder to remain practical in these things. Personally, when I have tortured (or slow running) tests, I take it as an opportunity to think about why my test is tortured or slow. Often, there's an opportunity lurking for redesign/refactoring. Thanks again. — jvillian, Feb 19 '16 at 15:09

score 0 · Answer 2 · answered Feb 18 '16 at 16:09

So, this is an interesting question. Yes, you should definitely (IMO) test authentication separately from the target method/action. Each of these constitutes a unit of functionality and should be tested as such.

In my current project, I'm favoring POROs (I often keep them in a directory called 'managers' although I know many people prefer to call them 'services') for all sorts of things because it lets me isolate functionality and test it independently. So, I might end up with something like:

# controllers/foo_controller.rb
class FooController < ApplicationController
  before_action :authenticate

  def create
    @results = FooManager.create(params)
    redirect_to (@results[:success] ? my_happy_path : my_sad_path)
  end

  def authenticate
    redirect_to unauthorized_path unless AuthenticationManager.authenticate(params, request)
  end
end

# managers/foo_manager.rb
class FooManager
  class << self

    def create(params)
      # do a bunch of great stuff and return a hash (perhaps a 
      # HashWithIndifferentAccess, if you like) which will 
      # allow for evaluation of @results[:success] back in the
      # controller.
    end

  end
end

# managers/authentication_manager.rb
class AuthenticationManager
  class << self

    def authenticate(params, request)
      # do a bunch of great stuff and return a boolean
    end

  end
end

With an approach like this, I can very easily test FooManager.create and AuthenticationManager.authenticate (as well as FooController.create and FooController.authenticate routing) all independently. Hooray!

Now, whether your authentication framework or controller method is behaving correctly at a unit level, as Dave points out very well, is a separate issue from whether your entire system is behaving as expected. I'm with him on having high-level integration tests so you're clear about what 'done' looks like and you know when to holler 'ship it!'

What tests should I write to test user validations for an admin-only feature?

2 Answers2