Add some tests

Shopify · Jul 13, 2018 · d39a44a · d39a44a
1 parent d6e0014
commit d39a44a
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 11 deletions.
diff --git a/lib/kubernetes-deploy/sync_mediator.rb b/lib/kubernetes-deploy/sync_mediator.rb
@@ -32,7 +32,7 @@ def get_all(kind, selector = nil)
     def sync(resources)
       clear_cache
 
-      if resources.count < LARGE_BATCH_THRESHOLD
+      if resources.count > LARGE_BATCH_THRESHOLD
         dependencies = resources.map(&:class).uniq.flat_map do |c|
           c::SYNC_DEPENDENCIES if c.const_defined?('SYNC_DEPENDENCIES')
         end

diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -192,7 +192,7 @@ def fixture_path(set_name)
 
     def stub_kubectl_response(*args, resp:, err: "", success: true, json: true, times: 1)
       resp = resp.to_json if json
-      response = [resp, err, stub(success?: success)]
+      response = times > 0 ? [resp, err, stub(success?: success)] : []
       KubernetesDeploy::Kubectl.any_instance.expects(:run)
         .with(*args)
         .returns(response)

diff --git a/test/unit/sync_mediator_test.rb b/test/unit/sync_mediator_test.rb
@@ -90,7 +90,8 @@ def test_sync_caches_the_types_of_the_instances_given
     all_cm = [@fake_cm.kubectl_response, @fake_cm2.kubectl_response]
     stub_kubectl_response('get', 'FakeConfigMap', *@params, resp: { "items" => all_cm }, times: 1)
     stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1)
-    mediator.sync([@fake_cm, @fake_pod])
+    # Cache is only warmed if we batch fetch
+    mediator.sync([@fake_cm, @fake_pod] * KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD)
 
     # These should use the warm cache
     assert_equal @fake_cm.kubectl_response, mediator.get_instance('FakeConfigMap', @fake_cm.name)
@@ -106,7 +107,9 @@ def test_sync_caches_the_types_of_the_dependencies_of_the_instances_given
     stub_kubectl_response('get', 'FakeDeployment', *@params,
       resp: { "items" => [@fake_deployment.kubectl_response] }, times: 1)
     stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1)
-    mediator.sync([@fake_deployment]) # pod is a depedency so should get cached too
+    # Dependency fetching is only done if we batch fetch
+    # pod is a depedency so should get cached too
+    mediator.sync([@fake_deployment] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1))
 
     assert_equal @fake_deployment.kubectl_response, mediator.get_instance('FakeDeployment', @fake_deployment.name)
     assert_equal @fake_pod.kubectl_response, mediator.get_instance('FakePod', @fake_pod.name)
@@ -118,18 +121,31 @@ def test_calling_instance_sync_does_not_allow_instances_to_affect_the_global_cac
     bad_citizen = BadCitizen.new('foo')
     stub_kubectl_response('get', 'BadCitizen', *@params, resp: { "items" => [bad_citizen.kubectl_response] }, times: 1)
 
-    mediator.sync([bad_citizen])
+    # Cache is only warmed if we batch fetch
+    mediator.sync([bad_citizen] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1))
     assert_equal bad_citizen.kubectl_response, mediator.get_instance('BadCitizen', bad_citizen.name) # still cached
   end
 
   def test_sync_calls_sync_on_each_instance
-    stub_kubectl_response('get', 'FakeDeployment', *@params,
-      resp: { "items" => [@fake_deployment.kubectl_response] }, times: 1)
-    stub_kubectl_response('get', 'FakePod', *@params, resp: { "items" => [@fake_pod.kubectl_response] }, times: 1)
-    all_cm = [@fake_cm.kubectl_response, @fake_cm2.kubectl_response]
-    stub_kubectl_response('get', 'FakeConfigMap', *@params, resp: { "items" => all_cm }, times: 1)
+    test_resources = [@fake_pod, @fake_cm, @fake_deployment]
+    test_resources.each { |r| r.expects(:sync).once }
+    mediator.sync(test_resources)
+  end
+
+  def test_sync_does_not_batch_with_few_resources
+    stub_kubectl_response('get', 'FakeConfigMap', @fake_cm.name, *@params,
+      resp: { "items" => [@fake_cm.kubectl_response] }, times: 0)
+
+    test_resources = [@fake_cm]
+    test_resources.each { |r| r.expects(:sync).once }
+    mediator.sync(test_resources)
+  end
+
+  def test_sync_does_batch_with_enough_resources
+    stub_kubectl_response('get', 'FakeConfigMap', *@params,
+      resp: { "items" => [@fake_cm.kubectl_response] }, times: 1)
 
-    test_resources = [@fake_pod, @fake_cm, @fake_cm2, @fake_deployment]
+    test_resources = [@fake_cm] * (KubernetesDeploy::SyncMediator::LARGE_BATCH_THRESHOLD + 1)
     test_resources.each { |r| r.expects(:sync).once }
     mediator.sync(test_resources)
   end