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

Already on GitHub? Sign in to your account

Support for transactional/truncation in before(:all) blocks. #496

Closed
stevenharman opened this Issue Jan 24, 2012 · 14 comments

Comments

Projects
None yet
7 participants

As an example, see rspec-set which adds a set(:foo) { ... } syntax that works much like let(:foo) { ... } except within a before(:all) block. This leads to much faster model specs as you can create a particular state once and then have a number of it blocks making independent assertions. In particular, I've found this very useful when triangulating a named scope, etc.

As rspec-set exists today, it is left as an exercise to the consumer to clean up any records created via the set block, but this doesn't have to be so. For example, @xaviershay has done this for Datamaper. datamapper-set

I also realize this can quickly get complicated once nested contexts are rolled into the mix, and that different databases support various flavors of nested transactions. I've addressed the later issue with DatabaseCleaner.strategey = :truncation. The first issue may require more thought, or perhaps is just not worth supporting?

Owner

dchelimsky commented Apr 22, 2012

@stevenharman a lot of folks have asked for this over the years, but you are correct that this can get very complicated. In addition to the nesting issues and differences across RDBMSs you mention, there would also be the general confusion over state persisting across examples (i.e. what happens to a model you create in before(:all) and destroy in an example?). Not to mention the choice between :truncation and :transaction.

This is something that could be supported in a separate gem. Any interest in developing one?

there would also be the general confusion over state persisting across examples
(i.e. what happens to a model you create in before(:all) and destroy in an example?

IMO, it is only reasonable to have shared state when the it blocks all make side-effect free assertions. I prefer the term "observations" as it more accurately describes the AAA flow of BDD.

  1. Arrange the state.
  2. Act on the state (possibly mutating it).
  3. Assert on the resulting state of the system (this is side-effect free).

With that constraint, would it be safe to move the transaction scope up to the before(:all) block? The user could opt into this behavior by passing certain metadata into the describe/context block.

This of course does not handle nested transactions, but maybe those should not be supported. Maybe opting into this behavior in a nested context could throw a meaningful exception.

To be clear, I'm not necessarily suggesting this be added to Rspec proper - a new gem may still be the right answer. I am curious if you agree with this line of thinking, and if a solution like this would/could work.

Thoughts?

/cc @alindeman

To be clear about what I said re: no nested contexts:

opting into this behavior in a nested context could throw a meaningful exception.

I mean that if you already have a describe/context which has opted into the higher-level transaction, you can do this:

describe FooBar do
  describe '.some_scope', transaction_scope: true do
    before(:all) do
      # set up some DB state to be shared across ALL child contexts and tests
      # act on the state.
    end

    it 'makes a side-effect free observation' do
      #...
    end

    it 'makes some other side-effect free observation' do
      #...
    end
  end
end

But you CANNOT do this:

describe FooBar do
  describe '.some_scope', transaction_scope: true do
    before(:all) do
      # set up some DB state to be shared across ALL child contexts and tests
      # act on the state.
    end

    it 'makes a side-effect free observation' do
      #...
    end

    # This will fail with a meaningful error
    context 'set more state for just these tests', transaction_scope: true do
      before(:all) do
        # add more state
      end

      it 'makes some other side-effect free observation' do
        #...
      end
    end
  end
end
Owner

dchelimsky commented Apr 23, 2012

Actually, I'd never though of setting the transaction_scope explicitly like that. That makes me much more comfortable with this whole idea, because you can see it in the code (rather than having RSpec try to figure out what you mean). Rather than true/false, however, how about:

describe Something, :transaction_scope => :all do
  # ...
end

The default :transaction_scope would be :each, this declaration would override it. In fact, I'd even be OK with this:

describe Thing, transaction_scope => :all do
  before(:all) { @thing = Thing.new }
  it "does something" do
    # ...
  end

  describe "in some other context", :transaction_scope => :all do
    it "does something else" do
    end
  end

  describe "in yet another context", :transaction_scope => :each do
    it "does something even elser" do
      # ...
    end
  end
end

If this falls apart because the RDBMS doesn't support it, it would be pretty clear, and since you're having to declare these things explicitly, you probably wouldn't do it unless you knew what you were doing.

Thoughts?

Ohh, I much prefer the :transaction_scope => :all API. And yes, the explicitness of this approach makes it much more palatable, and hopefully less surprising, to folks opting into it.

So, is this best done as an extension to rspec-rails, or could it be rolled in?

I'd love this, though let and subject would need to work the same way for it to make sense, and that would require a patch to rspec-core I believe. This whole thing may be better suited as a gem.

Also, a way to make this the default behavior would be great.

Regarding nested contexts, the best way to handle it is to treat each nested context as its own context entirely, so state, transactions, etc, would reset in between. Unfortunately, this is not how before :all works, so you would have to stick to not allowing it in a nested fashion unless you want to completely change the way before :all works in rspec (another thing I'd love, and another argument for a gem)

Contributor

rosenfeld commented Apr 24, 2012

@dchelimsky I liked this proposed API and find it very handy. It would be awesome to have such kind of support in RSpec. But how would we use this feature if we're using Sequel instead of ActiveRecord for instance?

Owner

dchelimsky commented Apr 24, 2012

@rosenfeld the idea here is for rspec to delegate to DatabaseCleaner to start and clean in different scopes. Support for specific libs and/or RDBMS's would be up to DatabaseCleaner, not rspec, so if DatabaseCleaner doesn't support it, it's not supported.

Contributor

rosenfeld commented Apr 24, 2012

@dchelimsky DatabaseCleaner is said to support Sequel, but I don't think it supports the notion of nested transactions.

In Sequel you can get nested transactions by using this way:

DB.transaction(savepoint: true) do
  ...
  DB.transaction(savepoint: true) do
      ...
      raise Sequel::Rollback
  end
  raise Sequel::Rollback
end

But the problem is that it does only support transactions in blocks as far as I can tell you.

Since RSpec doesn't support an around(:all) filter, I don't know how this could be implemented in Sequel...

@dchelimsky dchelimsky referenced this issue in rspec/rspec-core Aug 31, 2012

Closed

Transactions in before(:all) Blocks #669

Owner

myronmarston commented Sep 3, 2012

Since RSpec doesn't support an around(:all) filter, I don't know how this could be implemented in Sequel...

FWIW, an around(:all) hook can be trivially implemented on top of the existing before(:all)/after(:all) hooks using fibers. I wrote a blog post about it and released a microgem that you can drop in your project and use.

Contributor

alindeman commented May 22, 2013

I'm going to close this for now due to lack of activity and because there is a pretty reasonable alternative.

If someone wants to work something up, I'm willing to look at it, but I myself am not superstoked about implementing it ... so I feel this issue will just stagnate as open unless I close it.

Therefore, I'm closing it :)

@alindeman alindeman closed this May 22, 2013

Ok, thanks

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