Merge remote-tracking branch 'upupstream/master' into feature/add_xcc…

…ov_support

* upupstream/master: (24 commits)
  Explicitly install bundler 1
  Updated changelog
  Updated changelog
  Instead of looking at the ARG_MAX size let the exception occur and catch it. This circumvents guessing the usable size of ARG_MAX and will also work on systems where it's not possible to query 'getconf ARG_MAX'. I feel much better about this as it leaves the original flow intact and will only split the work into two if exactly Errno::E2BIG is raised. This should work across Unix/Win/MacOS.
  Remove redundant > 0 condition and clarify error message.
  Secured pathnames_per_binary split against empty results
  Add tests for implementation
  Fixed warnings re: dependency versions:
  Much clearer recursive solution
  Nicer iteration for chunk size
  Refactored the implementation to prevent failure if the argument size for to llvm-cov exceeds ARG_MAX
  Fixed CFPropertyList to be less restrictive. Slather doesn't have an absolute dependency on 3.0, so making it >= 2.2.
  Updated changelog and bumped version number.
  Updates slather dependencies, including dev and runtime. Verified that all unit tests pass after update.
  Update project.rb
  Slather 2.4.6
  Updated changelog
  Issue SlatherOrg#261 - Fixed an issue that was causing not all files to be listed when using --html export option.
  Updated changelog
  Fix existing tests
  ...

.travis.yml

@@ -4,7 +4,7 @@ osx_image: xcode9.2
 
 before_install:
   - curl http://curl.haxx.se/ca/cacert.pem -o /usr/local/share/cacert.pem
-  - gem install bundler --no-ri --no-rdoc
+  - gem install bundler -v "~> 1.0" --no-ri --no-rdoc
 
 install:
   - bundle install --without=documentation

CHANGELOG.md

@@ -1,14 +1,31 @@
 # CHANGELOG
 
