Minor cleanup to Milestone model and spec

Back-ported from my still-in-progress major cleanup.

2 files changed, 21 insertions, 20 deletions

diff --git a/app/models/milestone.rb b/app/models/milestone.rb
index 592c57f948c..d416fb630c5 100644
--- a/app/models/milestone.rb
+++ b/app/models/milestone.rb
@@ -28,17 +28,9 @@ class Milestone < ActiveRecord::Base
   end
 
   def percent_complete
-    @percent_complete ||= begin
-                            total_i = self.issues.count
-                            closed_i = self.issues.closed.count
-                            if total_i > 0
-                              (closed_i * 100) / total_i
-                            else
-                              100
-                            end
-                          rescue => ex
-                            0
-                          end
+    ((self.issues.closed.count * 100) / self.issues.count).abs
+  rescue ZeroDivisionError
+    100
   end
 
   def expires_at
diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb
index 880d3f307a3..e9acc4e2815 100644
--- a/spec/models/milestone_spec.rb
+++ b/spec/models/milestone_spec.rb
@@ -31,24 +31,33 @@ describe Milestone do
 
   it { milestone.should be_valid }
 
-  describe "Issues" do 
-    before do 
+  describe "#percent_complete" do
+    it "should not count open issues" do
       milestone.issues << issue
+      milestone.percent_complete.should == 0
     end
 
-    it { milestone.percent_complete.should == 0 }
+    it "should count closed issues" do
+      issue.update_attributes(closed: true)
+      milestone.issues << issue
+      milestone.percent_complete.should == 100
+    end
 
-    it do 
-      issue.update_attributes closed: true
+    it "should recover from dividing by zero" do
+      milestone.issues.should_receive(:count).and_return(0)
       milestone.percent_complete.should == 100
     end
   end
 
-  describe :expires_at do 
-    before do 
-      milestone.update_attributes due_date: Date.today + 1.day
+  describe "#expires_at" do
+    it "should be nil when due_date is unset" do
+      milestone.update_attributes(due_date: nil)
+      milestone.expires_at.should be_nil
     end
 
-    it { milestone.expires_at.should_not be_nil }
+    it "should not be nil when due_date is set" do
+      milestone.update_attributes(due_date: Date.tomorrow)
+      milestone.expires_at.should be_present
+    end
   end
 end