New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider removing `before(:all)` #573

Closed
dchelimsky opened this Issue Feb 14, 2012 · 34 comments

Comments

Projects
None yet
@dchelimsky
Member

dchelimsky commented Feb 14, 2012

The initial intent of before(:all) was to work like a class-level setup method in xUnit tools to set something up in the environment that you only want to set up once. At some point, we made it a cross-example state-sharing mechanism, and since that point it's been nothing but trouble.

The superficial motivation for using this as a state-sharing mechanism is speed, but this really stems from the one-expectation-per-example guideline, which, in turn, stems from the fact that expectations raise errors rather than recording and reporting later.

We should consider removing before(:all) from rspec-core, and supporting a non-fast-fail mode in rspec-expectations.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Feb 14, 2012

Member

FYI - this was motivated by issue #500, among others.

Member

dchelimsky commented Feb 14, 2012

FYI - this was motivated by issue #500, among others.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Feb 14, 2012

Member

#297 asks that we rename before(:all) to before(:group). I'd be open to leaving before(:group) as a non-state-sharing group-level setup as we remove before(:all).

Member

dchelimsky commented Feb 14, 2012

#297 asks that we rename before(:all) to before(:group). I'd be open to leaving before(:group) as a non-state-sharing group-level setup as we remove before(:all).

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

This is huge and a major +1

Contributor

justinko commented Feb 14, 2012

This is huge and a major +1

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 14, 2012

Member

FWIW, I like before(:all) and use it on occasion. I think it's a "use with care" power user's tool, but it does have its uses.

Member

myronmarston commented Feb 14, 2012

FWIW, I like before(:all) and use it on occasion. I think it's a "use with care" power user's tool, but it does have its uses.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Feb 14, 2012

Member

@myronmarston what do you think about introducing before(:group) as a non-state-sharing variant of before(:all), and then deprecating before(:all) as it stands today?

Member

dchelimsky commented Feb 14, 2012

@myronmarston what do you think about introducing before(:group) as a non-state-sharing variant of before(:all), and then deprecating before(:all) as it stands today?

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 14, 2012

Member

I have on occasion used before(:all) to share state, and those specs wouldn't work with the new before(:group) hook. Here's an example of where I use it:

I've got a library that does a bunch of web scraping and parsing. The specs are largely integrated (using VCR to record/playback the HTTP requests), and in many places I follow this pattern:

describe "Scraping page X" do
  before(:all) do
    @scrape_summary = MyScraper.perform_on("http://foo-bar.com/bazz")
  end

  it "extracts one kind of thing from the page" do
    @scrape_summary.total_results.should eq(25)
  end

  it "extracts something else from the page" do
    @scrape_summary.link_urls.should include("http://some-expected-url.com/")
  end

  # ...several more of these sorts of examples
end

All of the examples in this case are 100% side-effect free expectations set on the results of scraping. Thus, using before(:all) in this manner is safe, and gives the specs a huge speed boost.

You mention the "one expectation per example" guideline, and that a no-fail-fast mode in rspec-expectations could solve this problem...but it doesn't solve it completely for me. I've never followed "one expectation per example"; instead, it's "one behavior per example, and multiple expectations in the same example may be a sign you are specifying multiple behaviors". For some of the specs like the ones above, I already do put multiple expectations in the same example, when they are related to specifying the same behavior. If I was to rewrite these specs to use a new no-fail-fast mode and condense the multiple examples into one, I'd either wind up with a single mammoth it "does A, B, C, D and E" doc string, or a very generic it "works" or it "scrapes what it should" doc string so as not to have huge long one. I don't really like either option. I like having these as separate examples (since they specify different facets of behavior). It's important to me to have specific, descriptive doc strings.

Overall, I think before(:all) can get messy and is often a design smell. When you're writing de-coupled, well-factored code, you should never need to use it. In the case of a library that does web scraping, isolated testing doesn't work so well; you need to integrate with examples of the actual pages you are scraping and parsing.

Member

myronmarston commented Feb 14, 2012

I have on occasion used before(:all) to share state, and those specs wouldn't work with the new before(:group) hook. Here's an example of where I use it:

