From e9b638fda5bf14132ce9cd469a72e4a777215846 Mon Sep 17 00:00:00 2001
From: Fumiaki MATSUSHIMA <mtsmfm@gmail.com>
Date: Wed, 15 Mar 2017 23:57:45 +0900
Subject: Fix fragile test
 (`AssociationProxyTest#test_save_on_parent_saves_children`)

If we run only following tests:

- test/cases/scoping/default_scoping_test.rb
- test/cases/associations_test.rb

```
$ cat Rakefile.test
require "rake/testtask"

ENV["ARCONN"] = "postgresql"

Rake::TestTask.new do |t|
  t.libs << "test"
  t.test_files = %w(
    test/cases/scoping/default_scoping_test.rb
    test/cases/associations_test.rb
  )
end
```

a test will fail:

```
$ bundle exec rake test -f Rakefile.test
/app/activesupport/lib/active_support/core_ext/enumerable.rb:20: warning: method redefined; discarding old sum
Using postgresql
Run options: --seed 11830

# Running:

.........................................................................................F................

Finished in 6.939055s, 15.2759 runs/s, 27.9577 assertions/s.

  1) Failure:
AssociationProxyTest#test_save_on_parent_saves_children [/app/activerecord/test/cases/associations_test.rb:185]:
Expected: 1
  Actual: 2

106 runs, 194 assertions, 1 failures, 0 errors, 0 skips
rake aborted!
Command failed with status (1)
/usr/local/bin/bundle:22:in `load'
/usr/local/bin/bundle:22:in `<main>'
Tasks: TOP => test
(See full trace by running task with --trace)
```

In #28083, change `self.use_transactional_tests` to `false`
but we forget to clean-up fixture.
However we don't have to disable transaction except a few tests.
---
 .../test/cases/scoping/default_scoping_test.rb     | 70 +++++++++++-----------
 1 file changed, 34 insertions(+), 36 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/test/cases/scoping/default_scoping_test.rb b/activerecord/test/cases/scoping/default_scoping_test.rb
index 14fb2fbbfa..a6c22ac672 100644
--- a/activerecord/test/cases/scoping/default_scoping_test.rb
+++ b/activerecord/test/cases/scoping/default_scoping_test.rb
@@ -10,8 +10,6 @@ require "concurrent/atomic/cyclic_barrier"
 class DefaultScopingTest < ActiveRecord::TestCase
   fixtures :developers, :posts, :comments
 
-  self.use_transactional_tests = false
-
   def test_default_scope
     expected = Developer.all.merge!(order: "salary DESC").to_a.collect(&:salary)
     received = DeveloperOrderedBySalary.all.collect(&:salary)
@@ -61,17 +59,6 @@ class DefaultScopingTest < ActiveRecord::TestCase
     assert_equal "Jamis", DeveloperCalledJamis.create!.name
   end
 
-  unless in_memory_db?
-    def test_default_scoping_with_threads
-      2.times do
-        Thread.new {
-          assert_includes DeveloperOrderedBySalary.all.to_sql, "salary DESC"
-          DeveloperOrderedBySalary.connection.close
-        }.join
-      end
-    end
-  end
-
   def test_default_scope_with_inheritance
     wheres = InheritedPoorDeveloperCalledJamis.all.where_values_hash
     assert_equal "Jamis", wheres["name"]
@@ -435,29 +422,6 @@ class DefaultScopingTest < ActiveRecord::TestCase
     assert_equal comment, CommentWithDefaultScopeReferencesAssociation.find_by(id: comment.id)
   end
 
-  unless in_memory_db?
-    def test_default_scope_is_threadsafe
-      threads = []
-      assert_not_equal 1, ThreadsafeDeveloper.unscoped.count
-
-      barrier_1 = Concurrent::CyclicBarrier.new(2)
-      barrier_2 = Concurrent::CyclicBarrier.new(2)
-
-      threads << Thread.new do
-        Thread.current[:default_scope_delay] = -> { barrier_1.wait; barrier_2.wait }
-        assert_equal 1, ThreadsafeDeveloper.all.to_a.count
-        ThreadsafeDeveloper.connection.close
-      end
-      threads << Thread.new do
-        Thread.current[:default_scope_delay] = -> { barrier_2.wait }
-        barrier_1.wait
-        assert_equal 1, ThreadsafeDeveloper.all.to_a.count
-        ThreadsafeDeveloper.connection.close
-      end
-      threads.each(&:join)
-    end
-  end
-
   test "additional conditions are ANDed with the default scope" do
     scope = DeveloperCalledJamis.where(name: "David")
     assert_equal 2, scope.where_clause.ast.children.length
@@ -506,3 +470,37 @@ class DefaultScopingTest < ActiveRecord::TestCase
     assert_match gender_pattern, Lion.female.to_sql
   end
 end
+
+class DefaultScopingWithThreadTest < ActiveRecord::TestCase
+  self.use_transactional_tests = false
+
+  def test_default_scoping_with_threads
+    2.times do
+      Thread.new {
+        assert_includes DeveloperOrderedBySalary.all.to_sql, "salary DESC"
+        DeveloperOrderedBySalary.connection.close
+      }.join
+    end
+  end
+
+  def test_default_scope_is_threadsafe
+    threads = []
+    assert_not_equal 1, ThreadsafeDeveloper.unscoped.count
+
+    barrier_1 = Concurrent::CyclicBarrier.new(2)
+    barrier_2 = Concurrent::CyclicBarrier.new(2)
+
+    threads << Thread.new do
+      Thread.current[:default_scope_delay] = -> { barrier_1.wait; barrier_2.wait }
+      assert_equal 1, ThreadsafeDeveloper.all.to_a.count
+      ThreadsafeDeveloper.connection.close
+    end
+    threads << Thread.new do
+      Thread.current[:default_scope_delay] = -> { barrier_2.wait }
+      barrier_1.wait
+      assert_equal 1, ThreadsafeDeveloper.all.to_a.count
+      ThreadsafeDeveloper.connection.close
+    end
+    threads.each(&:join)
+  end
+end unless in_memory_db?
-- 
cgit v1.2.3