Transactional tests with Capybara and a real browser

[janko]

This is a follow-up on a previous google group thread, where I wanted to setup browser tests when Capybara runs the web app in a background Puma thread. I've used the following RSpec setup in a decently-sized application for more than half a year, and it worked without problems:

# in Sequel initializer:
DB = Sequel.connect("...", max_connections: ENV["RACK_ENV"] == "test" ? 1 : 50)

# in RSpec setup:
RSpec.configure do |config|
  config.around(:each) do |example|
    DB.transaction(rollback: :always, auto_savepoint: true) do
      DB.pool.send(:release, Sequel.current) # release connection for main thread/fiber
      example.run
    ensure
      DB.pool.send(:acquire, Sequel.current) # acquire back connection for main thread/fiber
    end
  end
end

I only needed to make sure it wraps Capybara hooks, to ensure any in-progress requests complete before the transaction rolls back. For me it would be worth adding to the documentation, and potentially making it easier by not needing to call private API. But I leave that up to you, I wanted to at least post this for other people to find 🙂

[jeremyevans]

I'm glad you found a way to make this work and it is working well for your app. However, I'm not comfortable adding documentation for an approach that uses private APIs and that I consider unsafe in the general case (even if works fine in your app).

The issue you are currently having is because in a simplified form,transaction with the options given does more or less:

  def transaction
    synchronize do
      add_transaction
      begin_transaction
      yield
    ensure
      rollback_transaction
      remove_transaction
    end
  end

While the behavior you want is:

  def transaction
    synchronize do
      add_transaction
      begin_transaction
    end
    yield
  ensure
    synchronize do
      rollback_transaction
      remove_transaction
    end
  end

Maybe you can consider adding an external Database extension that supported an approach like that. I would recommend a different method name, and accept no transaction options (using your expected transaction options internally):

  DB.shared_rolled_back_transaction{}

This extension would use the approach I indicated above. When it checks out the connection to do the rollback, it would make sure it is the same connection it started with (raising an error otherwise), and then rollback the transaction. The method would raise before doing anything if the pool was not limited to a single connection.

I realize that's more code that what you are currently using. So I would understand if you didn't want to do that.

[janko]

Thank you for the guidance, I agree with your suggestion 👍🏻 I came up with the following, which seems to work:

# lib/sequel/extensions/test.rb
module Sequel
  module Test
    def test_transaction
      fail "connection pool must have :max_connections equal to 1" unless opts[:max_connections] == 1

      transaction_opts = { rollback: :always, auto_savepoint: true }
      original_conn = nil

      begin
        synchronize do |conn|
          original_conn = conn
          add_transaction(conn, transaction_opts)
          begin_transaction(conn, transaction_opts)
        end
        yield
      ensure
        synchronize do |conn|
          fail "connection acquired for rollback is different than connection acquired for starting the transaction" if original_conn != conn
          rollback_transaction(conn, transaction_opts)
          remove_transaction(conn, transaction_opts)
        end
      end
    end
  end

  Database.register_extension(:test, Test)
end

DB = Sequel.sqlite(max_connections: 1)
DB.extension :test
DB.test_transaction { ... }

I like the more abstract "test" naming, to communicate that this functionality is intended for tests. The logic probably doesn't handle all edge cases – the Sequel::Database#_transaction method is much more careful in handling exceptions – but I think it's a good start. I might publish it as an external extension then.

[jeremyevans]

Looks good. One possible improvement is not running the synchronize in the ensure unless original_conn is set. Another is using a frozen constant for the transaction_opts. But otherwise, it looks mostly good. You probably also want:

begin
  rollback_transaction
ensure
  remove_transaction
end

One likely issue is disconnect handling. If there is a disconnect, I expect that a new connection is created outside of a transaction, and the changes made by that transaction will not be rolled back. That was also an issue with your original approach, I think. Maybe in testing, that doesn't matter. However, if you want to prevent that, an alternative approach would be to have new connections automatically setup transactions (override Database#connect or Database#new_connection to also do add_transaction and begin_transaction). You could override Database#disconnect_connection to try rollback_transaction and remove_connection, and call Database#disconnect in an at_exit block. You could handle that via a Database extension, and either disconnect when the extension is loaded, or have it do the add_transaction and begin_transaction for the available connection.