0

I wrote this scenario to test that when a user visits an unpublished entry they will see an error page:

Scenario: Unpublished entry
    Given there is an unpublished entry
    When a user visits the entry page
    Then he will see an error page

Steps

Given /^there is (?:an|one) unpublished entry$/ do
  @entry = create(:entry, published: false)
end

When /^a user visits the entry page$/ do
  visit entry_path(@entry)
end

Then(/^he will see an error page$/) do
  expect(response).to raise_error(ActiveRecord::RecordNotFound)
end

When running the test, it does not go past the second step because it fails with an ActiveRecord::RecordNotFound error; this is what I want, but it's happening on the wrong step.

When a user visits the entry page   # features/step_definitions/entry_steps.rb:17
  Couldn't find Entry with id=93 [WHERE "entries"."published" = 't'] (ActiveRecord::RecordNotFound)

Clearly I'm doing something wrong. How do I write my scenario to better express my intent, which is "visiting an unpublished entry will raise an error"?

Jumbalaya Wanton
  • 1,601
  • 1
  • 25
  • 47

1 Answers1

2

There is a problem with your web application if you are returning ActiveRecord::RecordNotFound to a user. Users should never see Rails errors because it could give them sensitive information about your server. Plus it just looks funky!

Here is what the pattern should be:

  1. User requests invalid item
  2. Rails tries to find invalid item and throws ActiveRecord::RecordNotFound
  3. Rails catches ActiveRecord::RecordNotFound and redirects user to 404 page.

Then in Cucumber, you would test for the 404 page.

Here is a list of common HTML error codes for your edification.

EDIT

Here is a great way to catch your ActiveRecord::RecordNotFound errors.

class SomeModel
  around_filter :catch_not_found

private
  def catch_not_found
    yield
  rescue
     redirect_to not_found_url
  end
end

EDIT 2

You may want to return 403 Forbidden instead of 404 Not Found

EDIT 3

Here is a detailed discussion on why catching exceptions with cucumber is weird. This is another argument for catching and redirecting.

Community
  • 1
  • 1
Dan Grahn
  • 9,044
  • 4
  • 37
  • 74
  • I disagree. He could be testing for the possibility that a user manually types an entry id in the address bar. Here the user is misusing the application, and it's not the developer's responsibility to protect him from a 404; there's little he can do to prevent this. For his end (the developer), he wants to be sure that an article that is unpublished does not get exposed via manually entering the url. – Mohamad Nov 18 '13 at 20:27
  • In which case it should throw an HTML error code. A web application should never be exposing an exception to the world. That is why there are HTML error codes! – Dan Grahn Nov 18 '13 at 20:29
  • I tend to agree, but in this particular case, where a user is potentially being malicious, I see no harm in displaying the Rails production error screen, which exposes very little other than there was an error of some sort. Going the extra mile to provide error handling for an error that should not happen with normal use seems like a generous undertaking for an undeserving recipient. – Mohamad Nov 18 '13 at 20:31
  • I think this is also a branding opportunity. It just looks tacky to return a standard error page. Instead if you return [something more interesting and helpful](http://www.smashingmagazine.com/2009/01/29/404-error-pages-one-more-time/), then the user may stick around. Returning the basic error page drives users away from your site reducing conversion. – Dan Grahn Nov 18 '13 at 20:34
  • 1
    Come to think of it, I think you are right. This type of test is more suited for the controller rather than cucumber, which is designed to test end user interaction. +1 – Mohamad Nov 18 '13 at 20:50
  • @Mohamad I love that we had this legitimate, well cultured debate and OP has completely gone AWOL. – Dan Grahn Nov 19 '13 at 12:46
  • hehehe! maybe he knows something we don't! – Mohamad Nov 19 '13 at 16:50