Update tests after refactoring

Author: mayorova

test/integration/finance/stripe_rate_limit_handling_test.rb

@@ -20,49 +20,46 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
 
   # ==================== Unit Tests for Rate Limit Detection ====================
 
-  test 'PaymentTransaction detects rate limit from HTTP 429 status code' do
-    transaction = PaymentTransaction.new
+  test 'StripeChargeService#rate_limit_error? detects rate limit from Stripe error code' do
+    service = prepare_stripe_charge_service
 
     response = stub(
       success?: false,
-      params: { 'error' => { 'http_code' => 429 } },
-      message: 'Rate limit exceeded'
+      params: { 'error' => { 'code' => 'rate_limit' } }
     )
 
-    assert transaction.send(:rate_limit_error?, response),
-           'Should detect rate limit from HTTP 429 status code'
+    assert service.send(:rate_limit_error?, response),
+           'Should detect rate limit from Stripe rate_limit error code'
   end
 
-  test 'PaymentTransaction detects rate limit from error message' do
-    transaction = PaymentTransaction.new
+  test 'StripeChargeService#rate_limit_error? does not false-positive on other errors' do
+    service = prepare_stripe_charge_service
 
     response = stub(
       success?: false,
-      params: {},
-      message: 'Too many requests - please try again later'
+      params: { 'error' => { 'code' => 'card_declined' } }
     )
 
-    assert transaction.send(:rate_limit_error?, response),
-           'Should detect rate limit from error message'
+    assert_not service.send(:rate_limit_error?, response),
+               'Should not detect rate limit for other error codes'
   end
 
-  test 'PaymentTransaction does not false-positive on other errors' do
-    transaction = PaymentTransaction.new
+  test 'StripeChargeService#rate_limit_error? does not detect rate limit on successful response' do
+    service = prepare_stripe_charge_service
 
     response = stub(
-      success?: false,
-      params: { 'error' => { 'http_code' => 402 } },
-      message: 'Card declined'
+      success?: true,
+      params: {}
     )
 
-    assert_not transaction.send(:rate_limit_error?, response),
-               'Should not detect rate limit for other error codes'
+    assert_not service.send(:rate_limit_error?, response),
+               'Should not detect rate limit on successful response'
   end
 
   # ==================== Integration Tests for Invoice Charging ====================
 
-  test 'Invoice charge re-raises RateLimitError when Stripe returns 429' do
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+  test 'Invoice charge re-raises RateLimitError when Stripe returns rate limit error' do
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
 
     invoice = prepare_invoice
 
@@ -103,23 +100,12 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
 
   # ==================== BillingWorker Retry Logic Tests ====================
 
-  test 'BillingWorker uses exponential backoff for rate limit errors' do
-    rate_limit_error = Finance::Payment::StripeRateLimitError.new
-
-    # First retry: ~15 seconds (3^1 * 5 = 15)
-    retry_delay_1 = BillingWorker.sidekiq_retry_in_block.call(1, rate_limit_error)
-    assert retry_delay_1 >= 15, "First retry should be >= 15 seconds, got #{retry_delay_1}"
-    assert retry_delay_1 <= 25, "First retry should be <= 25 seconds (with jitter), got #{retry_delay_1}"
+  test 'BillingWorker uses Sidekiq default exponential backoff for rate limit errors' do
+    rate_limit_error = create_rate_limit_error
 
-    # Second retry: ~45 seconds (3^2 * 5 = 45)
-    retry_delay_2 = BillingWorker.sidekiq_retry_in_block.call(2, rate_limit_error)
-    assert retry_delay_2 >= 45, "Second retry should be >= 45 seconds, got #{retry_delay_2}"
-    assert retry_delay_2 <= 55, "Second retry should be <= 55 seconds (with jitter), got #{retry_delay_2}"
-
-    # Third retry: ~135 seconds (3^3 * 5 = 135)
-    retry_delay_3 = BillingWorker.sidekiq_retry_in_block.call(3, rate_limit_error)
-    assert retry_delay_3 >= 135, "Third retry should be >= 135 seconds, got #{retry_delay_3}"
-    assert retry_delay_3 <= 145, "Third retry should be <= 145 seconds (with jitter), got #{retry_delay_3}"
+    # Returns nil to use Sidekiq's default exponential backoff
+    retry_delay = BillingWorker.sidekiq_retry_in_block.call(1, rate_limit_error)
+    assert_nil retry_delay, "Should return nil to use Sidekiq's default exponential backoff"
   end
 
   test 'BillingWorker uses standard retry delay for non-rate-limit errors' do
@@ -135,7 +121,7 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
   # ==================== Job-Level Integration Tests ====================
 
   test 'BillingService re-raises rate limit error and releases lock' do
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
 
     invoice = prepare_invoice
 
