Create a gist now

Instantly share code, notes, and snippets.

What would you like to do?
describe "GET current" do
before do
@request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
get :current, :format => 'js'
end
it { should respond_with(:success) }
it { should set_cookie(:hidden_notices).to("#{notices(:permanent).id}") }
it { should render_template('notices/current') }
end
end
# vs
test "GET current (or preferably an explanation WHY we are testing it)" do
@request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
get :current, :format => 'js'
assert respond_with(:success)
assert_equal "#{notices(:permanent).id}", cookies[:hidden_notices]
assert_template 'notices/current'
end
@jrwest

This comment has been minimized.

Show comment Hide comment
@jrwest

jrwest Mar 29, 2011

These aren't exactly the same thing, the example is a bit flawed. 2 options, in the RSpec example you could do the exact same thing with one "it" block and multiple shoulds (like your using one test block and multiple asserts) or you need to change the test/unit example to be 3 test cases. To be more accurate make the number of test cases and assertions per case the same. Then the examples look relatively similar and it boils down to do you prefer 'assert something' or 'object.should be_true'. I still prefer mini_test (test/unit) but I used to write a lot of my tests with RSpec.

describe "GET current" do
  it "does the same thing as your example" do
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    r = get :current, :format => 'js'

    r.should respond_with(:success) }
    r.should set_cookie(:hidden_notices).to("#{notices(:permanent).id}") }
    r.should render_template('notices/current') }
  end
end

jrwest commented Mar 29, 2011

These aren't exactly the same thing, the example is a bit flawed. 2 options, in the RSpec example you could do the exact same thing with one "it" block and multiple shoulds (like your using one test block and multiple asserts) or you need to change the test/unit example to be 3 test cases. To be more accurate make the number of test cases and assertions per case the same. Then the examples look relatively similar and it boils down to do you prefer 'assert something' or 'object.should be_true'. I still prefer mini_test (test/unit) but I used to write a lot of my tests with RSpec.

describe "GET current" do
  it "does the same thing as your example" do
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    r = get :current, :format => 'js'

    r.should respond_with(:success) }
    r.should set_cookie(:hidden_notices).to("#{notices(:permanent).id}") }
    r.should render_template('notices/current') }
  end
end
@justinko

This comment has been minimized.

Show comment Hide comment
@justinko

justinko Mar 29, 2011

In the T/U example, if assert respond_with(:success) fails, the two assertions that come after it will not run. This is not the case with the RSpec example.

@jrwest is correct - they are not the same.

In the T/U example, if assert respond_with(:success) fails, the two assertions that come after it will not run. This is not the case with the RSpec example.

@jrwest is correct - they are not the same.

@wilkerlucio

This comment has been minimized.

Show comment Hide comment
@wilkerlucio

wilkerlucio Mar 30, 2011

quick question, test unit has some before/after related commands?

quick question, test unit has some before/after related commands?

@christianromney

This comment has been minimized.

Show comment Hide comment
@christianromney

christianromney Mar 30, 2011

A lot of it has to do with taste. For me, language is very important. something.should equal(:something_else) sounds a hell of a lot more natural where assert_equals something, :something_else sounds very robotic and requires me to translate from computer language to human language, however briefly. Now, I also think that more complete examples have a tendency to spin out of control, especially with nested describe blocks, and before blocks thrown in. (Have a look at generated specs for controllers - UGH).

A lot of it has to do with taste. For me, language is very important. something.should equal(:something_else) sounds a hell of a lot more natural where assert_equals something, :something_else sounds very robotic and requires me to translate from computer language to human language, however briefly. Now, I also think that more complete examples have a tendency to spin out of control, especially with nested describe blocks, and before blocks thrown in. (Have a look at generated specs for controllers - UGH).

@justinko

This comment has been minimized.

Show comment Hide comment
@justinko

justinko Mar 30, 2011

@xmlblog - It really is about taste.

ActiveRecord & Datamapper
Prototype & JQuery
Postgresql & mysql
Vim & emacs
And much much more

Why do people pick on RSpec?

@xmlblog - It really is about taste.

ActiveRecord & Datamapper
Prototype & JQuery
Postgresql & mysql
Vim & emacs
And much much more

Why do people pick on RSpec?

@lucianosousa

This comment has been minimized.

Show comment Hide comment
@lucianosousa

lucianosousa Mar 30, 2011

confused comparing.

tdd is not equal bdd.

confused comparing.

tdd is not equal bdd.

@jrwest

This comment has been minimized.

Show comment Hide comment
@jrwest

