CODEWITHSUNDEEP
ruby-on-railsrubyruby-on-rails-5
Luiz E.
Luiz E.

Reputation: 7249

how to test lock mechanism

I have a piece of code where I import a BankAccountTransaction to a BankAccount

 bank_account.with_lock do
   transactions.each do |transaction|
     import(bank_account, transaction)
   end
 end

it works fine but I need to write an RSpec case to it so I can be 100% sure I'm not importing transactions twice.

I wrote the following helper

module ConcurrencyHelper
  def make_concurrent_calls(function, concurrent_calls: 2)
    threads = Array.new(concurrent_calls) do
      thread = Thread.new { function.call }
      thread.abort_on_exception = true

      thread
    end

    threads.each(&:join)
  end
end

and I'm calling it on RSpec

context 'when importing the same transaction twice' do
  subject(:concurrent_calls) { make_concurrent_calls(operation) }

  let!(:operation) { -> { described_class.call(params) } }
  let(:filename) { 'single-transaction-response.xml' }

  it 'creates only one transaction' do
    expect { concurrent_calls }.to change(BankaccountTransaction, :count).by(1)
  end
end

but nothing happens, the test suit gets stuck at this point and no errors are thrown or anything like that.

I've put a debug point (byebug) right after I instantiate the threads and tried to call the function and it runs fine but when I join the threads, nothing happens.

Things I tried so far

any more ideas?

Edit

this is my current DatabaseCleaner configuration

RSpec.configure do |config|
  config.before(:suite) do
    DatabaseCleaner.clean_with(:deletion)
  end

  config.before do
    DatabaseCleaner.strategy = :transaction
  end

  config.before(:each, js: true) do
    DatabaseCleaner.strategy = :deletion
  end

  config.before do
    DatabaseCleaner.start
  end

  config.after do
    DatabaseCleaner.clean
  end
end

I still haven't tried to modify the strategy to :deleteion, I will do it as well

Upvotes: 5

Views: 4132

Answers (3)

Rimian
Rimian

Reputation: 38418

Another approach is to stub out the active record object and negate the database cleaner completely. Something like this (roughly):

let(:bank_account) { double('bank_account') }

before do
  allow(sweeper).to receive(:each).and_return([bank_account])
  allow(bank_account).to receive(:with_lock).and_yield
end

it 'locks' do
  expect(bank_account).to receive(:with_lock)
  ImportService.new.import_transactions
end

Please note this code example is very rough.

Upvotes: 0

Scott Bartell
Scott Bartell

Reputation: 2840

It looks like there's a couple things going on here.

First, the heart of the issue likely lies in a couple areas:

  1. Database Cleaning Strategy: You're using the DatabaseCleaner gem which provide three cleaning strategies: Truncation, Transaction, and Deletion. My guess is that if you're using the Transaction strategy, the lock is never released because the first transaction is never committed and the second thread just waits for it to be released.
  2. Database Pool Config: Another possible theory is that the connection pool for your test database is too small. This would mean that one (or both) of your threads are waiting to obtain a database connection. Typically there is a timeout configured for this and if one is set you should see an exception that looks something like this: could not obtain a connection from the pool within 5.000 seconds. To fix this, in your database.yml under test adjust your pool setting. You can inspect the values that are set by looking at:
irb(main):001:0> ActiveRecord::Base.connection.pool.size
=> 5
irb(main):001:0> ActiveRecord::Base.connection.pool.checkout_timeout
=> 5

Second, in the code you provided, unless import modifies the transactions or the bank account that it imports, it appears as if the with_lock will not actually prevent multiple uploads.. it will just ensure that they run sequentially.

You might need to do something like this:

 bank_account.with_lock do
   unimported_transactions.each do |transaction|
     import(bank_account, transaction)
     transaction.mark_as_imported!
   end
 end

Additionally, if import is making some sort of external request, you should be careful around partial failures and rollbacks. (the with_lock wraps all SQL queries within it in a database transaction, and if an exception is thrown it will all rollback on your database but not on the external service)

Upvotes: 2

tejasbubane
tejasbubane

Reputation: 942

with_lock is Rails's implementation which we don't need to test. You can use a mock and check if your code calls with_lock. The only trick here is to ensure the transaction gets imported (ie. the code inside with_lock gets executed). RSpec will provide the block which you can call. Below is a snippet of how you can do it - full working implementation can be found here.

describe "#import_transactions" do
  it "runs with lock" do
    # Test if with_lock is getting called
    expect(subject).to receive(:with_lock) do |*_args, &block|
      # block is provided to with_lock method
      # execute the block and test if it creates transactions
      expect { block.call }
        .to change { BankAccountTransaction.count }.from(0).to(2)
    end

    ImportService.new.import_transactions(subject, transactions)
  end
end

Upvotes: 4

Related Questions