I've got a library that does a bunch of web scraping and parsing. The specs are largely integrated (using VCR to record/playback the HTTP requests), and in many places I follow this pattern:

describe "Scraping page X" do
  before(:all) do
    @scrape_summary = MyScraper.perform_on("http://foo-bar.com/bazz")
  end

  it "extracts one kind of thing from the page" do
    @scrape_summary.total_results.should eq(25)
  end

  it "extracts something else from the page" do
    @scrape_summary.link_urls.should include("http://some-expected-url.com/")
  end

  # ...several more of these sorts of examples
end

All of the examples in this case are 100% side-effect free expectations set on the results of scraping. Thus, using before(:all) in this manner is safe, and gives the specs a huge speed boost.

You mention the "one expectation per example" guideline, and that a no-fail-fast mode in rspec-expectations could solve this problem...but it doesn't solve it completely for me. I've never followed "one expectation per example"; instead, it's "one behavior per example, and multiple expectations in the same example may be a sign you are specifying multiple behaviors". For some of the specs like the ones above, I already do put multiple expectations in the same example, when they are related to specifying the same behavior. If I was to rewrite these specs to use a new no-fail-fast mode and condense the multiple examples into one, I'd either wind up with a single mammoth it "does A, B, C, D and E" doc string, or a very generic it "works" or it "scrapes what it should" doc string so as not to have huge long one. I don't really like either option. I like having these as separate examples (since they specify different facets of behavior). It's important to me to have specific, descriptive doc strings.

Overall, I think before(:all) can get messy and is often a design smell. When you're writing de-coupled, well-factored code, you should never need to use it. In the case of a library that does web scraping, isolated testing doesn't work so well; you need to integrate with examples of the actual pages you are scraping and parsing.

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

I had a discussion on Twitter & Github gists sometime last year about all of this.

Basically, we use before(:all) because of descriptions. That's why I use them anyway. Here are some ideas to get around this:

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  it 'extracts the correct elements from the page'  do
    detail "extracts one kind of thing from the page" do
      scrape_summary.total_results.should eq(25)
    end

    detail "extracts something else from the page" do
      scrape_summary.link_urls.should include("http://some-expected-url.com/")
    end
  end
end

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  it 'extracts the correct elements from the page (generic description)'  do
    expect('extracts total results', scrape_summary.total_results).to eq(25)
    expect('extracts link urls', scrape_summary.link_urls).to include("http://some-expected-url.com/")
  end
end

Or, we could move the work down to the "example" level:

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  # Each example group runs all the zits (obviously not the final method name) at once.

  zit "extracts one kind of thing from the page" do
    scrape_summary.total_results.should eq(25)
  end

  zit "extracts something else from the page" do
    scrape_summary.link_urls.should include("http://some-expected-url.com/")
  end
end

Like any of these ideas?

Contributor

justinko commented Feb 14, 2012

I had a discussion on Twitter & Github gists sometime last year about all of this.

Basically, we use before(:all) because of descriptions. That's why I use them anyway. Here are some ideas to get around this:

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  it 'extracts the correct elements from the page'  do
    detail "extracts one kind of thing from the page" do
      scrape_summary.total_results.should eq(25)
    end

    detail "extracts something else from the page" do
      scrape_summary.link_urls.should include("http://some-expected-url.com/")
    end
  end
end

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  it 'extracts the correct elements from the page (generic description)'  do
    expect('extracts total results', scrape_summary.total_results).to eq(25)
    expect('extracts link urls', scrape_summary.link_urls).to include("http://some-expected-url.com/")
  end
end

Or, we could move the work down to the "example" level:

describe "Scraping page X" do
  let(:scrape_summary) { MyScraper.perform_on("http://foo-bar.com/bazz") }

  # Each example group runs all the zits (obviously not the final method name) at once.

  zit "extracts one kind of thing from the page" do
    scrape_summary.total_results.should eq(25)
  end

  zit "extracts something else from the page" do
    scrape_summary.link_urls.should include("http://some-expected-url.com/")
  end
end

Like any of these ideas?

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 14, 2012

Member