jrwest Mar 30, 2011

Personally, I switched from RSpec back to Test/Unit (MiniTest) for 3 Reasons:

  • It is true that when you start nesting more and more describe/context blocks, specs become hard to read even with code folding. I started to prefer long method names describing context than trying to remember what sub-sub-sub-sub context I was in 500 lines into a spec file.
  • I was working on a project that was using mini test and I realized I was faster writing complex expressions and using simple assertions (assert, assert_true) than trying to remember all the really cool rspec matchers or being tempted to go on a tangent and write my own.
  • I really like the assert_performance_* class of assertions provided by mini_test

The change to MiniTest was key in my switch back especially because I could use the RSpec DSL a bit while I was switching over. I still will use and do use RSpec from time to time if let's say a project is already using it or someone asks me to.

jrwest commented Mar 30, 2011

Personally, I switched from RSpec back to Test/Unit (MiniTest) for 3 Reasons:

  • It is true that when you start nesting more and more describe/context blocks, specs become hard to read even with code folding. I started to prefer long method names describing context than trying to remember what sub-sub-sub-sub context I was in 500 lines into a spec file.
  • I was working on a project that was using mini test and I realized I was faster writing complex expressions and using simple assertions (assert, assert_true) than trying to remember all the really cool rspec matchers or being tempted to go on a tangent and write my own.
  • I really like the assert_performance_* class of assertions provided by mini_test

The change to MiniTest was key in my switch back especially because I could use the RSpec DSL a bit while I was switching over. I still will use and do use RSpec from time to time if let's say a project is already using it or someone asks me to.

@p8

This comment has been minimized.

Show comment Hide comment
@p8

p8 Mar 30, 2011

There's one 'end' too much in the rspec example.

If you'd use Cucumber instead of a controller test, it would be clear for other developers why you're setting the cookie.
Scenario: Refreshing my notices
Given I have some notices
When I refresh my notices
Then I should only see my permanent notices on some page.

This will also make sure the correct keys are used for both getting and setting :hidden_notices

p8 commented Mar 30, 2011

There's one 'end' too much in the rspec example.

If you'd use Cucumber instead of a controller test, it would be clear for other developers why you're setting the cookie.
Scenario: Refreshing my notices
Given I have some notices
When I refresh my notices
Then I should only see my permanent notices on some page.

This will also make sure the correct keys are used for both getting and setting :hidden_notices

@p8

This comment has been minimized.

Show comment Hide comment
@p8

p8 Mar 30, 2011

If you're advocating using classes for contexts shouldn't it be:

class GetCurrentForSomeControllerTest < ActiveSupport::TestCase
  def setup
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get :current, :format => 'js'
  end

  test "responds with success" do
    assert respond_with(:success)
  end

  test "sets cookies because when I am on some page I only need to see my permanent notices" do
    assert_equal "#{notices(:permanent).id}", cookies[:hidden_notices]
  end

  test "renders the notices template because..." do
    assert_template 'notices/current'
  end
end

p8 commented Mar 30, 2011

If you're advocating using classes for contexts shouldn't it be:

class GetCurrentForSomeControllerTest < ActiveSupport::TestCase
  def setup
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get :current, :format => 'js'
  end

  test "responds with success" do
    assert respond_with(:success)
  end

  test "sets cookies because when I am on some page I only need to see my permanent notices" do
    assert_equal "#{notices(:permanent).id}", cookies[:hidden_notices]
  end

  test "renders the notices template because..." do
    assert_template 'notices/current'
  end
end
@eparreno

This comment has been minimized.

Show comment Hide comment
@eparreno

eparreno Mar 30, 2011

"Opinionated" example. As we've already seen you can do the same with RSpec.
IMHO Some people loves sugar and some not, and this is not bad. Use tools that you feel comfortable with

"Opinionated" example. As we've already seen you can do the same with RSpec.
IMHO Some people loves sugar and some not, and this is not bad. Use tools that you feel comfortable with

@jacaetevha

This comment has been minimized.

Show comment Hide comment
@jacaetevha

jacaetevha Mar 31, 2011

@justinko the only reason your statement is true is because of the way @dhh wrote the example. With the "shoulds" in a single "it" block the last 2 "should" statements will not run if the first fails, just as in T/U. @dhh is either being disingenuous or ignorant in this example -- take your pick.

@justinko the only reason your statement is true is because of the way @dhh wrote the example. With the "shoulds" in a single "it" block the last 2 "should" statements will not run if the first fails, just as in T/U. @dhh is either being disingenuous or ignorant in this example -- take your pick.