@@ -147,13 +133,15 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
     assert_instance_of Finance::Payment::StripeRateLimitError, error
 
     # Lock should be released - we can acquire it again immediately
+    lock_service = Synchronization::BillingLockService.new(@buyer.id.to_s)
     assert_nothing_raised do
-      Finance::BillingService.new(@buyer.id, provider_account_id: @provider.id, now: invoice.due_on).send(:acquire_lock)
+      lock_service.lock
     end
+    lock_service.unlock
   end
 
   test 'rate limit error flow through BillingWorker' do
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
 
     invoice = prepare_invoice
 
@@ -182,7 +170,7 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
     # First attempt: Invoice 1 succeeds, Invoice 2 hits rate limit
     call_sequence = sequence('charging')
     Account.any_instance.expects(:charge!).with(invoice1.cost, invoice: invoice1).returns(true).in_sequence(call_sequence)
-    Account.any_instance.expects(:charge!).with(invoice2.cost, invoice: invoice2).raises(Finance::Payment::StripeRateLimitError.new).in_sequence(call_sequence)
+    Account.any_instance.expects(:charge!).with(invoice2.cost, invoice: invoice2).raises(create_rate_limit_error).in_sequence(call_sequence)
 
     # Job fails with rate limit
     assert_raises(Finance::Payment::StripeRateLimitError) do
@@ -214,12 +202,12 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
     invoice = prepare_invoice
 
     # First rate limit error
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
     assert_raises(Finance::Payment::StripeRateLimitError) { invoice.charge! }
     assert_equal 0, invoice.reload.charging_retries_count
 
     # Second rate limit error
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
     assert_raises(Finance::Payment::StripeRateLimitError) { invoice.charge! }
     assert_equal 0, invoice.reload.charging_retries_count
 
@@ -231,7 +219,7 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
     invoice = prepare_invoice
 
     # First attempt: rate limit
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
     assert_raises(Finance::Payment::StripeRateLimitError) { invoice.charge! }
 
     # Second attempt: success
@@ -246,7 +234,7 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
     invoice = prepare_invoice
 
     # First attempt: rate limit
-    Account.any_instance.expects(:charge!).raises(Finance::Payment::StripeRateLimitError.new)
+    Account.any_instance.expects(:charge!).raises(create_rate_limit_error)
     assert_raises(Finance::Payment::StripeRateLimitError) { invoice.charge! }
     assert_equal 0, invoice.reload.charging_retries_count
 
@@ -260,6 +248,32 @@ class Finance::StripeRateLimitHandlingTest < ActionDispatch::IntegrationTest
 
   private
 
+  def prepare_stripe_charge_service(invoice: nil)
+    invoice ||= prepare_invoice
+    gateway = stub('gateway')
+    Finance::StripeChargeService.new(
+      gateway,
+      payment_method_id: 'pm_test',
+      invoice: invoice,
+      gateway_options: {}
+    )
+  end
+
+  def create_rate_limit_error
+    response = stub(
+      success?: false,
+      params: { 'error' => { 'code' => 'rate_limit' } },
+      message: 'Request rate limit exceeded'
+    )
+    payment_metadata = {
+      invoice_id: 123,
+      buyer_id: 456,
+      payment_method_id: 'pm_test',
+      gateway_options: {}
+    }
+    Finance::Payment::StripeRateLimitError.new(response, payment_metadata)
+  end
+
   def prepare_invoice
     prepare_invoice_with_id('00000001')
   end

test/unit/services/synchronization/billing_lock_service_test.rb

