On Sun, May 16, 2021 at 1:55 AM Janko Marohnić <[email protected]> wrote:
> 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. > Technically, you don't know the same connection will available. There could be a disconnect error the closes the connection, with the rest of the code executing outside of a transaction on a new connection. It also can result in pool timeouts in certain cases depending on access patterns. > DB.transaction(rollback: :always, auto_savepoint: true) do > DB.pool.send(:release, Thread.current) > spec.run > DB.pool.send(:acquire, Thread.current) > end > This does look safer. I would also override Dataset#connect after the connection has been established to raise an error, to handle cases where there is a disconnect error. 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. > While it may not be possible to support all possible cases, it may be possible to support the most common cases. > 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. > I guess it depends on what the access patterns are. Unless there is some sort of asynchronous behavior, I would assume usage such as: it "some spec" do interact_with_database run_request interact_with_database run_request interact_with_database run_request interact_with_database end Would be fine with the single threaded pool. However, this assumes that run_request doesn't return until all database access has completed on the server. Maybe there are cases where that isn't true. 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 don't think the main advantage is performance, I think the main advantage is ease of use. With the truncation/deletion method, you need to explicitly undo any changes you are making during the test (figuring out which tables are being modified), instead of having that implicitly handled by the transaction. There may be performance advantages, especially with a large number of tables, or with significantly latency to the database. Just to be clear, I'm not saying what you are doing in regards to connection sharing will not work. I just am hesitant to accept it as part of Sequel's documentation until I can prove to myself that it is safe. Have you used this approach in a decently sized application with positive results? Personally, I avoid this issue as much as possible by avoiding javascript as much as possible, and using transactions with rack-test for the vast majority of my web testing. For the tests that require javascript, I use non-transactional tests. However, I realize that not all developers have that luxury. 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/CADGZSSf1XC427SQ2j3iOTYAX5d53tWwhiKOOTffG71U6wf%3DnUA%40mail.gmail.com.
