Merge pull request #8 from abicky/fix-flaky-tests

Fix flaky test

spec/activerecord/debug_errors/ext/connection_adapters/abstract_mysql_adapter_spec.rb

@@ -1,4 +1,5 @@
 require "spec_helper"
+require "support/cyclic_barrier"
 
 RSpec.describe ActiveRecord::DebugErrors::DisplayConnectionOwners do
   let(:log) { StringIO.new }
@@ -19,12 +20,14 @@
   describe "#execute" do
     context "when ActiveRecord::Deadlocked occurs" do
       def cause_deadlock(role:)
+        barrier = CyclicBarrier.new(2)
+
         ths = []
         ths << Thread.new do
           ActiveRecord::Base.connected_to(role: role) do
             User.transaction do
               User.lock.find_by!(name: 'foo')
-              sleep 0.1
+              barrier.await(1)
               User.lock.find_by!(name: 'bar')
             end
           end
@@ -34,7 +37,7 @@ def cause_deadlock(role:)
           ActiveRecord::Base.connected_to(role: role) do
             User.transaction do
               User.lock.find_by!(name: 'bar')
-              sleep 0.1
+              barrier.await(1)
               User.lock.find_by!(name: 'foo')
             end
           end
@@ -67,18 +70,15 @@ def cause_deadlock(role:)
 
     context "when ActiveRecord::LockWaitTimeout occurs" do
       it "displays transactions and processlist" do
-        ths = []
-        ths << Thread.new do
-          User.transaction do
-            User.lock.find_by!(name: 'foo')
-            sleep 2
-          end
-        end
+        barrier = CyclicBarrier.new(2)
 
-        ths << Thread.new do
-          User.transaction do
-            User.lock.find_by!(name: 'foo')
-            sleep 2
+        ths = Array.new(2) do
+          Thread.new do
+            User.transaction do
+              barrier.await(1)
+              User.lock.find_by!(name: 'foo')
+              sleep 2
+            end
           end
         end
 

spec/activerecord/debug_errors/ext/connection_adapters/connection_pool_spec.rb

@@ -1,4 +1,5 @@
 require "spec_helper"
+require "support/cyclic_barrier"
 
 RSpec.describe ActiveRecord::DebugErrors::DisplayConnectionOwners do
   let(:log) { StringIO.new }
@@ -20,20 +21,18 @@
     it "displays connection owners and other threads" do
       Thread.new { sleep 10 } # another thread
 
-      mutex = Mutex.new
-      cv = ConditionVariable.new
+      barrier = CyclicBarrier.new(ActiveRecord::Base.connection_pool.size)
 
       expect {
         ActiveRecord::Base.connection # Ensure to acquire a connection
         Array.new(ActiveRecord::Base.connection_pool.size) do
           Thread.new do
-            mutex.synchronize do
-              ActiveRecord::Base.connection_pool.checkout(0.1)
-              cv.wait(mutex, 1)
-            rescue
-              cv.broadcast
-              raise
-            end
+            ActiveRecord::Base.connection_pool.checkout(0.1)
+            barrier.await(1)
+          rescue Timeout::Error
+            # CyclicBarrier#await is expected to raise Timeout::Error
+            # because it is not called ActiveRecord::Base.connection_pool.size times
+            # due to ActiveRecord::ConnectionTimeoutError
           end
         end.each(&:join)
       }.to raise_error(ActiveRecord::ConnectionTimeoutError)

spec/support/cyclic_barrier.rb

@@ -0,0 +1,21 @@
+# This class is a simple implementation of CyclicBarrier in Java
+class CyclicBarrier
+  def initialize(parties)
+    @cv = ConditionVariable.new
+    @mutex = Mutex.new
+    @parties = parties
+    @number_waiting = 0
+  end
+
+  def await(timeout = nil)
+    @mutex.synchronize do
+      @number_waiting += 1
+      if @number_waiting == @parties
+        @cv.broadcast
+      else
+        @cv.wait(@mutex, timeout)
+        raise Timeout::Error if @number_waiting != @parties
+      end
+    end
+  end
+end