@justinko -- of those options, the first one (the detail one is the one I like best), but I don't think I like the idea of introducing a new construct for this. It's not a very common case, and adding more to the DSL adds additional cognitive load.

@dchelimsky -- is the main reason you want to remove before(:all) because of all the gotchas surrounding the shared state it allows? I think we're all agreed that the shared state feature of before(:all) is usually a bad idea. As I explained above, in a few very particular circumstances I find it useful. Given that...what if we made it a "disabled by default" feature that users must opt in to use? Here's what I mean:

With this change, instance variables initialized in before(:all) would be ignored by RSpec, and would thus not available in examples. However, if you really need this feature, you could do:

describe "Scraping page X" do
  before(:all, :share_state) do
    @scrape_summary = MyScraper.perform_on("http://foo-bar.com/bazz")
  end
end

Or it could just be a config option in the RSpec.configure block--but I kinda like it being a per-before(:all) hook thing.

Member

myronmarston commented Feb 14, 2012

@justinko -- of those options, the first one (the detail one is the one I like best), but I don't think I like the idea of introducing a new construct for this. It's not a very common case, and adding more to the DSL adds additional cognitive load.

@dchelimsky -- is the main reason you want to remove before(:all) because of all the gotchas surrounding the shared state it allows? I think we're all agreed that the shared state feature of before(:all) is usually a bad idea. As I explained above, in a few very particular circumstances I find it useful. Given that...what if we made it a "disabled by default" feature that users must opt in to use? Here's what I mean:

With this change, instance variables initialized in before(:all) would be ignored by RSpec, and would thus not available in examples. However, if you really need this feature, you could do:

describe "Scraping page X" do
  before(:all, :share_state) do
    @scrape_summary = MyScraper.perform_on("http://foo-bar.com/bazz")
  end
end

Or it could just be a config option in the RSpec.configure block--but I kinda like it being a per-before(:all) hook thing.

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

@myronmarston the discussion here is to entirely remove before(:all) from RSpec. :share_state doesn't change anything -- especially code wise.

Contributor

justinko commented Feb 14, 2012

@myronmarston the discussion here is to entirely remove before(:all) from RSpec. :share_state doesn't change anything -- especially code wise.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 14, 2012

Member

@myronmarston the discussion here is to entirely remove before(:all) from RSpec.

My understanding is that the discussion is to remove before(:all) in favor of before(:group) that acts the same, except for the fact that it doesn't share state. We could certainly add before(:group) as an alias (or similar-behaving method) but I'd like to see a way to preserve the existing before(:all) behavior, but make it opt-in, rather than turned on by default. Whether that's keeping before(:all) itself or migrating to before(:group) doesn't matter as much to me.

:share_state doesn't change anything -- especially code wise.

:share_state would preserve the existing behavior. The real change I'm suggesting is to make state not shared by default. This forces people to be intentional about "yes, I really do want to share state across all these examples", rather than hacking together some specs that use before(:all) and happen to work but will later cause trouble.

Member

myronmarston commented Feb 14, 2012

@myronmarston the discussion here is to entirely remove before(:all) from RSpec.

My understanding is that the discussion is to remove before(:all) in favor of before(:group) that acts the same, except for the fact that it doesn't share state. We could certainly add before(:group) as an alias (or similar-behaving method) but I'd like to see a way to preserve the existing before(:all) behavior, but make it opt-in, rather than turned on by default. Whether that's keeping before(:all) itself or migrating to before(:group) doesn't matter as much to me.

:share_state doesn't change anything -- especially code wise.

:share_state would preserve the existing behavior. The real change I'm suggesting is to make state not shared by default. This forces people to be intentional about "yes, I really do want to share state across all these examples", rather than hacking together some specs that use before(:all) and happen to work but will later cause trouble.

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

I'm tempted to write a gem that allows you to have "shared state" and remove before(:all) from RSpec:

describe do
  let(:foo) { shared { do_something } }
  # or
  shared(:foo) { do_something }

  it do
    foo # computed first
  end

  it do
    foo # memoized value returned
  end
end

The shared method would handle memorization and "clean up". It could store the shared values in the describe (ExampleGroup) instance. Or, it could use global variables namespaced by "rspec".

