Merge pull request #52 from codeclimate/force_blob_id_encoding

Force UTF-8 to calculate blob ids.

Author: calavera

.travis.yml

@@ -1,4 +1,7 @@
 language: ruby
+before_script:
+  - git config --global user.email "[email protected]"
+  - git config --global user.name "Code Climate CI"
 rvm:
   - 1.9.3
   - 2.1.2

lib/code_climate/test_reporter/ci.rb

@@ -2,65 +2,65 @@ module CodeClimate
   module TestReporter
     class Ci
 
-      def self.service_data
-        if ENV['TRAVIS']
+      def self.service_data(env = ENV)
+        if env['TRAVIS']
           {
             name:             "travis-ci",
-            branch:           ENV['TRAVIS_BRANCH'],
-            build_identifier: ENV['TRAVIS_JOB_ID'],
-            pull_request:     ENV['TRAVIS_PULL_REQUEST']
+            branch:           env['TRAVIS_BRANCH'],
+            build_identifier: env['TRAVIS_JOB_ID'],
+            pull_request:     env['TRAVIS_PULL_REQUEST']
           }
-        elsif ENV['CIRCLECI']
+        elsif env['CIRCLECI']
           {
             name:             "circlci",
-            build_identifier: ENV['CIRCLE_BUILD_NUM'],
-            branch:           ENV['CIRCLE_BRANCH'],
-            commit_sha:       ENV['CIRCLE_SHA1']
+            build_identifier: env['CIRCLE_BUILD_NUM'],
+            branch:           env['CIRCLE_BRANCH'],
+            commit_sha:       env['CIRCLE_SHA1']
           }
-        elsif ENV['SEMAPHORE']
+        elsif env['SEMAPHORE']
           {
             name:             "semaphore",
-            branch:           ENV['BRANCH_NAME'],
-            build_identifier: ENV['SEMAPHORE_BUILD_NUMBER']
+            branch:           env['BRANCH_NAME'],
+            build_identifier: env['SEMAPHORE_BUILD_NUMBER']
           }
-        elsif ENV['JENKINS_URL']
+        elsif env['JENKINS_URL']
           {
             name:             "jenkins",
-            build_identifier: ENV['BUILD_NUMBER'],
-            build_url:        ENV['BUILD_URL'],
-            branch:           ENV['GIT_BRANCH'],
-            commit_sha:       ENV['GIT_COMMIT']
+            build_identifier: env['BUILD_NUMBER'],
+            build_url:        env['BUILD_URL'],
+            branch:           env['GIT_BRANCH'],
+            commit_sha:       env['GIT_COMMIT']
           }
-        elsif ENV['TDDIUM']
+        elsif env['TDDIUM']
           {
             name:             "tddium",
-            build_identifier: ENV['TDDIUM_SESSION_ID'],
-            worker_id:        ENV['TDDIUM_TID']
+            build_identifier: env['TDDIUM_SESSION_ID'],
+            worker_id:        env['TDDIUM_TID']
           }
-        elsif ENV['WERCKER']
+        elsif env['WERCKER']
           {
             name:             "wercker",
-            build_identifier: ENV['WERCKER_BUILD_ID'],
-            build_url:        ENV['WERCKER_BUILD_URL'],
-            branch:           ENV['WERCKER_GIT_BRANCH'],
-            commit_sha:       ENV['WERCKER_GIT_COMMIT']
+            build_identifier: env['WERCKER_BUILD_ID'],
+            build_url:        env['WERCKER_BUILD_URL'],
+            branch:           env['WERCKER_GIT_BRANCH'],
+            commit_sha:       env['WERCKER_GIT_COMMIT']
           }
-        elsif ENV['CI_NAME'] =~ /DRONE/i
+        elsif env['CI_NAME'] =~ /DRONE/i
           {
             name:             "drone",
-            build_identifier: ENV['CI_BUILD_NUMBER'],
-            build_url:        ENV['CI_BUILD_URL'],
-            branch:           ENV['CI_BRANCH'],
-            commit_sha:       ENV['CI_BUILD_NUMBER'],
-            pull_request:     ENV['CI_PULL_REQUEST']
+            build_identifier: env['CI_BUILD_NUMBER'],
+            build_url:        env['CI_BUILD_URL'],
+            branch:           env['CI_BRANCH'],
+            commit_sha:       env['CI_BUILD_NUMBER'],
+            pull_request:     env['CI_PULL_REQUEST']
           }
