Merge pull request #34 from tarbrain/master

Fix crashes when source files are empty.

CHANGELOG.md

@@ -2,9 +2,16 @@
 
 ## master
 
+## v1.5.1
+
+* Avoid crashes when coverage data is empty
+* Fix bug which prevented source files without coverage data to be included in Cobertura xml report  
+  [Julian Krumow](https://github.com/tarbrain)
+  [#34](https://github.com/venmo/slather/pull/34)
+
 ## v1.5.0
 
-* Add support for Cobertura.  
+* Add support for Cobertura  
   [Julian Krumow](https://github.com/tarbrain)
   [#30](https://github.com/venmo/slather/pull/30)
 

lib/slather/coverage_file.rb

@@ -45,6 +45,8 @@ def gcov_data
         gcov_file_name = "./#{source_file_pathname.basename}.gcov"
         if File.exists?(gcov_file_name)
           gcov_data = File.new(gcov_file_name).read
+        else
+          gcov_data = ""
         end
 
         gcov_files_created.each { |file| FileUtils.rm_f(file) }
@@ -53,7 +55,7 @@ def gcov_data
     end
 
     def line_coverage_data
-      if cleaned_gcov_data
+      unless cleaned_gcov_data.empty?
         first_line_start = cleaned_gcov_data =~ /^\s+(-|#+|[0-9+]):\s+1:/
 
         cleaned_gcov_data[first_line_start..-1].split("\n").map do |line|
@@ -92,38 +94,38 @@ def num_lines_testable
     end
 
     def rate_lines_tested
-      (num_lines_tested / num_lines_testable.to_f)
-    end
-
-    def percentage_lines_tested
-      if num_lines_testable > 0 
-        (num_lines_tested / num_lines_testable.to_f) * 100.0
+      if num_lines_testable > 0
+        (num_lines_tested / num_lines_testable.to_f)
       else
         0
       end
     end
 
+    def percentage_lines_tested
+      rate_lines_tested * 100
+    end
+
     def branch_coverage_data
       @branch_coverage_data ||= begin
         branch_coverage_data = Hash.new
 
-        @gcov_data.scan(/(^(\s+(-|#+|[0-9]+):\s+[1-9]+:(.*)$\r?\n)(^branch\s+[0-9]+\s+[a-zA-Z0-9]+\s+[a-zA-Z0-9]+$\r?\n)+)+/) {|data|
-          lines = data[0].split("\n")
-          line_number = lines[0].split(':')[1].strip.to_i
-          branch_coverage_data[line_number] = lines[1..-1].map do |line|
-            if line.split(' ')[2].strip == "never"
-              0
-            else
-              line.split(' ')[3].strip.to_i
+          gcov_data.scan(/(^(\s+(-|#+|[0-9]+):\s+[1-9]+:(.*)$\r?\n)(^branch\s+[0-9]+\s+[a-zA-Z0-9]+\s+[a-zA-Z0-9]+$\r?\n)+)+/) do |data|
+            lines = data[0].split("\n")
+            line_number = lines[0].split(':')[1].strip.to_i
+            branch_coverage_data[line_number] = lines[1..-1].map do |line|
+              if line.split(' ')[2].strip == "never"
+                0
+              else
+                line.split(' ')[3].strip.to_i
+              end
             end
           end
-        }
         branch_coverage_data
       end
     end
 
     def branch_coverage_data_for_statement_on_line(line_number)
-      branch_coverage_data[line_number]
+      branch_coverage_data[line_number] || []
     end
 
     def num_branches_for_statement_on_line(line_number)
@@ -136,7 +138,7 @@ def num_branch_hits_for_statement_on_line(line_number)
 
     def rate_branch_coverage_for_statement_on_line(line_number)
       branch_data = branch_coverage_data_for_statement_on_line(line_number)
-      if branch_data == nil
+      if branch_data.empty?
         0.0
       else
         (num_branch_hits_for_statement_on_line(line_number) / branch_data.length.to_f)
@@ -160,10 +162,10 @@ def num_branches_tested
     end
 
     def rate_branches_tested
-      if (num_branches_testable == 0)
-        0.0
-      else
+      if (num_branches_testable > 0)
         (num_branches_tested / num_branches_testable.to_f)
+      else
+        0.0
       end
     end
 

lib/slather/coverage_service/cobertura_xml_output.rb

@@ -56,7 +56,6 @@ def create_xml_report(coverage_files)
           total_package_branches_tested = 0
 
           package_coverage_files.each do |package_coverage_file|
-            next unless package_coverage_file.gcov_data
             class_node = create_class_node(package_coverage_file)
             class_node.parent = classes_node
             total_package_lines += package_coverage_file.num_lines_testable
@@ -128,7 +127,7 @@ def create_line_node(line, coverage_file)
         line_node['branch'] = "false"
         line_node['hits'] = coverage_file.coverage_for_line(line)
 
-        if coverage_file.branch_coverage_data_for_statement_on_line(line_number)
+        unless coverage_file.branch_coverage_data_for_statement_on_line(line_number).empty?
           line_node['branch'] = "true"  
           conditions_node = Nokogiri::XML::Node.new "conditions", @doc
           conditions_node.parent = line_node

spec/fixtures/cobertura.xml

@@ -48,6 +48,10 @@
             <line number="43" branch="false" hits="2"/>
           </lines>
         </class>
+        <class name="Empty" filename="spec/fixtures/fixtures/more_files/Empty.m" line-rate="0.0000000000000000" branch-rate="0.0000000000000000" complexity="0.0">
+          <methods/>
+          <lines/>
+        </class>
         <class name="peekaview" filename="spec/fixtures/fixtures/more_files/peekaview.m" line-rate="0.0000000000000000" branch-rate="0.0000000000000000" complexity="0.0">
           <methods/>
           <lines>

spec/fixtures/fixtures.xcodeproj/project.pbxproj

@@ -10,6 +10,8 @@
 		155BBCA519E9CB6700BACB13 /* Branches.h in Headers */ = {isa = PBXBuildFile; fileRef = 155BBCA319E9CB6700BACB13 /* Branches.h */; };
 		155BBCA619E9CB6700BACB13 /* Branches.m in Sources */ = {isa = PBXBuildFile; fileRef = 155BBCA419E9CB6700BACB13 /* Branches.m */; };
 		155BBCA819E9CCC500BACB13 /* BranchesTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 155BBCA719E9CCC500BACB13 /* BranchesTests.m */; };
+		155D8AFE19FED984004666BA /* Empty.h in Headers */ = {isa = PBXBuildFile; fileRef = 155D8AFC19FED984004666BA /* Empty.h */; };
+		155D8AFF19FED984004666BA /* Empty.m in Sources */ = {isa = PBXBuildFile; fileRef = 155D8AFD19FED984004666BA /* Empty.m */; };
 		8C52AEF7195AAE32008A882A /* peekaview.h in Headers */ = {isa = PBXBuildFile; fileRef = 8C52AEF5195AAE32008A882A /* peekaview.h */; };
 		8C52AEF8195AAE33008A882A /* peekaview.m in Sources */ = {isa = PBXBuildFile; fileRef = 8C52AEF6195AAE32008A882A /* peekaview.m */; };
 		8C52AEFA195AAE70008A882A /* peekaviewTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 8C52AEF9195AAE70008A882A /* peekaviewTests.m */; };
@@ -36,6 +38,8 @@
 		155BBCA319E9CB6700BACB13 /* Branches.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Branches.h; sourceTree = "<group>"; };
 		155BBCA419E9CB6700BACB13 /* Branches.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = Branches.m; sourceTree = "<group>"; };
 		155BBCA719E9CCC500BACB13 /* BranchesTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BranchesTests.m; sourceTree = "<group>"; };
+		155D8AFC19FED984004666BA /* Empty.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = Empty.h; sourceTree = "<group>"; };
+		155D8AFD19FED984004666BA /* Empty.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = Empty.m; sourceTree = "<group>"; };
 		8C52AEF5195AAE32008A882A /* peekaview.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = peekaview.h; sourceTree = "<group>"; };
 		8C52AEF6195AAE32008A882A /* peekaview.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = peekaview.m; sourceTree = "<group>"; };
 		8C52AEF9195AAE70008A882A /* peekaviewTests.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = peekaviewTests.m; sourceTree = "<group>"; };
@@ -83,6 +87,8 @@
 				8C52AEF6195AAE32008A882A /* peekaview.m */,
 				155BBCA319E9CB6700BACB13 /* Branches.h */,
 				155BBCA419E9CB6700BACB13 /* Branches.m */,
+				155D8AFC19FED984004666BA /* Empty.h */,
+				155D8AFD19FED984004666BA /* Empty.m */,
 			);
 			path = more_files;
 			sourceTree = "<group>";
@@ -172,6 +178,7 @@
 			isa = PBXHeadersBuildPhase;
 			buildActionMask = 2147483647;
 			files = (
+				155D8AFE19FED984004666BA /* Empty.h in Headers */,
 				155BBCA519E9CB6700BACB13 /* Branches.h in Headers */,
 				8C52AEF7195AAE32008A882A /* peekaview.h in Headers */,
 			);
@@ -259,6 +266,7 @@
 			buildActionMask = 2147483647;
 			files = (
 				8C9A203B195965F10013B6B3 /* fixtures.m in Sources */,
+				155D8AFF19FED984004666BA /* Empty.m in Sources */,
 				155BBCA619E9CB6700BACB13 /* Branches.m in Sources */,
 				8C52AEF8195AAE33008A882A /* peekaview.m in Sources */,
 			);

spec/fixtures/fixtures/more_files/Empty.h

@@ -0,0 +1,13 @@
+//
+//  Empty.h
+//  fixtures
+//
+//  Created by Julian Krumow on 27.10.14.
+//  Copyright (c) 2014 marklarr. All rights reserved.
+//
+
+#import <Foundation/Foundation.h>
+
+@interface Empty : NSObject
+
+@end

spec/fixtures/fixtures/more_files/Empty.m

@@ -0,0 +1,13 @@
+//
+//  Empty.m
+//  fixtures
+//
+//  Created by Julian Krumow on 27.10.14.
+//  Copyright (c) 2014 marklarr. All rights reserved.
+//
+
+#import "Empty.h"
+
+@implementation Empty
+
+@end

spec/slather/coverage_file_spec.rb

@@ -266,4 +266,68 @@
     end
   end
 
+  describe "empty coverage data" do
+
+    let(:empty_file) do
+      fixtures_project.send(:coverage_files).detect { |cf| cf.source_file_pathname.basename.to_s == "Empty.m" }
+    end
+
+    describe "gcov_data" do
+      it "returns an empty string" do
+        gcov_data = empty_file.gcov_data
+        expect(gcov_data).to be_empty
+      end
+    end
+
+    describe "cleaned_gcov_data" do
+      it "returns an empty string" do
+        cleaned_gcov_data = empty_file.cleaned_gcov_data
+        expect(cleaned_gcov_data).to be_empty
+      end
+    end
+
+    describe "branch_coverage_data" do
+      it "returns an empty hash for branch_coverage_data of an empty file" do
+        data = empty_file.branch_coverage_data
+        expect(data).to be_empty
+      end
+    end
+
+    describe "num_branch_hits_for_statement_on_line" do
+      it "returns 0.0 when no data is available" do
+        expect(empty_file.num_branch_hits_for_statement_on_line(1)).to eq(0)
+      end
+    end
+
+    describe "rate_branch_coverage_for_statement_on_line" do
+      it "returns 0.0 when no data is available" do
+        expect(empty_file.rate_branch_coverage_for_statement_on_line(1)).to eq(0.0)
+      end
+    end
+
+    describe "percentage_branch_coverage_for_statement_on_line" do
+      it "returns 0 when no data is available" do
+        expect(empty_file.percentage_branch_coverage_for_statement_on_line(1)).to eq(0)
+      end
+    end
+
+    describe "num_branches_testable" do
+      it "returns 0 when no data is available" do
+        expect(empty_file.num_branches_testable).to eq(0)
+      end
+    end
+
+    describe "num_branches_tested" do
+      it "returns 0 when no data is available" do
+        expect(empty_file.num_branches_tested).to eq(0)
+      end
+    end
+
+    describe "rate_branches_tested" do
+      it "returns 0.0 when no data is available" do
+        expect(empty_file.rate_branches_tested).to eq(0.0)
+      end
+    end
+  end
+
 end