Thoughts?

Contributor

justinko commented Feb 14, 2012

I'm tempted to write a gem that allows you to have "shared state" and remove before(:all) from RSpec:

describe do
  let(:foo) { shared { do_something } }
  # or
  shared(:foo) { do_something }

  it do
    foo # computed first
  end

  it do
    foo # memoized value returned
  end
end

The shared method would handle memorization and "clean up". It could store the shared values in the describe (ExampleGroup) instance. Or, it could use global variables namespaced by "rspec".

Thoughts?

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

The real change I'm suggesting is to make state not shared by default.

Doesn't matter if it's on or off, we would still have to support the case when it is on. This would only add more code: "if before all shared state is on...".

Contributor

justinko commented Feb 14, 2012

The real change I'm suggesting is to make state not shared by default.

Doesn't matter if it's on or off, we would still have to support the case when it is on. This would only add more code: "if before all shared state is on...".

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 14, 2012

Member

An external gem that provides this behavior may be the way to go. A single shared DSL method that mirrors let has much less cognitive load than the other APIs you proposed above. I also like that putting it in an external gem means it's clear that it's not the "normal" way to write specs.

Member

myronmarston commented Feb 14, 2012

An external gem that provides this behavior may be the way to go. A single shared DSL method that mirrors let has much less cognitive load than the other APIs you proposed above. I also like that putting it in an external gem means it's clear that it's not the "normal" way to write specs.

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Feb 14, 2012

Contributor

I would store the "shared" values in Thread.current, btw.

Contributor

justinko commented Feb 14, 2012

I would store the "shared" values in Thread.current, btw.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Feb 16, 2012

Member

is the main reason you want to remove before(:all) because of all the gotchas surrounding the shared state it allows?

@myronmarston it's more specifically the confusing nature of the relationship between shared state and features that are intended to work per-example, like let and subject. I think shared state is manageable if it's visible (i.e. instance variables in before(:all)).

Member

dchelimsky commented Feb 16, 2012

is the main reason you want to remove before(:all) because of all the gotchas surrounding the shared state it allows?

@myronmarston it's more specifically the confusing nature of the relationship between shared state and features that are intended to work per-example, like let and subject. I think shared state is manageable if it's visible (i.e. instance variables in before(:all)).

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 16, 2012

Member

@dchelimsky -- that makes sense. Earlier you said:

I'd be open to leaving before(:group) as a non-state-sharing group-level setup as we remove before(:all).

How would let and subject behave when called from within a before(:group) hook?

Member

myronmarston commented Feb 16, 2012

@dchelimsky -- that makes sense. Earlier you said:

I'd be open to leaving before(:group) as a non-state-sharing group-level setup as we remove before(:all).

How would let and subject behave when called from within a before(:group) hook?

@coffeeaddict

This comment has been minimized.

Show comment
Hide comment
@coffeeaddict

coffeeaddict Mar 12, 2012

I use before(:all) quite often to perform setup (other then just of the subject) - I do agree that :each and :all feel a bit clumsy - :group on the other hand also feels a tad clumsy. What about before(:context)? Given the fact that there is know verb called context in the DSL, this makes most sense imho

I use before(:all) quite often to perform setup (other then just of the subject) - I do agree that :each and :all feel a bit clumsy - :group on the other hand also feels a tad clumsy. What about before(:context)? Given the fact that there is know verb called context in the DSL, this makes most sense imho

@justinko

This comment has been minimized.

Show comment
Hide comment
@justinko

justinko Mar 13, 2012

Contributor

Okay here's the shared method:
https://github.com/justinko/rspec-shared

So looks like the roadmap is this:

  • Change before(:all) to before(:group) and remove shared instance variables.
  • Change/alias before(:each) to before(:example)
Contributor

justinko commented Mar 13, 2012

Okay here's the shared method:
https://github.com/justinko/rspec-shared

So looks like the roadmap is this:

  • Change before(:all) to before(:group) and remove shared instance variables.
  • Change/alias before(:each) to before(:example)
@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky Mar 13, 2012

Member

