Skip to content

Tune RUBY remediation points to match Classic #67

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 30, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/cc/engine/analyzers/analyzer_base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ def mass_threshold
engine_config.mass_threshold_for(self.class::LANGUAGE) || self.class::DEFAULT_MASS_THRESHOLD
end

def base_points
self.class::BASE_POINTS
def calculate_points(issue)
self.class::BASE_POINTS * issue.mass
end

private
Expand Down
8 changes: 2 additions & 6 deletions lib/cc/engine/analyzers/reporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,15 @@ def flay

attr_reader :engine_config, :language_strategy, :io

def mass_threshold
@mass_threshold ||= language_strategy.mass_threshold
end

def new_violation(issue)
hashes = flay.hashes[issue.structural_hash]
Violation.new(language_strategy.base_points, issue, hashes)
Violation.new(language_strategy, issue, hashes)
end

def flay_options
{
diff: false,
mass: mass_threshold,
mass: language_strategy.mass_threshold,
summary: false,
verbose: false,
number: true,
Expand Down
11 changes: 10 additions & 1 deletion lib/cc/engine/analyzers/ruby/main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,20 @@ class Main < CC::Engine::Analyzers::Base

]
DEFAULT_MASS_THRESHOLD = 18
BASE_POINTS = 10_000
BASE_POINTS = 1_500_000
POINTS_PER_OVERAGE = 100_000
TIMEOUT = 300

def calculate_points(issue)
BASE_POINTS + (overage(issue) * POINTS_PER_OVERAGE)
end

private

def overage(issue)
issue.mass - mass_threshold
end

def process_file(file)
RubyParser.new.process(File.binread(file), file, TIMEOUT)
end
Expand Down
8 changes: 4 additions & 4 deletions lib/cc/engine/analyzers/violation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ module Analyzers
class Violation
attr_reader :issue

def initialize(base_points, issue, hashes)
@base_points = base_points
def initialize(language_strategy, issue, hashes)
@language_strategy = language_strategy
@issue = issue
@hashes = hashes
end
Expand All @@ -34,7 +34,7 @@ def report_name

private

attr_reader :base_points, :hashes
attr_reader :language_strategy, :hashes

def current_sexp
@location ||= sorted_hashes.first
Expand All @@ -57,7 +57,7 @@ def name
end

def calculate_points
base_points * issue.mass
language_strategy.calculate_points(issue)
end

def format_location
Expand Down
133 changes: 89 additions & 44 deletions spec/cc/engine/analyzers/ruby/main_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,60 +4,105 @@
require 'flay'
require 'tmpdir'

RSpec.describe CC::Engine::Analyzers::Ruby::Main, in_tmpdir: true do
include AnalyzerSpecHelpers
module CC::Engine::Analyzers
RSpec.describe Ruby::Main, in_tmpdir: true do
include AnalyzerSpecHelpers

describe "#run" do
it "prints an issue" do
create_source_file("foo.rb", <<-EORUBY)
describe '#ruby?' do
before { subject.type = 'ruby' }
describe "#run" do
it "prints an issue" do
create_source_file("foo.rb", <<-EORUBY)
describe '#ruby?' do
before { subject.type = 'ruby' }

it 'returns true' do
expect(subject.ruby?).to be true
it 'returns true' do
expect(subject.ruby?).to be true
end
end
end

describe '#js?' do
before { subject.type = 'js' }
describe '#js?' do
before { subject.type = 'js' }

it 'returns true' do
expect(subject.js?).to be true
it 'returns true' do
expect(subject.js?).to be true
end
end
end
EORUBY

result = run_engine(engine_conf).strip
json = JSON.parse(result)

expect(json["type"]).to eq("issue")
expect(json["check_name"]).to eq("Similar code")
expect(json["description"]).to eq("Similar code found in 1 other location")
expect(json["categories"]).to eq(["Duplication"])
expect(json["location"]).to eq({
"path" => "foo.rb",
"lines" => { "begin" => 1, "end" => 5 },
})
expect(json["remediation_points"]).to eq(360000)
expect(json["other_locations"]).to eq([
{"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} },
])
expect(json["content"]["body"]).to match /This issue has a mass of `36`/
expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762")
EORUBY

result = run_engine(engine_conf).strip
json = JSON.parse(result)

expect(json["type"]).to eq("issue")
expect(json["check_name"]).to eq("Similar code")
expect(json["description"]).to eq("Similar code found in 1 other location")
expect(json["categories"]).to eq(["Duplication"])
expect(json["location"]).to eq({
"path" => "foo.rb",
"lines" => { "begin" => 1, "end" => 5 },
})
expect(json["remediation_points"]).to eq(3300000)
expect(json["other_locations"]).to eq([
{"path" => "foo.rb", "lines" => { "begin" => 9, "end" => 13} },
])
expect(json["content"]["body"]).to match /This issue has a mass of `36`/
expect(json["fingerprint"]).to eq("f21b75bbd135ec3ae6638364d5c73762")
end

it "skips unparsable files" do
create_source_file("foo.rb", <<-EORUBY)
---
EORUBY

expect {
expect(run_engine(engine_conf)).to eq("")
}.to output(/Skipping file/).to_stderr
end
end

it "skips unparsable files" do
create_source_file("foo.rb", <<-EORUBY)
---
EORUBY
describe "#calculate_points(issue)" do
let(:analyzer) { Ruby::Main.new(engine_config: engine_conf) }
let(:base_points) { Ruby::Main::BASE_POINTS }
let(:points_per) { Ruby::Main::POINTS_PER_OVERAGE }
let(:threshold) { Ruby::Main::DEFAULT_MASS_THRESHOLD }

context "when issue mass exceeds threshold" do
it "calculates mass overage points" do
issue = double(:issue, mass: threshold + 10)
overage = issue.mass - threshold

expected_points = base_points + overage * points_per
points = analyzer.calculate_points(issue)

expect(points).to eq(expected_points)
end
end

expect {
expect(run_engine(engine_conf)).to eq("")
}.to output(/Skipping file/).to_stderr
context "when issue mass is less than threshold" do
it "calculates mass overage points" do
issue = double(:issue, mass: threshold - 5)
overage = issue.mass - threshold

expected_points = base_points + overage * points_per
points = analyzer.calculate_points(issue)

expect(points).to eq(expected_points)
end
end

context "when issue mass equals threshold" do
it "calculates mass overage points" do
issue = double(:issue, mass: threshold)
overage = issue.mass - threshold

expected_points = base_points + overage * points_per
points = analyzer.calculate_points(issue)

expect(points).to eq(expected_points)
end
end
end
end

def engine_conf
CC::Engine::Analyzers::EngineConfig.new({})
def engine_conf
EngineConfig.new({})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably small enough now to inline on L62.

end
end
end