>From my understanding, releasing the connection back into the pool after starting the outer transaction allows the test & app threads to use the same connection in a thread-safe way, because additional calls to Database#synchronize will correctly lock the connection for the duration of queries. I don't see any risk here, because we know the same connection will be available when the test transaction should be rolled back. We should probably acquire the connection back, though, to ensure the app thread isn't using it at the moment of rollback.
DB.transaction(rollback: :always, auto_savepoint: true) do DB.pool.send(:release, Thread.current) spec.run DB.pool.send(:acquire, Thread.current) end There is still risk that the app thread would continue executing queries in a request that still hasn't finished before the end of the test, which could cause data to get written outside of a transaction, but there is probably ways to work around that as well. I would generally like to explore whether it's possible to make browser tests work with transactional tests in a safe way, because that's the level of convenience that Rails system tests provide. A single-threaded pool wouldn't work for me in this case, because of the lack of locking around connection access, which would almost certainly result in race conditions. I've experienced <https://github.com/janko/sequel-activerecord_connection/issues/12> these when working on the sequel-activerecord_connection gem, and I don't know how I would avoid them with a single-threaded pool. Yes, my current code is using private API, but it might be worth the trade-off to gain better performance. Though I'm still yet to verify whether the truncation approach really does impact test suite performance significantly. If it's not, then I don't understand why did Active Record maintainers go to such great lengths <https://github.com/rails/rails/blob/61fb58f6a7f852d765d616d1da41d17851ec7afc/activerecord/lib/active_record/test_fixtures.rb> to make transactional tests work with Capybara browser tests. I appreciate the discussion, it's helping me a lot in organizing my thoughts :) Kind regards, Janko On Saturday, May 15, 2021 at 8:22:34 PM UTC+2 Jeremy Evans wrote: > On Sat, May 15, 2021 at 10:48 AM Janko Marohnić <[email protected]> > wrote: > >> Hello, >> >> I'm wondering what's the recommended Sequel test setup when using >> Capybara with a JS driver that runs the Rack app server in a background >> thread. >> >> Ideally, I would like tests to run inside a transaction, in which case >> the DB connection would have to be shared between the test thread and the >> background app. This is what I came up with: >> >> DB = Sequel.connect("...", max_connections: 1) >> >> RSpec.configure do |c| >> c.around(:each) do |spec| >> DB.transaction(rollback: :always, auto_savepoint: true) do >> DB.pool.send(:release, Thread.current) # allow the app to use the >> connection >> spec.run >> end >> end >> end >> >> Does that look ok? If yes, do you think we could add it to the testing >> guide? >> > > Releasing a connection from the pool while still using the connection for > the transaction is not acceptable. There is a reason #release is private, > it should not be called directly. > > Instead of max_connections: 1, couldn't you use single_threaded: true, so > the single threaded pool is used and you don't have to worry about > connection checkouts? Note that that only fixes the API issues, it doesn't > fix the general issue of multiple threads accessing a connection > simultaneously. > > >> The alternative would be not having a transaction, but rather >> truncating/deleting data after each test run using something like >> DatabaseCleaner, but I guess that would have a too significant performance >> impact (I don't have a large enough app to verify that, though). >> > > That's the approach I would recommend for tests that require multiple > threads. > > Thanks, > Jeremy > -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/sequel-talk/d0e8963d-ff48-45ad-8cbb-2f0f056060een%40googlegroups.com.