I think we should leave before(:all) as/is through the 3.0 series, deprecating it in the last 3.x before 4.0. That gives users a year or so to gradually change before(:all) to before(:group).

Member

dchelimsky commented Mar 13, 2012

I think we should leave before(:all) as/is through the 3.0 series, deprecating it in the last 3.x before 4.0. That gives users a year or so to gradually change before(:all) to before(:group).

@jarmo

This comment has been minimized.

Show comment
Hide comment
@jarmo

jarmo Apr 23, 2012

Contributor

I'm using before(:all) in my acceptance tests to open up the browser and store it's value in an instance variable. If before(:all) would be removed, how would i solve that use-case instead?

Contributor

jarmo commented Apr 23, 2012

I'm using before(:all) in my acceptance tests to open up the browser and store it's value in an instance variable. If before(:all) would be removed, how would i solve that use-case instead?

@d11wtq

This comment has been minimized.

Show comment
Hide comment
@d11wtq

d11wtq May 26, 2012

I'm using before/after all to start and stop a system process in a test rig. Perhaps there's another approach to this? Doing this before/after each would be too slow. The daemon in question is sphinx. I basically write out a "test" sphinx.conf to a temp dir, start the process in a before(:all), run all the examples, emptying the indexes in a before(:each) (this is quite fast), then stop sphinx in a after(:all) and clean up the temporary data directory. I'm not sure how I'd do this without such hooks.

d11wtq commented May 26, 2012

I'm using before/after all to start and stop a system process in a test rig. Perhaps there's another approach to this? Doing this before/after each would be too slow. The daemon in question is sphinx. I basically write out a "test" sphinx.conf to a temp dir, start the process in a before(:all), run all the examples, emptying the indexes in a before(:each) (this is quite fast), then stop sphinx in a after(:all) and clean up the temporary data directory. I'm not sure how I'd do this without such hooks.

@dchelimsky

This comment has been minimized.

Show comment
Hide comment
@dchelimsky

dchelimsky May 27, 2012

Member

@d11wtq @jarmo the "instance state sharing" provided by before(:all) is a bit misleading because we're actually just copying instance variables around. A more representative solution would be one that forces the user to manage state sharing through global variables or constants. if/when we do this we'll still have a place to put code that should only run once (either before(:group) as originally planned or before(:context) as suggested earlier this thread), but it will be up to you to manage the state e.g.

before(:group) do
  $browser = BrowserSimulator.new
  $browser.start
end

after(:group) do
  $browser.stop
end

This would have the additional advantage of clarifying what is per-example state (using @) and what is per-group state (using $) e.g.

before(:group) do
  $browser = BrowserSimulator.new
  $browser.start
end

before(:example) do
  @page = Page.load(some_path)
end
Member

dchelimsky commented May 27, 2012

@d11wtq @jarmo the "instance state sharing" provided by before(:all) is a bit misleading because we're actually just copying instance variables around. A more representative solution would be one that forces the user to manage state sharing through global variables or constants. if/when we do this we'll still have a place to put code that should only run once (either before(:group) as originally planned or before(:context) as suggested earlier this thread), but it will be up to you to manage the state e.g.

before(:group) do
  $browser = BrowserSimulator.new
  $browser.start
end

after(:group) do
  $browser.stop
end

This would have the additional advantage of clarifying what is per-example state (using @) and what is per-group state (using $) e.g.

before(:group) do
  $browser = BrowserSimulator.new
  $browser.start
end

before(:example) do
  @page = Page.load(some_path)
end
@wulftone

This comment has been minimized.

Show comment
Hide comment
@wulftone

wulftone Aug 16, 2012

This gem is related to the issue: https://github.com/LRDesign/rspec-steps

I use that gem and the :all feature very often. (It would be great to see the steps feature integrated into rspec) Isolating specs is all fine and good, but frequently I just know what I'm doing and want things to run as fast as possible. I'd hate to see this spec group setup thing disappear, however I do like @justinko's detail thing.

