From 5aee04532d17acf1dea9c05e54a414a89cb37d32 Mon Sep 17 00:00:00 2001 From: Andy Schrage Date: Mon, 23 Dec 2013 17:56:55 -0800 Subject: [PATCH 1/5] Adding initial git support --- lib/metric_fu/cli/parser.rb | 9 +++++++ lib/metric_fu/configuration.rb | 4 +-- lib/metric_fu/formatter/html.rb | 8 +++++- lib/metric_fu/run.rb | 35 +++++++++++++++++++++++++-- lib/metric_fu/utility.rb | 1 + metric_fu.gemspec | 6 +++-- spec/cli/helper_spec.rb | 6 +++++ spec/metric_fu/formatter/html_spec.rb | 7 ++++++ spec/metric_fu/run_spec.rb | 25 +++++++++++++++++++ 9 files changed, 94 insertions(+), 7 deletions(-) diff --git a/lib/metric_fu/cli/parser.rb b/lib/metric_fu/cli/parser.rb index b50415be6..6c524cf9f 100644 --- a/lib/metric_fu/cli/parser.rb +++ b/lib/metric_fu/cli/parser.rb @@ -49,6 +49,7 @@ def configure_options(p) add_format_option(p) add_output_option(p) + add_git_option(p) p.banner = @banner unless @banner.nil? p.on_tail("-h", "--help", "Show this message") {puts p ; success!} @@ -76,6 +77,14 @@ def add_general_option(p, o) end end + def add_git_option(p) + p.on('--githash COMMIT_HASH', + 'Specify the git hash run metric_fu against') do |o| + @result[:githash] ||= '' + @result[:githash] << o + end + end + def add_output_option(p) p.on("--out FILE|DIR", "Specify the file or directory to use for output", diff --git a/lib/metric_fu/configuration.rb b/lib/metric_fu/configuration.rb index 67995fcbb..111d09f03 100644 --- a/lib/metric_fu/configuration.rb +++ b/lib/metric_fu/configuration.rb @@ -122,8 +122,8 @@ def metric_manually_configured?(metric) end # TODO: Reconsider method name/behavior, as it really adds a formatter - def configure_formatter(format, output = nil) - @formatters << MetricFu::Formatter.class_for(format).new(output: output) + def configure_formatter(format, output = nil, filename = nil) + @formatters << MetricFu::Formatter.class_for(format).new(output: output, filename: filename) end # @return [Array] names of enabled metrics with graphs diff --git a/lib/metric_fu/formatter/html.rb b/lib/metric_fu/formatter/html.rb index 812c437b9..cede0bb3b 100644 --- a/lib/metric_fu/formatter/html.rb +++ b/lib/metric_fu/formatter/html.rb @@ -17,7 +17,7 @@ def finish mf_debug "** SAVING REPORT DATA OUTPUT TO #{MetricFu::Io::FileSystem.directory('data_directory')}" # TODO: Allow customizing output filenames MetricFu::Formatter::YAML.new( - output: MetricFu.run_path.join("#{MetricFu::Io::FileSystem.directory('data_directory')}/#{Time.now.strftime("%Y%m%d")}.yml") + output: output_yml ).finish mf_debug "** SAVING TEMPLATIZED REPORT" @@ -42,6 +42,12 @@ def output_directory @output ||= dir_for(@options[:output]) || MetricFu.run_path.join(MetricFu::Io::FileSystem.directory('output_directory')) end + def output_yml + filename_prefix = @options[:filename] || Time.now.strftime("%Y%m%d") + + MetricFu.run_path.join("#{MetricFu::Io::FileSystem.directory('data_directory')}/#{filename_prefix}.yml") + end + # Instantiates a new template class based on the configuration set # in MetricFu::Configuration, or through the MetricFu.config block # in your rake file (defaults to the included AwesomeTemplate), diff --git a/lib/metric_fu/run.rb b/lib/metric_fu/run.rb index 38362995f..113ca8ace 100644 --- a/lib/metric_fu/run.rb +++ b/lib/metric_fu/run.rb @@ -1,3 +1,4 @@ +require 'git' MetricFu.configure module MetricFu class Run @@ -7,6 +8,7 @@ def initialize def run(options={}) configure_run(options) measure + reset_git(options) if git_configured? options display_results if options[:open] end @@ -28,6 +30,7 @@ def display_results private def configure_run(options) disable_metrics(options) + configure_git(options) configure_formatters(options) end def disable_metrics(options) @@ -55,20 +58,48 @@ def configure_metric(metric, metric_options) end end def configure_formatters(options) + filename = options[:githash] # Set the filename for the output; nil is a valid value + # Configure from command line if any. if options[:format] MetricFu.configuration.formatters.clear # Command-line format takes precedence. Array(options[:format]).each do |format, o| - MetricFu.configuration.configure_formatter(format, o) + MetricFu.configuration.configure_formatter(format, o, filename) end end # If no formatters specified, use defaults. if MetricFu.configuration.formatters.empty? Array(MetricFu::Formatter::DEFAULT).each do |format, o| - MetricFu.configuration.configure_formatter(format, o) + MetricFu.configuration.configure_formatter(format, o, filename) end end end + def configure_git(options) + return unless git_configured? options + + begin + @git ||= Git.init + @orig_branch ||= @git.current_branch + @git.checkout(options[:githash]) + rescue Git::GitExecuteError => e + mf_log "Unable to checkout githash: #{options[:githash]}" + raise "Unable to checkout githash: #{options[:githash]}: #{e}" + end + end + def reset_git(options) + return unless git_configured? options + + begin + @git ||= Git.init + @git.checkout(@orig_branch) + rescue Git::GitExecuteError => e + mf_log "Unable to reset git status to branch '#{@orig_branch}'" + raise "Unable to reset git status to branch '#{@orig_branch}'" + end + end + def git_configured?(options) + options.size > 0 && options.has_key?(:githash) + end def reporter MetricFu::Reporter.new(MetricFu.configuration.formatters) end diff --git a/lib/metric_fu/utility.rb b/lib/metric_fu/utility.rb index 841c2521c..ab148dbda 100644 --- a/lib/metric_fu/utility.rb +++ b/lib/metric_fu/utility.rb @@ -1,5 +1,6 @@ require 'yaml' require 'fileutils' +require 'git' module MetricFu module Utility module_function diff --git a/metric_fu.gemspec b/metric_fu.gemspec index 631c8c58e..4a3a7972e 100644 --- a/metric_fu.gemspec +++ b/metric_fu.gemspec @@ -12,8 +12,8 @@ Gem::Specification.new do |s| s.authors = File.readlines(author_file, :encoding => Encoding::UTF_8).map(&:strip) # used with gem i metric_fu -P HighSecurity - s.cert_chain = ['certs/bf4.pem'] - s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/ + #s.cert_chain = ['certs/bf4.pem'] + #s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/ s.rubyforge_project = 'metric_fu' s.license = 'MIT' @@ -55,6 +55,8 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'coderay' # to_json support s.add_runtime_dependency 'multi_json' + # git support + s.add_runtime_dependency 'git' # temporary filesystem to act on s.add_development_dependency 'test-construct' diff --git a/spec/cli/helper_spec.rb b/spec/cli/helper_spec.rb index 9d02317fb..74720cd7e 100644 --- a/spec/cli/helper_spec.rb +++ b/spec/cli/helper_spec.rb @@ -167,6 +167,12 @@ end end + context 'given a single git hash/tag' do + it 'sets the git flag' do + helper.process_options(['--githash', 'testhash'])[:githash].should eq 'testhash' + end + end + end end diff --git a/spec/metric_fu/formatter/html_spec.rb b/spec/metric_fu/formatter/html_spec.rb index 05d718cc5..530a9cf24 100644 --- a/spec/metric_fu/formatter/html_spec.rb +++ b/spec/metric_fu/formatter/html_spec.rb @@ -49,6 +49,13 @@ def directory(name) }.to create_file("#{directory('data_directory')}/#{Time.now.strftime("%Y%m%d")}.yml") end + it 'creates a data yaml file based on specified filename' do + fn = 'testfilename' + expect { + MetricFu::Formatter::HTML.new(filename: fn).finish + }.to create_file("#{directory('data_directory')}/#{fn}.yml") + end + it "creates a report index html file" do expect { MetricFu::Formatter::HTML.new.finish diff --git a/spec/metric_fu/run_spec.rb b/spec/metric_fu/run_spec.rb index 354a4b322..0ae9261fd 100644 --- a/spec/metric_fu/run_spec.rb +++ b/spec/metric_fu/run_spec.rb @@ -60,6 +60,31 @@ def data_directory expect { metric_fu }.to create_file("#{output_directory}/index.html") end + context 'with a git-hash specified' do + let(:githash) { 'testhash' } + + before do + @branch = 'original_branch' + + # Stub out the git checkout method to not actually checkout a branch, but keep track of the state + Git::Base.any_instance.stub(:checkout) do |branch| + @branch = branch + end + + Git::Base.any_instance.stub(:current_branch) { @branch } + end + + it "creates a git-hash yaml file" do + expect { metric_fu "--githash #{githash} --no-open" }.to create_file("#{data_directory}/#{githash}.yml") + end + + it 'resets context back to the original git state' do + metric_fu "--githash #{githash} --no-open" + + expect(@branch).to eq 'original_branch' + end + end + context "with configured formatter" do it "outputs using configured formatter" do expect { From 223780ed08aa75b5973e113463bb908c1915c35e Mon Sep 17 00:00:00 2001 From: Andy Schrage Date: Mon, 23 Dec 2013 18:46:14 -0800 Subject: [PATCH 2/5] Adding better logging for when checking out the githash and reseting the branch --- lib/metric_fu/run.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/metric_fu/run.rb b/lib/metric_fu/run.rb index 113ca8ace..7caab1d40 100644 --- a/lib/metric_fu/run.rb +++ b/lib/metric_fu/run.rb @@ -80,6 +80,8 @@ def configure_git(options) begin @git ||= Git.init @orig_branch ||= @git.current_branch + + mf_log "CHECKING OUT '#{options[:githash]}'" @git.checkout(options[:githash]) rescue Git::GitExecuteError => e mf_log "Unable to checkout githash: #{options[:githash]}" @@ -91,6 +93,7 @@ def reset_git(options) begin @git ||= Git.init + mf_log "CHECKING OUT ORIGINAL BRANCH '#{@orig_branch}'" @git.checkout(@orig_branch) rescue Git::GitExecuteError => e mf_log "Unable to reset git status to branch '#{@orig_branch}'" From 419a6e4258b7bdd29807d09934afa0c3f9c69355 Mon Sep 17 00:00:00 2001 From: Andy Schrage Date: Mon, 23 Dec 2013 18:47:04 -0800 Subject: [PATCH 3/5] Fixing the "generate_graphs_for_file" method to not be dependent on the filename containing a date. --- lib/metric_fu/metrics/graph.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/metric_fu/metrics/graph.rb b/lib/metric_fu/metrics/graph.rb index 25d1464b1..16f5fbc3f 100644 --- a/lib/metric_fu/metrics/graph.rb +++ b/lib/metric_fu/metrics/graph.rb @@ -48,7 +48,11 @@ def generate_graphs_for_file(metric_file) date_parts = year_month_day_from_filename(metric_file) metrics = MetricFu::Utility.load_yaml(metric_file) - build_graph(metrics, "#{date_parts[:m]}/#{date_parts[:d]}") + # Determine the name of the graph files based on the original metric_file name + graph_filename = metric_file + graph_filename = "#{date_parts[:m]}/#{date_parts[:d]}" if date_parts + + build_graph(metrics, graph_filename) rescue NameError => e mf_log "#{e.message} called in MetricFu::Graph.generate with #{metric_file}" end @@ -66,7 +70,9 @@ def graph! end def year_month_day_from_filename(path_to_file_with_date) - date = path_to_file_with_date.match(/\/(\d+).yml$/)[1] + date = path_to_file_with_date.match(/\/(\d+).yml$/) + return nil unless date + date = date[1] {:y => date[0..3].to_i, :m => date[4..5].to_i, :d => date[6..7].to_i} end end From 97d8552157d1fddbf9fe197ec08756f6951a90c5 Mon Sep 17 00:00:00 2001 From: Andy Schrage Date: Mon, 30 Dec 2013 14:36:48 -0800 Subject: [PATCH 4/5] Removing the dependency on the "git" gem --- Gemfile | 2 ++ lib/metric_fu/cli/parser.rb | 2 +- lib/metric_fu/run.rb | 42 ++++++++++++++++++++++++------------- lib/metric_fu/utility.rb | 1 - metric_fu.gemspec | 2 -- spec/spec_helper.rb | 1 + 6 files changed, 31 insertions(+), 19 deletions(-) diff --git a/Gemfile b/Gemfile index 46b620eb1..59c549a99 100644 --- a/Gemfile +++ b/Gemfile @@ -6,6 +6,8 @@ group :test, :local_development do gem 'pry-nav' # https://github.com/kina/simplecov-rcov-text gem 'simplecov-rcov-text', group: :metrics + # Some tests require git gem to test the Git integration + gem 'git' end gemspec :path => File.expand_path('..', __FILE__) diff --git a/lib/metric_fu/cli/parser.rb b/lib/metric_fu/cli/parser.rb index 6c524cf9f..b070768d4 100644 --- a/lib/metric_fu/cli/parser.rb +++ b/lib/metric_fu/cli/parser.rb @@ -79,7 +79,7 @@ def add_general_option(p, o) def add_git_option(p) p.on('--githash COMMIT_HASH', - 'Specify the git hash run metric_fu against') do |o| + 'Specify the git hash to run metric_fu against') do |o| @result[:githash] ||= '' @result[:githash] << o end diff --git a/lib/metric_fu/run.rb b/lib/metric_fu/run.rb index 7caab1d40..dbcafce0c 100644 --- a/lib/metric_fu/run.rb +++ b/lib/metric_fu/run.rb @@ -1,4 +1,3 @@ -require 'git' MetricFu.configure module MetricFu class Run @@ -77,27 +76,40 @@ def configure_formatters(options) def configure_git(options) return unless git_configured? options - begin - @git ||= Git.init - @orig_branch ||= @git.current_branch + execute_with_git do + begin + @git ||= Git.init + @orig_branch ||= @git.current_branch - mf_log "CHECKING OUT '#{options[:githash]}'" - @git.checkout(options[:githash]) - rescue Git::GitExecuteError => e - mf_log "Unable to checkout githash: #{options[:githash]}" - raise "Unable to checkout githash: #{options[:githash]}: #{e}" + mf_log "CHECKING OUT '#{options[:githash]}'" + @git.checkout(options[:githash]) + rescue Git::GitExecuteError => e + mf_log "Unable to checkout githash: #{options[:githash]}" + raise "Unable to checkout githash: #{options[:githash]}: #{e}" + end end end def reset_git(options) return unless git_configured? options + execute_with_git do + begin + @git ||= Git.init + mf_log "CHECKING OUT ORIGINAL BRANCH '#{@orig_branch}'" + @git.checkout(@orig_branch) + rescue Git::GitExecuteError => e + mf_log "Unable to reset git status to branch '#{@orig_branch}'" + raise "Unable to reset git status to branch '#{@orig_branch}'" + end + end + end + def execute_with_git(&block) begin - @git ||= Git.init - mf_log "CHECKING OUT ORIGINAL BRANCH '#{@orig_branch}'" - @git.checkout(@orig_branch) - rescue Git::GitExecuteError => e - mf_log "Unable to reset git status to branch '#{@orig_branch}'" - raise "Unable to reset git status to branch '#{@orig_branch}'" + require 'git' + yield + rescue LoadError => e + mf_log 'Cannot select git hash; please install git gem' + raise 'Cannot select git hash; please install git gem' end end def git_configured?(options) diff --git a/lib/metric_fu/utility.rb b/lib/metric_fu/utility.rb index ab148dbda..841c2521c 100644 --- a/lib/metric_fu/utility.rb +++ b/lib/metric_fu/utility.rb @@ -1,6 +1,5 @@ require 'yaml' require 'fileutils' -require 'git' module MetricFu module Utility module_function diff --git a/metric_fu.gemspec b/metric_fu.gemspec index 4a3a7972e..a98ced28e 100644 --- a/metric_fu.gemspec +++ b/metric_fu.gemspec @@ -55,8 +55,6 @@ Gem::Specification.new do |s| s.add_runtime_dependency 'coderay' # to_json support s.add_runtime_dependency 'multi_json' - # git support - s.add_runtime_dependency 'git' # temporary filesystem to act on s.add_development_dependency 'test-construct' diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index ac3f75447..5c3a7a178 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,7 @@ require 'construct' require 'json' require 'pry-nav' +require 'git' # add lib to the load path just like rubygems does $:.unshift File.expand_path("../../lib", __FILE__) From 02822a3b4c054e2a2c2c5adaace19256bfd8f13c Mon Sep 17 00:00:00 2001 From: Andy Schrage Date: Mon, 30 Dec 2013 14:43:23 -0800 Subject: [PATCH 5/5] Uncommenting the gemspec lines for cert_chain and signing_key. They shouldnt have been checked in commented out in the first place --- metric_fu.gemspec | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/metric_fu.gemspec b/metric_fu.gemspec index a98ced28e..631c8c58e 100644 --- a/metric_fu.gemspec +++ b/metric_fu.gemspec @@ -12,8 +12,8 @@ Gem::Specification.new do |s| s.authors = File.readlines(author_file, :encoding => Encoding::UTF_8).map(&:strip) # used with gem i metric_fu -P HighSecurity - #s.cert_chain = ['certs/bf4.pem'] - #s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/ + s.cert_chain = ['certs/bf4.pem'] + s.signing_key = File.expand_path("~/.ssh/gem-private_key.pem") if $0 =~ /gem\z/ s.rubyforge_project = 'metric_fu' s.license = 'MIT'