-## master
+## v2.4.7
+
+* Update dependencies
+  [dnedrow](https://github.com/dnedrow)
+
+* Fixed errors when llvm-cov argument length exceeds ARG_MAX
+  [weibel](https://github.com/weibel)
+  [#414](https://github.com/SlatherOrg/slather/pull/414)
+
+* Show "No coverage directory found." instead of "implicit conversion nil into String"
+  [phimage](https://github.com/phimage)
+  [#381](https://github.com/SlatherOrg/slather/pull/381) [#341](https://github.com/SlatherOrg/slather/issues/341)
+
+## v2.4.6
 
 * Fix .dSYM and .swiftmodule files filtering in find_binary_files()
   [krin-san](https://github.com/krin-san)
   [#368](https://github.com/SlatherOrg/slather/pull/368)
 
 * Fixed loading coverage for a single source file
   [blackm00n](https://github.com/blackm00n)
-  [#377](https://github.com/SlatherOrg/slather/pull/377)
+  [#377](https://github.com/SlatherOrg/slather/pull/377) [#398](https://github.com/SlatherOrg/slather/pull/398)
+
+* Fixed truncated file list in HTML export
+  [miroslavkovac](https://github.com/miroslavkovac)
+  [#402](https://github.com/SlatherOrg/slather/pull/402) [#261](https://github.com/SlatherOrg/slather/issues/261)
 
 ## v2.4.5
 

lib/slather/project.rb

@@ -161,31 +161,67 @@ def xccov_coverage_files
 
     def profdata_coverage_files
       coverage_files = []
-      source_files = find_source_files || []
-      line_numbers_first = Gem::Version.new(self.llvm_version) >= Gem::Version.new('8.1.0')
 
       if self.binary_file
         self.binary_file.each do |binary_path|
-          files = profdata_llvm_cov_output(binary_path, source_files).split("\n\n")
-
-          coverage_files.concat(files.map do |source|
-            coverage_file = coverage_file_class.new(self, source, line_numbers_first)
-            # If a single source file is used, the resulting output does not contain the file name.
-            coverage_file.source_file_pathname = source_files.first if source_files.count == 1
-            !coverage_file.ignored? ? coverage_file : nil
-          end.compact)
-
-          if !source_files.empty?
-            coverage_file_paths = coverage_files.map { |file| file.source_file_pathname }.to_set
-            source_files.select! { |path| !coverage_file_paths.include?(path) }
-          end
+          pathnames_per_binary = pathnames_per_binary(binary_path)
+          coverage_files.concat(create_coverage_files_for_binary(binary_path, pathnames_per_binary))
         end
       end
 
       coverage_files
     end
     private :profdata_coverage_files
 
+    def pathnames_per_binary(binary_path)
+      coverage_json_string = llvm_cov_export_output(binary_path)
+      coverage_json = JSON.parse(coverage_json_string)
+      coverage_json["data"].reduce([]) do |result, chunk|
+        result.concat(chunk["files"].map do |file|
+          Pathname(file["filename"]).realpath
+        end)
+      end
+    end
+    private :pathnames_per_binary
+
+    def create_coverage_files_for_binary(binary_path, pathnames_per_binary)
+      coverage_files = []
+
+      begin
+        coverage_files.concat(create_coverage_files(binary_path, pathnames_per_binary))
+      rescue Errno::E2BIG => e
+        # pathnames_per_binary is too big for the OS to handle so it's split in two halfs which are processed independently
+        if pathnames_per_binary.count > 1
+          left, right = pathnames_per_binary.each_slice( (pathnames_per_binary.size/2.0).round ).to_a
+          coverage_files.concat(create_coverage_files_for_binary(binary_path, left))
+          coverage_files.concat(create_coverage_files_for_binary(binary_path, right))
+        else
+          # pathnames_per_binary contains one element which is too big for the OS to handle. 
+          raise e, "#{e}. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414", e.backtrace
+        end
+      end
+
+      coverage_files
+    end
+    private :create_coverage_files_for_binary
+
+    def create_coverage_files(binary_path, pathnames)
+      line_numbers_first = Gem::Version.new(self.llvm_version) >= Gem::Version.new('8.1.0')
+      files = create_profdata(binary_path, pathnames)
+      files.map do |source|
+        coverage_file = coverage_file_class.new(self, source, line_numbers_first)
+        # If a single source file is used, the resulting output does not contain the file name.
+        coverage_file.source_file_pathname = pathnames.first if pathnames.count == 1
+        !coverage_file.ignored? ? coverage_file : nil
+      end.compact
+    end
+    private :create_coverage_files
+
+    def create_profdata(binary_path, pathnames)
+      profdata_llvm_cov_output(binary_path, pathnames).split("\n\n")
+    end
+    private :create_profdata
+
     def remove_extension(path)
       path.split(".")[0..-2].join(".")
     end
@@ -221,7 +257,7 @@ def profdata_coverage_dir
           coverage_files = Dir[File.join(build_directory, "../**/ProfileData/*/Coverage.profdata")]
         end
 
-        if coverage_files != nil
+        if coverage_files != nil && coverage_files.count != 0
           dir = Pathname.new(coverage_files.first).parent()
         end
       end
@@ -244,6 +280,30 @@ def profdata_file
     end
     private :profdata_file
 
+    def unsafe_llvm_cov_export_output(binary_path)
+      profdata_file_arg = profdata_file
+      if profdata_file_arg == nil
+        raise StandardError, "No Coverage.profdata files found. Please make sure the \"Code Coverage\" checkbox is enabled in your scheme's Test action or the build_directory property is set."
+      end
+
+      if binary_path == nil
+        raise StandardError, "No binary file found."
+      end
+
+      llvm_cov_args = %W(export -instr-profile #{profdata_file_arg} #{binary_path})
+      if self.arch
+        llvm_cov_args << "--arch" << self.arch
+      end
+      `xcrun llvm-cov #{llvm_cov_args.shelljoin}`
+    end
+    private :unsafe_llvm_cov_export_output
+
+    def llvm_cov_export_output(binary_path)
+      output = unsafe_llvm_cov_export_output(binary_path)
+      output.valid_encoding? ? output : output.encode!('UTF-8', 'binary', :invalid => :replace, undef: :replace)
+    end
+    private :llvm_cov_export_output
+
     def unsafe_profdata_llvm_cov_output(binary_path, source_files)
       profdata_file_arg = profdata_file
       if profdata_file_arg == nil

lib/slather/version.rb

@@ -1,3 +1,3 @@
 module Slather
-  VERSION = '2.4.6' unless defined?(Slather::VERSION)
+  VERSION = '2.4.7' unless defined?(Slather::VERSION)
 end

slather.gemspec

@@ -17,20 +17,20 @@ Gem::Specification.new do |spec|
   spec.test_files    = spec.files.grep(%r{^(test|spec|features)/})
   spec.require_paths = ['lib']
 
-  spec.add_development_dependency 'bundler', '~> 1.6'
-  spec.add_development_dependency 'coveralls'
-  spec.add_development_dependency 'simplecov'
-  spec.add_development_dependency 'rake', '~> 10.4'
-  spec.add_development_dependency 'rspec', '~> 3.4'
-  spec.add_development_dependency 'pry', '~> 0.9'
-  spec.add_development_dependency 'cocoapods', '~> 1.2'
-  spec.add_development_dependency 'json_spec', '~> 1.1.4'
-  spec.add_development_dependency 'equivalent-xml', '~> 0.5.1'
+  spec.add_development_dependency 'bundler', '~> 1.17'
+  spec.add_development_dependency 'coveralls', '~> 0'
+  spec.add_development_dependency 'simplecov', '~> 0'
+  spec.add_development_dependency 'rake', '~> 12.3'
+  spec.add_development_dependency 'rspec', '~> 3.8'
+  spec.add_development_dependency 'pry', '~> 0.12'
+  spec.add_development_dependency 'cocoapods', '~> 1.5'
+  spec.add_development_dependency 'json_spec', '~> 1.1'
+  spec.add_development_dependency 'equivalent-xml', '~> 0.6'
 
-  spec.add_dependency 'clamp', '~> 0.6'
-  spec.add_dependency 'xcodeproj', '~> 1.4'
-  spec.add_dependency 'nokogiri', '~> 1.8.2'
-  spec.add_dependency 'CFPropertyList', '>= 2.2'
+  spec.add_dependency 'clamp', '~> 1.3'
+  spec.add_dependency 'xcodeproj', '~> 1.7'
+  spec.add_dependency 'nokogiri', '~> 1.8'
+  spec.add_dependency 'CFPropertyList', '>= 2.2', '< 4'
 
-  spec.add_runtime_dependency 'activesupport', '>= 4.0.2'
+  spec.add_runtime_dependency 'activesupport', '< 5', '>= 4.0.2'
 end

spec/slather/coverage_service/coveralls_spec.rb

@@ -158,6 +158,15 @@
       before(:each) {
         fixtures_project.ci_service = :travis_ci
         project_root = Pathname("./").realpath
+        allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(%q(
+          {
+             "data":[
+                {
+                   "files":[]
+                }
+             ]
+          }
+        ))
         allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return("#{project_root}/spec/fixtures/fixtures/fixtures.m:
        |    1|//
        |    2|//  fixtures.m

spec/slather/coverage_service/hardcover_spec.rb

@@ -66,6 +66,15 @@
       allow(Slather::Project).to receive(:yml).and_return(fixture_yaml)
       fixtures_project.ci_service = :jenkins_ci
       project_root = Pathname("./").realpath
+      allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(%q(
+        {
+           "data":[
+              {
+                 "files":[]
+              }
+           ]
+        }
+      ))
       allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return("#{project_root}/spec/fixtures/fixtures/fixtures.m:
        |    1|//
        |    2|//  fixtures.m
@@ -104,7 +113,6 @@
 
     it "should always remove the hardcover_json_file after it's done" do
       allow(fixtures_project).to receive(:`)
-
       allow(fixtures_project).to receive(:jenkins_job_id).and_return("slather-master/9182")
       allow(fixtures_project).to receive(:coverage_service_url).and_return("http://api.hardcover.io")
       fixtures_project.post

spec/slather/project_spec.rb

@@ -64,15 +64,91 @@ class SpecCoverageFile < Slather::CoverageFile
     end
   end
 
-  describe "#profdata_coverage_files" do
+  describe "#profdata_coverage_files with large file lists" do
     class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
     end
 
+    llvm_cov_export_output = %q(
+        {
+           "data":[
+              {
+                 "files":[
+                    {
+                       "filename":"spec/fixtures/fixtures/Fixtures.swift"
+                    }
+                 ]
+              },
+              {
+                 "files":[
+                    {
+                       "filename":"spec/fixtures/fixtures/Fixtures.swift"
+                    }
+                 ]
+              }
+           ]
+        })
+
+    profdata_llvm_cov_output = "#{FIXTURES_SWIFT_FILE_PATH}:
+       |    0|
+       |    1|import UIKit
+       |    2|
+       |    3|@UIApplicationMain
+       |    4|class AppDelegate: UIResponder, UIApplicationDelegate {
+       |    5|
+       |    6|    var window: UIWindow?
+       |    7|
+      1|    8|    func application(application: UIApplication, didFinishLaunchingWithOptions launchOptions: [NSObject: AnyObject]?) -> Bool {
+      1|    9|        return true
+      1|   10|    }
+       |   11|
+      0|   12|    func applicationWillResignActive(application: UIApplication) {
+      0|   13|    }
+      0|   14|}"
+
     before(:each) do
       allow(Dir).to receive(:[]).and_call_original
       allow(Dir).to receive(:[]).with("#{fixtures_project.build_directory}/**/Coverage.profdata").and_return(["/some/path/Coverage.profdata"])
       allow(fixtures_project).to receive(:binary_file).and_return(["Fixtures"])
-      allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return("#{FIXTURES_SWIFT_FILE_PATH}:
+      allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(llvm_cov_export_output)
+      allow(fixtures_project).to receive(:coverage_file_class).and_return(SpecXcode7CoverageFile)
+      allow(fixtures_project).to receive(:ignore_list).and_return([])
+    end
+
+    it "Should catch Errno::E2BIG and re-raise if the input is too large to split into multiple chunks" do
+      allow(fixtures_project).to receive(:unsafe_profdata_llvm_cov_output).and_raise(Errno::E2BIG)
+      expect { fixtures_project.send(:profdata_coverage_files) }.to raise_error(Errno::E2BIG, "Argument list too long. A path in your project is close to the E2BIG limit. https://github.com/SlatherOrg/slather/pull/414")
+    end
+
+    it "Should catch Errno::E2BIG and return Coverage.profdata file objects when the work can be split into two" do
+      allow(fixtures_project).to receive(:unsafe_profdata_llvm_cov_output).once { 
+        # raise once and then insert the stub
+        allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return(profdata_llvm_cov_output)
+        raise Errno::E2BIG 
+      }
+      profdata_coverage_files = fixtures_project.send(:profdata_coverage_files)
+      profdata_coverage_files.each { |cf| expect(cf.kind_of?(SpecXcode7CoverageFile)).to be_truthy }
+      expect(profdata_coverage_files.map { |cf| cf.source_file_pathname.basename.to_s }).to eq(["Fixtures.swift", "Fixtures.swift"])
+    end
+  end
+
+  describe "#profdata_coverage_files" do
+    class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
+    end
+
+    llvm_cov_export_output = %q(
+        {
+           "data":[
+              {
+                 "files":[
+                    {
+                       "filename":"spec/fixtures/fixtures/Fixtures.swift"
+                    }
+                 ]
+              }
+           ]
+        })
+
+    profdata_llvm_cov_output = "#{FIXTURES_SWIFT_FILE_PATH}:
        |    0|
        |    1|import UIKit
        |    2|
@@ -87,7 +163,14 @@ class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
        |   11|
       0|   12|    func applicationWillResignActive(application: UIApplication) {
       0|   13|    }
-      0|   14|}")
+      0|   14|}"
+
+    before(:each) do
+      allow(Dir).to receive(:[]).and_call_original
+      allow(Dir).to receive(:[]).with("#{fixtures_project.build_directory}/**/Coverage.profdata").and_return(["/some/path/Coverage.profdata"])
+      allow(fixtures_project).to receive(:binary_file).and_return(["Fixtures"])
+      allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(llvm_cov_export_output)
+      allow(fixtures_project).to receive(:profdata_llvm_cov_output).and_return(profdata_llvm_cov_output)
       allow(fixtures_project).to receive(:coverage_file_class).and_return(SpecXcode7CoverageFile)
       allow(fixtures_project).to receive(:ignore_list).and_return([])
     end
@@ -111,6 +194,18 @@ class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
       allow(fixtures_project).to receive(:ignore_list).and_return([])
       allow(Dir).to receive(:[]).with("#{fixtures_project.build_directory}/**/Coverage.profdata").and_return(["/some/path/Coverage.profdata"])
       allow(fixtures_project).to receive(:binary_file).and_return(["Fixtures"])
+      allow(fixtures_project).to receive(:unsafe_llvm_cov_export_output).and_return("
+      {
+         \"data\":[
+            {
+               \"files\":[
+                  {
+                     \"filename\":\"sp\145c/fixtures/fixtures/fixtures.m\"
+                  }
+               ]
+            }
+         ]
+      }")
       allow(fixtures_project).to receive(:unsafe_profdata_llvm_cov_output).and_return("#{FIXTURES_SWIFT_FILE_PATH}:
       1|    8|    func application(application: \255, didFinishLaunchingWithOptions launchOptions: [NSObject: AnyObject]?) -> Bool {
       1|    9|        return true
@@ -563,6 +658,22 @@ class SpecXcode7CoverageFile < Slather::ProfdataCoverageFile
     end
 
     it "should print out the coverage for each file, and then total coverage" do
+      allow(fixtures_project).to receive(:llvm_cov_export_output).and_return(%q(
+        {
+           "data":[
+              {
+                 "files":[
+                    {
+                       "filename":"spec/fixtures/fixtures/fixtures.m"
+                    },
+                    {
+                       "filename":"spec/fixtures/fixturesTwo/fixturesTwo.m"
+                    }
+                 ]
+              }
+           ]
+        }
+      ))
       ["spec/fixtures/fixtures/fixtures.m: 3 of 6 lines (50.00%)",
       "spec/fixtures/fixturesTwo/fixturesTwo.m: 6 of 6 lines (100.00%)",
       "Tested 9/12 statements",