He's right, we really just want the group behavior for the descriptions in the spec output, but we're trained to shun having multiple .should's in a single it block, though it's a really natural way to test things (but you don't get a description for each should in a single it block!)

This gem is related to the issue: https://github.com/LRDesign/rspec-steps

I use that gem and the :all feature very often. (It would be great to see the steps feature integrated into rspec) Isolating specs is all fine and good, but frequently I just know what I'm doing and want things to run as fast as possible. I'd hate to see this spec group setup thing disappear, however I do like @justinko's detail thing.

He's right, we really just want the group behavior for the descriptions in the spec output, but we're trained to shun having multiple .should's in a single it block, though it's a really natural way to test things (but you don't get a description for each should in a single it block!)

@patmaddox

This comment has been minimized.

Show comment
Hide comment
@patmaddox

patmaddox Sep 11, 2012

Contributor

+1000 !! death to before(:all) - and any_instance too while we're at it :)

Contributor

patmaddox commented Sep 11, 2012

+1000 !! death to before(:all) - and any_instance too while we're at it :)

@likethesky

This comment has been minimized.

Show comment
Hide comment
@likethesky

likethesky Nov 14, 2012

Has any thought been made to throwing an error if a before(:all) has been used without a corresponding after(:all) ... after all? ;-) Perhaps this would be one way of leaving this feature for the minority of cases where it's really needed, but this error would at least alert the unaware that "something extra" must be thought about before blindly proceeding. IOW, you'd have to have at least an empty after(:all) do end any time you want to use a before(:all) do # something; end ? I also like the gem approach, but that seems more tedious and not as pleasurable, to me.

I also like the before(:group) and before(:example) naming, with deprecation warnings issued for before(:each) perhaps, or just letting it continue to work as-is... Also, what's the thought on before do end (that is, without any scope identifier), will that stay as-is? I note that the docs don't discuss this variant though it's what I see day-in and day-out... Happy to open an issue to get the docs updated, if that's just an oversight.

Has any thought been made to throwing an error if a before(:all) has been used without a corresponding after(:all) ... after all? ;-) Perhaps this would be one way of leaving this feature for the minority of cases where it's really needed, but this error would at least alert the unaware that "something extra" must be thought about before blindly proceeding. IOW, you'd have to have at least an empty after(:all) do end any time you want to use a before(:all) do # something; end ? I also like the gem approach, but that seems more tedious and not as pleasurable, to me.

I also like the before(:group) and before(:example) naming, with deprecation warnings issued for before(:each) perhaps, or just letting it continue to work as-is... Also, what's the thought on before do end (that is, without any scope identifier), will that stay as-is? I note that the docs don't discuss this variant though it's what I see day-in and day-out... Happy to open an issue to get the docs updated, if that's just an oversight.

@EarthCitizen

This comment has been minimized.

Show comment
Hide comment
@EarthCitizen

EarthCitizen Feb 15, 2013

I use before(:all) when testing shell scripts and database scripts:

describe "My shell script" do
    context "When I run my script" do
        before(:all) do
              ---code here to run the script---
        end
        it { accomplishes task1 }
        it { accomplishes task2 }
        it { accomplishes task3 }
        it { accomplishes task4 }
    end
end

In this case, I am not checking the output of the run. I am checking to see that it performed it's required objectives. How else could I check multiple objects for one run without a before(:all)?

I use before(:all) when testing shell scripts and database scripts:

describe "My shell script" do
    context "When I run my script" do
        before(:all) do
              ---code here to run the script---
        end
        it { accomplishes task1 }
        it { accomplishes task2 }
        it { accomplishes task3 }
        it { accomplishes task4 }
    end
end

In this case, I am not checking the output of the run. I am checking to see that it performed it's required objectives. How else could I check multiple objects for one run without a before(:all)?

@soulcutter

This comment has been minimized.

Show comment
Hide comment
@soulcutter

soulcutter Feb 15, 2013

Member

@EarthCitizen With multiple assertions inside a single 'it' block, or just using multiple runs to make separate assertions.

Member

soulcutter commented Feb 15, 2013

@EarthCitizen With multiple assertions inside a single 'it' block, or just using multiple runs to make separate assertions.

@EarthCitizen

This comment has been minimized.

Show comment
Hide comment
@EarthCitizen

EarthCitizen Feb 15, 2013

