From d81c8679dd4411f93917faae0f5568c7d6936f96 Mon Sep 17 00:00:00 2001
From: Anuj Biyani <abiyani@lytro.com>
Date: Mon, 10 Jun 2013 15:05:15 -0700
Subject: [PATCH] extract some logic into a more testable method

---
 .../aws-ha-release/aws-ha-release.rb          | 16 ++++--
 .../aws-ha-release/aws-ha-release_spec.rb     | 51 ++++++++++++-------
 2 files changed, 45 insertions(+), 22 deletions(-)

diff --git a/lib/aws-missing-tools/aws-ha-release/aws-ha-release.rb b/lib/aws-missing-tools/aws-ha-release/aws-ha-release.rb
index 7b608ee..cc48c64 100755
--- a/lib/aws-missing-tools/aws-ha-release/aws-ha-release.rb
+++ b/lib/aws-missing-tools/aws-ha-release/aws-ha-release.rb
@@ -89,10 +89,10 @@ module AwsMissingTools
 
       @group.suspend_processes PROCESSES_TO_SUSPEND
 
-      if @group.max_size == @group.desired_capacity
-        puts "#{@group.name} has a max-size of #{@group.max_size}. In order to recycle instances max-size will be temporarily increased by #{@opts[:num_simultaneous_instances]}."
-        @group.update(max_size: @group.max_size + @opts[:num_simultaneous_instances])
-        @max_size_change = @opts[:num_simultaneous_instances]
+      @max_size_change = determine_max_size_change
+      if @max_size_change > 0
+        puts "#{@group.name} has a max-size of #{@group.max_size}. In order to recycle instances max-size will be temporarily increased by #{@max_size_change}."
+        @group.update(max_size: @group.max_size + @max_size_change)
       end
 
       @group.update(desired_capacity: @group.desired_capacity + @opts[:num_simultaneous_instances])
@@ -147,6 +147,14 @@ module AwsMissingTools
       @group.resume_all_processes
     end
 
+    def determine_max_size_change
+      if @group.max_size - @group.desired_capacity < @opts[:num_simultaneous_instances]
+        @group.desired_capacity + @opts[:num_simultaneous_instances] - @group.max_size
+      else
+        0
+      end
+    end
+
     def deregister_instance(instance, load_balancers)
       load_balancers.each do |load_balancer|
         load_balancer.instances.deregister instance
diff --git a/spec/aws-missing-tools/aws-ha-release/aws-ha-release_spec.rb b/spec/aws-missing-tools/aws-ha-release/aws-ha-release_spec.rb
index a404ae4..04736b5 100644
--- a/spec/aws-missing-tools/aws-ha-release/aws-ha-release_spec.rb
+++ b/spec/aws-missing-tools/aws-ha-release/aws-ha-release_spec.rb
@@ -123,24 +123,6 @@ describe 'aws-ha-release' do
       @aws_ha_release.group.should_receive(:update).with(desired_capacity: 1).ordered.and_call_original
       @aws_ha_release.execute!
     end
-
-    it 'cycles more than one instance at a time if specified' do
-      @group.update(max_size: 2, desired_capacity: 2)
-
-      aws_ha_release = AwsMissingTools::AwsHaRelease.new(%w(-a test_group --num-simultaneous-instances 2 -o testaccesskey -s testsecretkey -r test_region -i 1 -t 0 -m 5))
-      aws_ha_release.stub!(:all_instances_inservice_for_time_period?).and_return(true)
-
-      aws_ha_release.group.should_receive(:update).with(max_size: 4).ordered.and_call_original
-      aws_ha_release.group.should_receive(:update).with(desired_capacity: 4).ordered.and_call_original
-
-      aws_ha_release.should_receive(:deregister_instance).twice.ordered.and_call_original
-      @group.auto_scaling_instances[0].should_receive(:terminate).ordered.and_call_original
-      @group.auto_scaling_instances[1].should_receive(:terminate).ordered.and_call_original
-
-      aws_ha_release.group.should_receive(:update).with(desired_capacity: 2).ordered.and_call_original
-      aws_ha_release.group.should_receive(:update).with(max_size: 2).ordered.and_call_original
-      aws_ha_release.execute!
-    end
   end
 
   describe 'determining if instances are in service' do
@@ -289,4 +271,37 @@ describe 'aws-ha-release' do
       expect(elb_two.instances).not_to include instance_one
     end
   end
+
+  describe '#determine_max_size_change' do
+    before do
+      @group = as.groups.create opts[1]
+    end
+
+    it 'does not change the desired capacity by default' do
+      @group.update(max_size: 4, desired_capacity: 2)
+      aws_ha_release = AwsMissingTools::AwsHaRelease.new(opts)
+
+      expect(aws_ha_release.determine_max_size_change).to eq 0
+    end
+
+    it 'adjusts the max size when it is equal to the desired capacity' do
+      @group.update(max_size: 2, desired_capacity: 2)
+      aws_ha_release = AwsMissingTools::AwsHaRelease.new(opts)
+
+      expect(aws_ha_release.determine_max_size_change).to eq 1
+    end
+
+    it 'accounts for num_simultaneous_instances' do
+      @group.update(max_size: 2, desired_capacity: 2)
+      aws_ha_release = AwsMissingTools::AwsHaRelease.new(%w(-a test_group --num-simultaneous-instances 2 -o testaccesskey -s testsecretkey -r test_region -i 1 -t 0 -m 5))
+
+      expect(aws_ha_release.determine_max_size_change).to eq 2
+
+      @group.update(max_size: 3, desired_capacity: 2)
+      expect(aws_ha_release.determine_max_size_change).to eq 1
+
+      @group.update(max_size: 4, desired_capacity: 2)
+      expect(aws_ha_release.determine_max_size_change).to eq 0
+    end
+  end
 end