@rubypanther

This comment has been minimized.

Show comment Hide comment
@rubypanther

rubypanther Mar 31, 2011

rspec has more lolcats, surely that is worth some points?

rspec has more lolcats, surely that is worth some points?

@zerothabhishek

This comment has been minimized.

Show comment Hide comment
@zerothabhishek

zerothabhishek Apr 1, 2011

As a newcomer to ruby/rails testing, I found Test:Unit much easier to get started with. Switched to rspec recently when was asked to, and have a feeling it fits more smoothly into the BDD workflow. Still, Test::Unit is surely not un-cool!

As a newcomer to ruby/rails testing, I found Test:Unit much easier to get started with. Switched to rspec recently when was asked to, and have a feeling it fits more smoothly into the BDD workflow. Still, Test::Unit is surely not un-cool!

@btakita

This comment has been minimized.

Show comment Hide comment
@btakita

btakita Apr 1, 2011

Why have controller specs/tests? Testing on Rack is so much cleaner and easier to maintain.
I (nor the user) don't care what template got rendered, I just care what content got rendered.

Here's how I would really want to test this:

describe "GET /notices/current" do
  it "removes the prefix from the hidden_notices cookie and renders the current notices" do
    request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get "/notices/current", {}, {"HTTP_ACCEPT" => "text/javascript"}

    response.code.should == 200
    cookies[:hidden_notices].should == "#{notices(:permanent).id}"
    response.body.should include("something I actually care about")
  end    
end

btakita commented Apr 1, 2011

Why have controller specs/tests? Testing on Rack is so much cleaner and easier to maintain.
I (nor the user) don't care what template got rendered, I just care what content got rendered.

Here's how I would really want to test this:

describe "GET /notices/current" do
  it "removes the prefix from the hidden_notices cookie and renders the current notices" do
    request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get "/notices/current", {}, {"HTTP_ACCEPT" => "text/javascript"}

    response.code.should == 200
    cookies[:hidden_notices].should == "#{notices(:permanent).id}"
    response.body.should include("something I actually care about")
  end    
end
@christianromney

This comment has been minimized.

Show comment Hide comment
@christianromney

christianromney Apr 1, 2011

@justinko

This comment has been minimized.

Show comment Hide comment
@justinko

justinko Apr 1, 2011

@btakita - "removes the prefix from the hidden_notices cookie and renders the current notices" <<- Isn't that a little much to digest? What if you needed to add another "should" in the example? The example description would get even more out of control.

IMO, one expectation per example really keeps things maintainable.

justinko commented Apr 1, 2011

@btakita - "removes the prefix from the hidden_notices cookie and renders the current notices" <<- Isn't that a little much to digest? What if you needed to add another "should" in the example? The example description would get even more out of control.

IMO, one expectation per example really keeps things maintainable.

@btakita

This comment has been minimized.

Show comment Hide comment
@btakita

btakita Apr 2, 2011

@xmlblog - Sometimes "brittle" tests are ok and part of the normal development feedback loop. A lot of the time the content rarely changes so it's not brittle anyways. If it is brittle, then the test can change. I guess it can be annoying at times, but whatevs.
Sure I can accept that you want to know what template gets rendered. I actually like the convention where templates render a div with a custom html attribute which contains the template path. It's easy to tell what templates are rendered when viewing the page in the browser.

@justinko - Yeah, there's multiple pieces of behavior there. Splitting things up works out well in certain situations. Sometimes it's easier just to have it all in one spec. Of course personal taste plays into this.

btakita commented Apr 2, 2011

@xmlblog - Sometimes "brittle" tests are ok and part of the normal development feedback loop. A lot of the time the content rarely changes so it's not brittle anyways. If it is brittle, then the test can change. I guess it can be annoying at times, but whatevs.
Sure I can accept that you want to know what template gets rendered. I actually like the convention where templates render a div with a custom html attribute which contains the template path. It's easy to tell what templates are rendered when viewing the page in the browser.

@justinko - Yeah, there's multiple pieces of behavior there. Splitting things up works out well in certain situations. Sometimes it's easier just to have it all in one spec. Of course personal taste plays into this.

@mattwynne

This comment has been minimized.

Show comment Hide comment
@mattwynne

mattwynne Apr 14, 2011

Just read the two versions out loud, they speak for themselves. See also rspec foo.rb -f s

I'm surprised you don't get it, @dhh, you've always been a wonderfully big advocate of communicative code.

Just read the two versions out loud, they speak for themselves. See also rspec foo.rb -f s