-        elsif ENV['CI_NAME'] =~ /codeship/i
+        elsif env['CI_NAME'] =~ /codeship/i
           {
             name:             "codeship",
-            build_identifier: ENV['CI_BUILD_NUMBER'],
-            build_url:        ENV['CI_BUILD_URL'],
-            branch:           ENV['CI_BRANCH'],
-            commit_sha:       ENV['CI_COMMIT_ID'],
+            build_identifier: env['CI_BUILD_NUMBER'],
+            build_url:        env['CI_BUILD_URL'],
+            branch:           env['CI_BRANCH'],
+            commit_sha:       env['CI_COMMIT_ID'],
           }
         else
           {}

lib/code_climate/test_reporter/formatter.rb

@@ -1,3 +1,5 @@
+# encoding: utf-8
+
 require "tmpdir"
 require "securerandom"
 require "json"
@@ -80,10 +82,13 @@ def to_payload(result)
       end
 
       def calculate_blob_id(path)
-        content = File.open(path, "rb") {|f| f.read }
-        header = "blob #{content.length}\0"
-        store = header + content
-        Digest::SHA1.hexdigest(store)
+        File.open(path, "rb") do |file|
+          header  = "blob #{file.size}\0"
+          content = file.read.force_encoding("iso-8859-1").encode("utf-8", replace: nil)
+          store   = header + content
+
+          return Digest::SHA1.hexdigest(store)
+        end
       end
 
       def short_filename(filename)

lib/code_climate/test_reporter/git.rb

@@ -12,16 +12,21 @@ def info
         end
 
         def branch_from_git_or_ci
-          git_branch = branch_from_git
-          ci_branch = Ci.service_data[:branch]
-
-          if ci_branch.to_s.strip.size > 0
-            ci_branch.sub(/^origin\//, "")
-          elsif git_branch.to_s.strip.size > 0 && !git_branch.to_s.strip.start_with?("(")
-            git_branch.sub(/^origin\//, "")
-          else
-            "master"
-          end
+          clean_service_branch || clean_git_branch || "master"
+        end
+
+        def clean_service_branch
+          ci_branch = String(Ci.service_data[:branch])
+          clean = ci_branch.strip.sub(/^origin\//, "")
+
+          clean.size > 0 ? clean : nil
+        end
+
+        def clean_git_branch
+          git_branch = String(branch_from_git)
+          clean = git_branch.sub(/^origin\//, "") unless git_branch.start_with?("(")
+
+          clean.size > 0 ? clean : nil
         end
 
         private
@@ -32,9 +37,7 @@ def committed_at
         end
 
         def branch_from_git
-          branch = `git branch`.split("\n").delete_if { |i| i[0] != "*" }
-          branch = [branch].flatten.first
-          branch ? branch.gsub("* ","") : nil
+          `git rev-parse --abbrev-ref HEAD`.chomp
         end
       end
     end

spec/fixtures/encoding_test.rb

@@ -0,0 +1,6 @@
+# encoding: utf-8
+class EncodingTest
+  def foo
+    "ä"
+  end
+end

spec/fixtures/encoding_test_iso.rb

@@ -0,0 +1,6 @@
+# encoding: iso-8859-1
+class EncodingTest
+  def foo
+    "ä"
+  end
+end

spec/lib/ci_spec.rb

@@ -5,15 +5,11 @@ module CodeClimate::TestReporter
 
     describe '.service_data' do
       before :each do
-        ENV['SEMAPHORE'] = 'yes?'
-        ENV['BRANCH_NAME'] = 'master'
-        ENV['SEMAPHORE_BUILD_NUMBER'] = '1234'
-      end
-
-      after :each do
-        ENV.delete('SEMAPHORE')
-        ENV.delete('BRANCH_NAME')
-        ENV.delete('SEMAPHORE_BUILD_NUMBER')
+        @env = {
+          'SEMAPHORE' => 'yes?',
+          'BRANCH_NAME' => 'master',
+          'SEMAPHORE_BUILD_NUMBER' => '1234'
+        }
       end
 
       it 'returns a hash of CI environment info' do
@@ -23,9 +19,8 @@ module CodeClimate::TestReporter
           build_identifier: '1234'
         }
 
-        expect(Ci.service_data).to include expected_semaphore_hash
+        expect(Ci.service_data(@env)).to include expected_semaphore_hash
       end
     end
-
   end
 end

spec/lib/formatter_spec.rb

@@ -70,7 +70,6 @@ module CodeClimate::TestReporter
             "simplecov_root" => Dir.pwd,
             "gem_version" => VERSION
           },
-        "ci_service" => {}
       }
     }
 
@@ -106,34 +105,46 @@ module CodeClimate::TestReporter
 
       uncompressed = inflate(app.request_body)
 
-      expect(JSON.parse(uncompressed)).to eq(expected_request)
+      expected_request.merge!("ci_service" => Ci.service_data)
+      expected_json = JSON.parse(expected_request.to_json, symbolize_names: true)
+
+      expect(JSON.parse(uncompressed, symbolize_names: true)).to eq(expected_json)
       expect(app.http_user_agent).to include("v#{CodeClimate::TestReporter::VERSION}")
     end
-  end
 
-  describe '#short_filename' do
-    let(:formatter) { CodeClimate::TestReporter::Formatter.new }
-    it 'should return the filename of the file relative to the SimpleCov root' do
-      expect(formatter.short_filename('file1')).to eq('file1')
-      expect(formatter.short_filename("#{::SimpleCov.root}/file1")).to eq('file1')
-    end
+    describe '#short_filename' do
+      it 'should return the filename of the file relative to the SimpleCov root' do
+        expect(formatter.short_filename('file1')).to eq('file1')
+        expect(formatter.short_filename("#{::SimpleCov.root}/file1")).to eq('file1')
+      end
 
-    context "with path prefix" do
-      before do
-        CodeClimate::TestReporter.configure do |config|
-          config.path_prefix = 'custom'
+      context "with path prefix" do
+        before do
+          CodeClimate::TestReporter.configure do |config|
+            config.path_prefix = 'custom'
+          end
         end
-      end
 
-      after do
-        CodeClimate::TestReporter.configure do |config|
-          config.path_prefix = nil
+        after do
+          CodeClimate::TestReporter.configure do |config|
+            config.path_prefix = nil
+          end
+        end
+
+        it 'should include the path prefix if set' do
+          expect(formatter.short_filename('file1')).to eq('custom/file1')
+          expect(formatter.short_filename("#{::SimpleCov.root}/file1")).to eq('custom/file1')
         end
       end
+    end
+
+    describe "#calculate_blob_id" do
+      it "forces UTF-8 as encoding for the file content" do
+        blob_id = formatter.calculate_blob_id(File.expand_path("../../fixtures/encoding_test_iso.rb", __FILE__))
+        expect(blob_id).to_not be_nil
 
-      it 'should include the path prefix if set' do
-        expect(formatter.short_filename('file1')).to eq('custom/file1')
-        expect(formatter.short_filename("#{::SimpleCov.root}/file1")).to eq('custom/file1')
+        blob_id = formatter.calculate_blob_id(File.expand_path("../../fixtures/encoding_test.rb", __FILE__))
+        expect(blob_id).to_not be_nil
       end
     end
   end

spec/lib/git_spec.rb

@@ -24,7 +24,7 @@ module CodeClimate::TestReporter
       it 'returns the branch from git if there is no ci branch' do
         allow(Ci).to receive(:service_data).and_return({})
 
-        expect(Git.branch_from_git_or_ci).to eq Git.send(:branch_from_git)
+        expect(Git.branch_from_git_or_ci).to eq Git.clean_git_branch
       end
 
       it 'returns master otherwise' do