From fc9d72a6320115bf84c0f86b1dfb9bdf85c734f3 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Mon, 28 Apr 2014 22:22:08 -0500 Subject: [PATCH 1/2] Ask don't tell for Hotspot generate_records --- lib/metric_fu/metrics/churn/hotspot.rb | 9 +++++---- lib/metric_fu/metrics/flay/hotspot.rb | 10 +++++----- lib/metric_fu/metrics/flog/hotspot.rb | 10 +++++----- .../metrics/hotspots/analysis/analyzer_tables.rb | 4 ++++ lib/metric_fu/metrics/hotspots/analysis/table.rb | 7 +++++++ lib/metric_fu/metrics/hotspots/hotspot.rb | 5 ++--- lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb | 2 +- lib/metric_fu/metrics/rcov/hotspot.rb | 12 ++++++------ lib/metric_fu/metrics/reek/hotspot.rb | 10 +++++----- lib/metric_fu/metrics/roodi/hotspot.rb | 8 ++++---- lib/metric_fu/metrics/saikuro/hotspot.rb | 12 ++++++------ lib/metric_fu/metrics/stats/hotspot.rb | 10 +++++----- .../hotspots/analysis/analyzed_problems_spec.rb | 2 +- .../hotspots/analysis/analyzer_tables_spec.rb | 2 +- .../metrics/hotspots/analysis/rankings_spec.rb | 4 ++-- 15 files changed, 59 insertions(+), 48 deletions(-) diff --git a/lib/metric_fu/metrics/churn/hotspot.rb b/lib/metric_fu/metrics/churn/hotspot.rb index f6f14c514..8f9cc3433 100644 --- a/lib/metric_fu/metrics/churn/hotspot.rb +++ b/lib/metric_fu/metrics/churn/hotspot.rb @@ -27,10 +27,11 @@ def calculate_score(metric_ranking, item) metric_ranking.scored?(item) ? flat_churn_score : 0 end - def generate_records(data, table) - return if data==nil - Array(data[:changes]).each do |change| - table << { + # @return [Array] + def generate_records(data) + return [] if data==nil + Array(data[:changes]).map do |change| + { "metric" => :churn, "times_changed" => change[:times_changed], "file_path" => change[:file_path] diff --git a/lib/metric_fu/metrics/flay/hotspot.rb b/lib/metric_fu/metrics/flay/hotspot.rb index 0b72bf62e..da976eaef 100644 --- a/lib/metric_fu/metrics/flay/hotspot.rb +++ b/lib/metric_fu/metrics/flay/hotspot.rb @@ -22,9 +22,9 @@ def score_strategy :percentile end - def generate_records(data, table) - return if data==nil - Array(data[:matches]).each do |match| + def generate_records(data) + return [] if data==nil + Array(data[:matches]).flat_map do |match| problems = match[:reason] matching_reason = problems.gsub(/^[0-9]+\) /,'').gsub(/\:[0-9]+/,'') files = [] @@ -35,8 +35,8 @@ def generate_records(data, table) files << file_path end files = files.uniq - files.each do |file| - table << { + files.map do |file| + { "metric" => self.name, "file_path" => file, "flay_reason" => problems+" files: #{locations.join(', ')}", diff --git a/lib/metric_fu/metrics/flog/hotspot.rb b/lib/metric_fu/metrics/flog/hotspot.rb index 80156be3c..e991fdddb 100644 --- a/lib/metric_fu/metrics/flog/hotspot.rb +++ b/lib/metric_fu/metrics/flog/hotspot.rb @@ -22,13 +22,13 @@ def score_strategy :identity end - def generate_records(data, table) - return if data==nil - Array(data[:method_containers]).each do |method_container| - Array(method_container[:methods]).each do |entry| + def generate_records(data) + return [] if data==nil + Array(data[:method_containers]).flat_map do |method_container| + Array(method_container[:methods]).map do |entry| file_path = entry[1][:path].sub(%r{^/},'') if entry[1][:path] location = MetricFu::Location.for(entry.first) - table << { + { "metric" => name, "score" => entry[1][:score], "file_path" => file_path, diff --git a/lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb b/lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb index f942dad4e..3b9a5684a 100644 --- a/lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb +++ b/lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb @@ -13,6 +13,10 @@ def generate_records process_rows! end + def update(generated_records) + table.concat generated_records + end + def tool_tables @tool_tables ||= make_table_hash(@columns) end diff --git a/lib/metric_fu/metrics/hotspots/analysis/table.rb b/lib/metric_fu/metrics/hotspots/analysis/table.rb index 81b7a6d1e..e007e8bf3 100644 --- a/lib/metric_fu/metrics/hotspots/analysis/table.rb +++ b/lib/metric_fu/metrics/hotspots/analysis/table.rb @@ -25,6 +25,13 @@ def <<(row) updated_key_index(record) if @make_index end + def concat(records) + records.each do |record| + self << record + end + self + end + def each @rows.each do |row| yield row diff --git a/lib/metric_fu/metrics/hotspots/hotspot.rb b/lib/metric_fu/metrics/hotspots/hotspot.rb index 97288e474..d7f0e5b68 100644 --- a/lib/metric_fu/metrics/hotspots/hotspot.rb +++ b/lib/metric_fu/metrics/hotspots/hotspot.rb @@ -69,9 +69,8 @@ def score_strategy # Transforms the data param, if non-nil, into a hash with keys: # 'metric', etc. - # and appends the hash to the table param - # Has no return value - def generate_records(data, table) + # @return [Array] + def generate_records(data) not_implemented end diff --git a/lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb b/lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb index f912178ba..038618ee9 100644 --- a/lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb +++ b/lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb @@ -50,7 +50,7 @@ def setup(result_hash) # to ultimately generate the hotspots @analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns) tool_analyzers.each do |analyzer| - analyzer.generate_records(result_hash[analyzer.name], @analyzer_tables.table) + @analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name]) end @analyzer_tables.generate_records @rankings = MetricFu::HotspotRankings.new(@analyzer_tables.tool_tables) diff --git a/lib/metric_fu/metrics/rcov/hotspot.rb b/lib/metric_fu/metrics/rcov/hotspot.rb index b9fdd88f6..d123bce55 100644 --- a/lib/metric_fu/metrics/rcov/hotspot.rb +++ b/lib/metric_fu/metrics/rcov/hotspot.rb @@ -22,13 +22,13 @@ def score_strategy :identity end - def generate_records(data, table) - return if data==nil - data.each do |file_name, info| + def generate_records(data) + return [] if data==nil + data.flat_map do |file_name, info| next if (file_name == :global_percent_run) || (info[:methods].nil?) - info[:methods].each do |method_name, percentage_uncovered| + info[:methods].map do |method_name, percentage_uncovered| location = MetricFu::Location.for(method_name) - table << { + { "metric" => :rcov, 'file_path' => file_name, 'class_name' => location.class_name, @@ -36,7 +36,7 @@ def generate_records(data, table) "percentage_uncovered" => percentage_uncovered } end - end + end.compact end def present_group(group) diff --git a/lib/metric_fu/metrics/reek/hotspot.rb b/lib/metric_fu/metrics/reek/hotspot.rb index 3269a872a..99211714f 100644 --- a/lib/metric_fu/metrics/reek/hotspot.rb +++ b/lib/metric_fu/metrics/reek/hotspot.rb @@ -26,15 +26,15 @@ def score_strategy :percentile end - def generate_records(data, table) - return if data==nil - data[:matches].each do |match| + def generate_records(data) + return [] if data==nil + data[:matches].flat_map do |match| file_path = match[:file_path] - match[:code_smells].each do |smell| + match[:code_smells].map do |smell| location = MetricFu::Location.for(smell[:method]) smell_type = smell[:type] message = smell[:message] - table << { + { "metric" => name, # important "file_path" => file_path, # important # NOTE: ReekHotspot is currently different than other hotspots with regard diff --git a/lib/metric_fu/metrics/roodi/hotspot.rb b/lib/metric_fu/metrics/roodi/hotspot.rb index f2c06b66a..20c789cf5 100644 --- a/lib/metric_fu/metrics/roodi/hotspot.rb +++ b/lib/metric_fu/metrics/roodi/hotspot.rb @@ -22,10 +22,10 @@ def score_strategy :percentile end - def generate_records(data, table) - return if data==nil - Array(data[:problems]).each do |problem| - table << { + def generate_records(data) + return [] if data==nil + Array(data[:problems]).map do |problem| + { "metric" => name, "problems" => problem[:problem], "file_path" => problem[:file] diff --git a/lib/metric_fu/metrics/saikuro/hotspot.rb b/lib/metric_fu/metrics/saikuro/hotspot.rb index 00ae9bc4c..13b60aae7 100644 --- a/lib/metric_fu/metrics/saikuro/hotspot.rb +++ b/lib/metric_fu/metrics/saikuro/hotspot.rb @@ -22,16 +22,16 @@ def score_strategy :identity end - def generate_records(data, table) - return if data == nil - data[:files].each do |file| + def generate_records(data) + return [] if data == nil + data[:files].flat_map do |file| file_name = file[:filename] - file[:classes].each do |klass| + file[:classes].flat_map do |klass| location = MetricFu::Location.for(klass[:class_name]) offending_class = location.class_name - klass[:methods].each do |match| + klass[:methods].map do |match| offending_method = MetricFu::Location.for(match[:name]).method_name - table << { + { "metric" => name, "lines" => match[:lines], "complexity" => match[:complexity], diff --git a/lib/metric_fu/metrics/stats/hotspot.rb b/lib/metric_fu/metrics/stats/hotspot.rb index 83bc8e800..b1ed3b191 100644 --- a/lib/metric_fu/metrics/stats/hotspot.rb +++ b/lib/metric_fu/metrics/stats/hotspot.rb @@ -22,16 +22,16 @@ def score_strategy :absent end - def generate_records(data, table) - return if data == nil - data.each do |key, value| + def generate_records(data) + return [] if data == nil + data.map do |key, value| next if value.kind_of?(Array) - table << { + { "metric" => name, "stat_name" => key, "stat_value" => value } - end + end.compact end end diff --git a/spec/metric_fu/metrics/hotspots/analysis/analyzed_problems_spec.rb b/spec/metric_fu/metrics/hotspots/analysis/analyzed_problems_spec.rb index 186885b77..e2467b414 100644 --- a/spec/metric_fu/metrics/hotspots/analysis/analyzed_problems_spec.rb +++ b/spec/metric_fu/metrics/hotspots/analysis/analyzed_problems_spec.rb @@ -17,7 +17,7 @@ def analyzed_problems(result_hash) analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns) tool_analyzers.each do |analyzer| - analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table) + analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name]) end analyzer_tables.generate_records rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables) diff --git a/spec/metric_fu/metrics/hotspots/analysis/analyzer_tables_spec.rb b/spec/metric_fu/metrics/hotspots/analysis/analyzer_tables_spec.rb index 0793bc98a..5edc04dec 100644 --- a/spec/metric_fu/metrics/hotspots/analysis/analyzer_tables_spec.rb +++ b/spec/metric_fu/metrics/hotspots/analysis/analyzer_tables_spec.rb @@ -17,7 +17,7 @@ def analyzer_table(result_hash) analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns) tool_analyzers.each do |analyzer| - analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table) + analyzer_tables.update analyzer.generate_records(result_hash[analyzer.name]) end analyzer_tables.generate_records rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables) diff --git a/spec/metric_fu/metrics/hotspots/analysis/rankings_spec.rb b/spec/metric_fu/metrics/hotspots/analysis/rankings_spec.rb index 5f687f775..c83d3e322 100644 --- a/spec/metric_fu/metrics/hotspots/analysis/rankings_spec.rb +++ b/spec/metric_fu/metrics/hotspots/analysis/rankings_spec.rb @@ -17,8 +17,8 @@ def rankings(result_hash) analyzer_columns = common_columns + granularities + tool_analyzers.map{|analyzer| analyzer.columns}.flatten analyzer_tables = MetricFu::AnalyzerTables.new(analyzer_columns) - tool_analyzers.each do |analyzer| - analyzer.generate_records(result_hash[analyzer.name], analyzer_tables.table) + tool_analyzers.each do |hotspot| + analyzer_tables.update hotspot.generate_records(result_hash[hotspot.name]) end analyzer_tables.generate_records rankings = MetricFu::HotspotRankings.new(analyzer_tables.tool_tables) From c0df5b22286aefe9593d294196450a6623b33e9f Mon Sep 17 00:00:00 2001 From: Benjamin Fleischer Date: Tue, 29 Apr 2014 22:43:37 -0500 Subject: [PATCH 2/2] Spike ReekHotspot (Analyzer) tests --- spec/metric_fu/metrics/reek/hotspot_spec.rb | 156 ++++++++++++++++++++ 1 file changed, 156 insertions(+) create mode 100644 spec/metric_fu/metrics/reek/hotspot_spec.rb diff --git a/spec/metric_fu/metrics/reek/hotspot_spec.rb b/spec/metric_fu/metrics/reek/hotspot_spec.rb new file mode 100644 index 000000000..6711a07d7 --- /dev/null +++ b/spec/metric_fu/metrics/reek/hotspot_spec.rb @@ -0,0 +1,156 @@ +require 'spec_helper' +MetricFu.metrics_require { 'hotspots/metric' } +MetricFu.metrics_require { 'hotspots/hotspot' } +MetricFu.metrics_require { 'hotspots/analysis/record' } +MetricFu.metrics_require { 'reek/hotspot' } +describe MetricFu::ReekHotspot do + let(:data) do + { + "metric"=>:reek, + "file_path"=>"lib/metric_fu.rb", + "reek__message"=>"doesn't depend on instance state", + "reek__type_name"=>"UtilityFunction", + "reek__value"=>nil, + "reek__value_description"=>nil, + "reek__comparable_message"=>"doesn't depend on instance state", + "class_name"=>"MetricFu", + "method_name"=>"MetricFu#current_time" + } + end + let(:row) do MetricFu::Record.new(data, :unused_variable) end + # TODO: This naming could be more clear + let(:analyzer) do MetricFu::ReekHotspot.new end + let(:tool_table) do + table = MetricFu::Table.new(:column_names => analyzer.columns) + table << row + table + end + let(:tool_tables) do {:reek => tool_table} end + let(:metric_violations) do tool_tables[analyzer.name] end + + it "ranks and calculates reek hotspot scores" do + granularity = 'file_path' + metric_ranking = build_metric_ranking(metric_ranking, granularity) + test_metric_ranking(metric_ranking, granularity) + + items_to_score = reek_items_to_score + master_ranking = build_master_ranking(items_to_score) + test_calculate_score(master_ranking, items_to_score) + end + + def build_metric_ranking(metric_ranking, granularity) + metric_ranking = MetricFu::Ranking.new + metric_violations.each do |row| + location = row[granularity] + expect(location).to eq(data["file_path"]) + metric_ranking[location] ||= [] + mapped_row = analyzer.map(row) + expect(mapped_row).to eq(1) # present + metric_ranking[location] << mapped_row + end + metric_ranking + end + def test_metric_ranking(metric_ranking, granularity) + metric_ranking.each do |item, scores| + expect(item).to eq(data["file_path"]) + expect(scores).to eq([1]) + reduced_score = analyzer.reduce(scores) + expect(reduced_score).to eq(1) # sum + metric_ranking[item] = reduced_score + end + end + def build_master_ranking(items_to_score) + master_ranking = MetricFu::Ranking.new + items_to_score.each do |location, score| + master_ranking[location] = score + end + master_ranking + end + def test_calculate_score(master_ranking, items_to_score) + item = 'lib/metric_fu.rb' + sorted_items = master_ranking.send(:sorted_items) + index = sorted_items.index(item) + expect(index).to eq(21) + length = items_to_score.size + expect(length).to eq(45) + + adjusted_index = index + 1 + worse_item_count = length - adjusted_index + + score = Float(worse_item_count) / length + + expect(analyzer.score(master_ranking, item)).to eq(score) # percentile + expect(master_ranking.percentile(item)).to eq(score) + expect(MetricFu::HotspotScoringStrategies.percentile(master_ranking, item)).to eq(score) + end + + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:58:in `calculate_metric_scores' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:46:in `calculate_score_for_granularity' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:41:in `block in calculate_scores_by_granularities' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:40:in `each' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:40:in `calculate_scores_by_granularities' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:18:in `block in calculate_scores' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:17:in `each' + # lib/metric_fu/metrics/hotspots/analysis/rankings.rb:17:in `calculate_scores' + # lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb:57:in `setup' + # lib/metric_fu/metrics/hotspots/hotspot_analyzer.rb:25:in `initialize' + # lib/metric_fu/metrics/hotspots/generator.rb:22:in `new' + # lib/metric_fu/metrics/hotspots/generator.rb:22:in `analyze' + # lib/metric_fu/generator.rb:107:in `generate_result' + # lib/metric_fu/reporting/result.rb:50:in `add' + # lib/metric_fu/run.rb:19:in `block in measure' + # lib/metric_fu/run.rb:17:in `each' + # lib/metric_fu/run.rb:17:in `measure' + # lib/metric_fu/run.rb:8:in `run' + # lib/metric_fu/cli/helper.rb:18:in `run' + # lib/metric_fu/cli/client.rb:18:in `run' + def reek_items_to_score + { + "lib/metric_fu.rb"=>4, + "lib/metric_fu/cli/client.rb"=>1, + "lib/metric_fu/cli/helper.rb"=>5, + "lib/metric_fu/cli/parser.rb"=>33, + "lib/metric_fu/configuration.rb"=>4, + "lib/metric_fu/constantize.rb"=>8, + "lib/metric_fu/data_structures/line_numbers.rb"=>11, + "lib/metric_fu/data_structures/location.rb"=>7, + "lib/metric_fu/data_structures/sexp_node.rb"=>5, + "lib/metric_fu/environment.rb"=>4, + "lib/metric_fu/errors/analysis_error.rb"=>1, + "lib/metric_fu/formatter.rb"=>1, + "lib/metric_fu/formatter/html.rb"=>11, + "lib/metric_fu/formatter/syntax.rb"=>2, + "lib/metric_fu/formatter/yaml.rb"=>1, + "lib/metric_fu/gem_run.rb"=>3, + "lib/metric_fu/gem_version.rb"=>5, + "lib/metric_fu/generator.rb"=>7, + "lib/metric_fu/io.rb"=>5, + "lib/metric_fu/loader.rb"=>6, + "lib/metric_fu/logger.rb"=>4, + "lib/metric_fu/logging/mf_debugger.rb"=>2, + "lib/metric_fu/metric.rb"=>7, + "lib/metric_fu/metrics/cane/generator.rb"=>6, + "lib/metric_fu/metrics/cane/grapher.rb"=>2, + "lib/metric_fu/metrics/cane/metric.rb"=>2, + "lib/metric_fu/metrics/cane/violations.rb"=>7, + "lib/metric_fu/metrics/churn/generator.rb"=>3, + "lib/metric_fu/metrics/churn/hotspot.rb"=>9, + "lib/metric_fu/metrics/churn/metric.rb"=>1, + "lib/metric_fu/metrics/flay/generator.rb"=>4, + "lib/metric_fu/metrics/flay/grapher.rb"=>2, + "lib/metric_fu/metrics/flay/hotspot.rb"=>5, + "lib/metric_fu/metrics/flay/metric.rb"=>1, + "lib/metric_fu/metrics/flog/generator.rb"=>11, + "lib/metric_fu/metrics/flog/grapher.rb"=>11, + "lib/metric_fu/metrics/flog/hotspot.rb"=>8, + "lib/metric_fu/metrics/flog/metric.rb"=>1, + "lib/metric_fu/metrics/hotspots/analysis/analyzed_problems.rb"=>1, + "lib/metric_fu/metrics/hotspots/analysis/analyzer_tables.rb"=>8, + "lib/metric_fu/metrics/hotspots/analysis/grouping.rb"=>1, + "lib/metric_fu/metrics/hotspots/analysis/groupings.rb"=>1, + "lib/metric_fu/metrics/hotspots/analysis/problems.rb"=>2, + "lib/metric_fu/metrics/hotspots/analysis/ranked_problem_location.rb"=>5, + "lib/metric_fu/metrics/hotspots/analysis/ranking.rb"=>1, + } + end +end