I'm surprised you don't get it, @dhh, you've always been a wonderfully big advocate of communicative code.

@rubypanther

This comment has been minimized.

Show comment Hide comment
@rubypanther

rubypanther Apr 14, 2011

@mattwynne: if when confronted with differing opinions you assume the other party simply doesn't "get it" you prevent yourself from useful communication entirely.

Also, the read out loud thing makes a lot of implicit assumptions not related to "communicative code." Communicative code does not automatically mean that when it's spoken it will communicate well. It usually means when it's read it will communicate well. Punctuation in code is not easily read, which is one of many reasons why code is usually read, not spoken. While it is certainly possible to write code that is fluidly spoken, this is not any sort of requirement of clear code. Indeed in Ruby there is lots of punctuation!

oh_noes! unless @ruby.has_punctuation?

@mattwynne: if when confronted with differing opinions you assume the other party simply doesn't "get it" you prevent yourself from useful communication entirely.

Also, the read out loud thing makes a lot of implicit assumptions not related to "communicative code." Communicative code does not automatically mean that when it's spoken it will communicate well. It usually means when it's read it will communicate well. Punctuation in code is not easily read, which is one of many reasons why code is usually read, not spoken. While it is certainly possible to write code that is fluidly spoken, this is not any sort of requirement of clear code. Indeed in Ruby there is lots of punctuation!

oh_noes! unless @ruby.has_punctuation?

@mattwynne

This comment has been minimized.

Show comment Hide comment
@mattwynne

mattwynne Apr 14, 2011

@rubypanther you're right, it was off-hand of me to say "get it". What I meant was "appreciate why people find this a useful technique". RSpec isn't The Silver Bullet, but if it's working for a significant number of people, I think it's healthy for us to be curious about why that's the case.

Personally, I really find that whether I'm reading code out loud or in my head, the closer it is to the way I'd describe the behaviour in English, the less likely I am to miss a mistake.

@rubypanther you're right, it was off-hand of me to say "get it". What I meant was "appreciate why people find this a useful technique". RSpec isn't The Silver Bullet, but if it's working for a significant number of people, I think it's healthy for us to be curious about why that's the case.

Personally, I really find that whether I'm reading code out loud or in my head, the closer it is to the way I'd describe the behaviour in English, the less likely I am to miss a mistake.

@rubypanther

This comment has been minimized.

Show comment Hide comment
@rubypanther

rubypanther Apr 14, 2011

@mattwaynne: I don't think it's a matter of word choice at all. You're still replacing his opinion with lack of knowledge.

He's been quite clear about it. In fact, that's largely his point: it's heavily used because of a cargo cult. The question you'd have him ask is exactly the question to which his answer brought us all here.

@mattwaynne: I don't think it's a matter of word choice at all. You're still replacing his opinion with lack of knowledge.

He's been quite clear about it. In fact, that's largely his point: it's heavily used because of a cargo cult. The question you'd have him ask is exactly the question to which his answer brought us all here.

@igbanam

This comment has been minimized.

Show comment Hide comment
@igbanam

igbanam Jul 28, 2011

@jrwest there is no need for the r variable.

describe "GET current" do
  it "does the same thing as your example" do
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get :current, :format => 'js'

    response.should respond_with(:success) }
    response.should set_cookie(:hidden_notices).to("#{notices(:permanent).id}") }
    response.should render_template('notices/current') }
  end
end

But in the end still it comes down to taste.

igbanam commented Jul 28, 2011

@jrwest there is no need for the r variable.

describe "GET current" do
  it "does the same thing as your example" do
    @request.cookies['hidden_notices'] = "1,#{notices(:permanent).id}"
    get :current, :format => 'js'

    response.should respond_with(:success) }
    response.should set_cookie(:hidden_notices).to("#{notices(:permanent).id}") }
    response.should render_template('notices/current') }
  end
end

But in the end still it comes down to taste.

@justinko

This comment has been minimized.

Show comment Hide comment
@justinko

justinko Mar 23, 2012

@StevenGerrard

You have great code ... I love it

Thanks bro!

@StevenGerrard

You have great code ... I love it

Thanks bro!

@cameron-martin

This comment has been minimized.

Show comment Hide comment
@cameron-martin

cameron-martin Aug 13, 2014

I can't take a design opinion seriously from the person who built rails.

Plus all the things that other people have pointed out about these examples being no way near equivalent.

I can't take a design opinion seriously from the person who built rails.

Plus all the things that other people have pointed out about these examples being no way near equivalent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment