Removes duplicate definition of skip? method

This fixes a bug where issues were being reported when the thresholds
were not being met, due to a second definition of `skip?` replacing the
correct one.

This also improves the specs to properly cover the behavior.

Author: Chris Hulton

lib/cc/engine/analyzers/reporter.rb

@@ -152,14 +152,6 @@ def insufficient_occurrence?(violation)
           (violation.occurrences + 1) < language_strategy.count_threshold
         end
 
-        def skip?(violation)
-          insufficient_occurrence?(violation) || check_disabled?(violation)
-        end
-
-        def insufficient_occurrence?(violation)
-          (violation.occurrences + 1) < language_strategy.count_threshold
-        end
-
         def check_disabled?(violation)
           !engine_config.check_enabled?(
             violation.fingerprint_check_name,

spec/cc/engine/analyzers/ruby/main_spec.rb

@@ -187,44 +187,93 @@ def self.from_remediation_amount(amount)
         expect(issues.length).to eq(0)
       end
 
-      it "respects the per-check mass thresholds" do
-        create_source_file("foo.rb", <<-EORUBY)
-          def identical
-            puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
-            puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
-            puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
-          end
-          describe 'similar1' do
-            before { subject.type = 'js' }
-            it 'returns true' do
-              expect(subject.ruby?).to be true
+      context "per-check mass thresholds" do
+        before do
+          create_source_file("foo.rb", <<-EORUBY)
+            # identical mass of 15
+            def identical
+              puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
+              puts "identical \#{thing.bar} \#{other.fun} \#{moo ? "moo" : "cluck"}"
             end
-          end
-          describe 'similar2' do
-            before { subject.type = 'js' }
-            it 'returns true' do
-              expect(subject.js?).to be true
+
+            # similar mass of 10
+            def similar1
+              foo.select { |f| if f.baz?; 10; end }
             end
+
+            def similar2
+              foo.select { |f| if f.bar?; 15; end }
+            end
+          EORUBY
+        end
+
+        it "reports identical issues when only that threshold exceeded" do
+          config = CC::Engine::Analyzers::EngineConfig.new({
+            "config" => {
+              "languages" => %w[ruby],
+              "checks" => {
+                "identical-code" => { "config" => { "threshold" => 5 } },
+                "similar-code" => { "config" => { "threshold" => 20 } },
+              },
+            },
+          })
+
+          issues = run_engine(config).strip.split("\0")
+          expect(issues.length).to eq(2)
+
+          issues.each do |issue|
+            expect(JSON.parse(issue)["check_name"]).to eq("identical-code")
           end
-        EORUBY
+        end
 
-        config = CC::Engine::Analyzers::EngineConfig.new({
-          "config" => {
-            "languages" => %w[ruby],
-            "checks" => {
-              "identical-code" => { "config" => { "threshold" => 5 } },
-              "similar-code" => { "config" => { "threshold" => 20 } },
+        it "reports similar issues when only that threshold exceeded" do
+          config = CC::Engine::Analyzers::EngineConfig.new({
+            "config" => {
+              "languages" => %w[ruby],
+              "checks" => {
+                "identical-code" => { "config" => { "threshold" => 20 } },
+                "similar-code" => { "config" => { "threshold" => 5 } },
+              },
             },
-          },
-        })
-        output = run_engine(config).strip.split("\0").first.strip
-        json = JSON.parse(output)
+          })
 
-        expect(json["check_name"]).to eq "identical-code"
-        expect(json["location"]).to eq({
-          "path" => "foo.rb",
-          "lines" => { "begin" => 2, "end" => 2 },
-        })
+          issues = run_engine(config).strip.split("\0")
+          expect(issues.length).to eq(2)
+
+          issues.each do |issue|
+            expect(JSON.parse(issue)["check_name"]).to eq("similar-code")
+          end
+        end
+
+        it "reports similar and identical issues when both thresholds exceeded" do
+          config = CC::Engine::Analyzers::EngineConfig.new({
+            "config" => {
+              "languages" => %w[ruby],
+              "checks" => {
+                "identical-code" => { "config" => { "threshold" => 5 } },
+                "similar-code" => { "config" => { "threshold" => 5 } },
+              },
+            },
+          })
+
+          issues = run_engine(config).strip.split("\0")
+          expect(issues.length).to eq(4)
+        end
+
+        it "reports no issues when neither threshold exceeded" do
+          config = CC::Engine::Analyzers::EngineConfig.new({
+            "config" => {
+              "languages" => %w[ruby],
+              "checks" => {
+                "identical-code" => { "config" => { "threshold" => 20 } },
+                "similar-code" => { "config" => { "threshold" => 20 } },
+              },
+            },
+          })
+
+          issues = run_engine(config).strip.split("\0")
+          expect(issues.length).to eq(0)
+        end
       end
     end