@soulcutter From what I understand, multiple assertions inside a single example is bad practice. Running the scripts multiple times can be unnecessarily inefficient because often the scripts can have a long run time. So running them before each example would create a huge overhead.

@soulcutter From what I understand, multiple assertions inside a single example is bad practice. Running the scripts multiple times can be unnecessarily inefficient because often the scripts can have a long run time. So running them before each example would create a huge overhead.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 15, 2013

Member

From what I understand, multiple assertions inside a single example is bad practice.

There's a time and place for "one assertion per example", but this is definitely not a bad practice. My thoughts on the manner:

lelylan/betterspecs#5 (comment)

Member

myronmarston commented Feb 15, 2013

From what I understand, multiple assertions inside a single example is bad practice.

There's a time and place for "one assertion per example", but this is definitely not a bad practice. My thoughts on the manner:

lelylan/betterspecs#5 (comment)

@EarthCitizen

This comment has been minimized.

Show comment
Hide comment
@EarthCitizen

EarthCitizen Feb 15, 2013

@myronmarston Thanks for the input. I see your points. But if we have to pack multiple assertions into one example for performance reasons, would it not just be better to use before(:all) ?

@myronmarston Thanks for the input. I see your points. But if we have to pack multiple assertions into one example for performance reasons, would it not just be better to use before(:all) ?

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Feb 15, 2013

Member

But if we have to pack multiple assertions into one example for performance reasons, would it not just be better to use before(:all) ?

I'm not necessarily convinced that one is better then the other, but lots of tools hook into before(:each) to ensure clean slate between each test. before(:all) can't easily work with such tools -- so I tend to favor before(:each) for such things. With before(:all) you have to be extra careful to manage your state well so you don't wind up with "leaked" state that affects other specs.

But in the case here, it sounds like you've had success using before(:all) and I think that's fine.

Member

myronmarston commented Feb 15, 2013

But if we have to pack multiple assertions into one example for performance reasons, would it not just be better to use before(:all) ?

I'm not necessarily convinced that one is better then the other, but lots of tools hook into before(:each) to ensure clean slate between each test. before(:all) can't easily work with such tools -- so I tend to favor before(:each) for such things. With before(:all) you have to be extra careful to manage your state well so you don't wind up with "leaked" state that affects other specs.

But in the case here, it sounds like you've had success using before(:all) and I think that's fine.

@EarthCitizen

This comment has been minimized.

Show comment
Hide comment
@EarthCitizen

EarthCitizen Feb 15, 2013

@myronmarston Yeah, I agree with you that it should be used with care. It should be for exceptional cases and not as a standard strategy.

@myronmarston Yeah, I agree with you that it should be used with care. It should be for exceptional cases and not as a standard strategy.

@myronmarston

This comment has been minimized.

Show comment
Hide comment
@myronmarston

myronmarston Mar 1, 2013

Member

@dchelimsky and I discussed this some more, and we've decided to keep before(:all). Misunderstandings about it have definitely been a source of issues, and there's something appealing about the simplicity it would bring to remove it, but it's been around for a long time and is still occasionally useful.

Member

myronmarston commented Mar 1, 2013

@dchelimsky and I discussed this some more, and we've decided to keep before(:all). Misunderstandings about it have definitely been a source of issues, and there's something appealing about the simplicity it would bring to remove it, but it's been around for a long time and is still occasionally useful.

@baash05

This comment has been minimized.

Show comment
Hide comment
@baash05

baash05 Jul 3, 2014

Thank you so much for keeping it.
changing the language to something so close just because some people abuse it frustrates migration. It would mean I couldn't ever upgrade my rspec. Rails did it with attribute_accessible and now at all the intro courses I have to explain why 90% of the tutorials on line don't work, and why you can't migrate your code from rails 3 to 4.

Thank you so much!

baash05 commented Jul 3, 2014

Thank you so much for keeping it.
changing the language to something so close just because some people abuse it frustrates migration. It would mean I couldn't ever upgrade my rspec. Rails did it with attribute_accessible and now at all the intro courses I have to explain why 90% of the tutorials on line don't work, and why you can't migrate your code from rails 3 to 4.

Thank you so much!

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