@@ -0,0 +1,168 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class Synchronization::BillingLockServiceTest < ActionDispatch::IntegrationTest
+  include BillingResultsTestHelpers
+
+  setup do
+    @account_id = SecureRandom.random_number(100000).to_s
+  end
+
+  teardown do
+    clear_billing_locks
+  end
+
+  test "initializes with account_id and creates billing resource key" do
+    service = Synchronization::BillingLockService.new(@account_id)
+
+    assert_equal @account_id, service.account_id
+    assert_equal "lock:billing:#{@account_id}", service.send(:lock_key)
+  end
+
+  test "uses default timeout of 1 hour" do
+    service = Synchronization::BillingLockService.new(@account_id)
+
+    assert_equal 1.hour.in_milliseconds, service.send(:timeout)
+  end
+
+  test "allows custom timeout" do
+    custom_timeout = 30.minutes.in_milliseconds
+    service = Synchronization::BillingLockService.new(@account_id, timeout: custom_timeout)
+
+    assert_equal custom_timeout, service.send(:timeout)
+  end
+
+  test "lock acquires lock and stores lock info" do
+    service = Synchronization::BillingLockService.new(@account_id)
+
+    service.lock
+
+    assert_not_nil service.lock_info
+    assert_instance_of Hash, service.lock_info
+    assert service.lock_info.key?(:validity)
+    assert service.lock_info.key?(:resource)
+  end
+
+  test "lock raises LockBillingError when lock is already held" do
+    service1 = Synchronization::BillingLockService.new(@account_id)
+    service2 = Synchronization::BillingLockService.new(@account_id)
+
+    service1.lock
+
+    error = assert_raises(Finance::LockBillingError) do
+      service2.lock
+    end
+
+    assert_match(/Concurrent billing job already running for account #{@account_id}/, error.message)
+  end
+
+  test "unlock releases the lock" do
+    service1 = Synchronization::BillingLockService.new(@account_id)
+    service2 = Synchronization::BillingLockService.new(@account_id)
+
+    # First service acquires lock
+    service1.lock
+
+    # Second service cannot acquire lock
+    assert_raises(Finance::LockBillingError) { service2.lock }
+
+    # First service releases lock
+    service1.unlock
+
+    # Now second service can acquire lock
+    assert_nothing_raised { service2.lock }
+
+    service2.unlock
+  end
+
+  test "unlock is idempotent and does not raise if lock is not held" do
+    service = Synchronization::BillingLockService.new(@account_id)
+
+    # Unlock without locking should not raise
+    assert_nothing_raised { service.unlock }
+
+    # Lock and unlock
+    service.lock
+    service.unlock
+
+    # Unlock again should not raise
+    assert_nothing_raised { service.unlock }
+  end
+
+  test "unlock sets lock_info to nil" do
+    service = Synchronization::BillingLockService.new(@account_id)
+
+    service.lock
+    assert_not_nil service.lock_info
+
+    service.unlock
+    assert_nil service.lock_info
+  end
+
+  test "unlock logs warning if unlock fails" do
+    service = Synchronization::BillingLockService.new(@account_id)
+    service.lock
+
+    # Simulate unlock failure
+    service.send(:manager).expects(:unlock).raises(StandardError.new("Redis error"))
+
+    Rails.logger.expects(:warn).with(
+      "Failed to release billing lock for account #{@account_id}: Redis error"
+    )
+
+    service.unlock
+  end
+
+  test "works with block when called" do
+    executed = false
+
+    result = Synchronization::BillingLockService.new(@account_id, timeout: 10000) do
+      executed = true
+    end.call
+
+    assert executed, "Block should have been executed"
+    assert result, "Should return truthy value when block executes successfully"
+  end
+
+  test "different account IDs have separate locks" do
+    account_id_1 = "123"
+    account_id_2 = "456"
+
+    service1 = Synchronization::BillingLockService.new(account_id_1)
+    service2 = Synchronization::BillingLockService.new(account_id_2)
+
+    # Both should be able to acquire locks simultaneously
+    assert_nothing_raised { service1.lock }
+    assert_nothing_raised { service2.lock }
+
+    service1.unlock
+    service2.unlock
+  end
+
+  test "lock expires after timeout" do
+    service1 = Synchronization::BillingLockService.new(@account_id, timeout: 100)
+    service2 = Synchronization::BillingLockService.new(@account_id, timeout: 100)
+
+    service1.lock
+
+    # Wait for lock to expire
+    sleep 0.12
+
+    # Should be able to acquire lock again
+    assert_nothing_raised { service2.lock }
+
+    service2.unlock
+  end
+
+  test "inherits from NowaitLockService" do
+    assert Synchronization::BillingLockService < Synchronization::NowaitLockService
+  end
+
+  test "call method works via parent class" do
+    # The parent NowaitLockService has a call method that should work
+    result = Synchronization::BillingLockService.new(@account_id, timeout: 10000).call
+
+    assert result, "Should successfully acquire lock via call method"
+  end
+end

test/unit/services/synchronization/nowait_lock_service_test.rb

@@ -46,4 +46,17 @@ class Synchronization::NowaitLockServiceTest < ActionDispatch::IntegrationTest
     sleep 0.12
     assert Synchronization::NowaitLockService.call(lock_key, timeout: 100).result
   end
+
+  test "lock_key prefixes resource with 'lock:'" do
+    service = Synchronization::NowaitLockService.new("billing:123", timeout: 10000)
+
+    assert_equal "lock:billing:123", service.send(:lock_key)
+  end
+
+  test "accepts timeout as keyword argument" do
+    timeout_ms = 5000
+    service = Synchronization::NowaitLockService.new(lock_key, timeout: timeout_ms)
+
+    assert_equal timeout_ms, service.send(:timeout)
+  end
 end