From d1cf385ec6fa8c2498a0a04980b0f9a2dbe4d31d Mon Sep 17 00:00:00 2001 From: Matt Wynne Date: Sun, 21 Jun 2009 22:21:35 +0100 Subject: [PATCH] Refactoring and cleaning up --- lib/cucover.rb | 49 ++++++++++- lib/cucover/cli_commands/coverage_of.rb | 2 +- lib/cucover/cli_commands/show_recordings.rb | 2 +- lib/cucover/controller.rb | 10 +-- lib/cucover/cucumber_hooks.rb | 6 +- lib/cucover/rails.rb | 5 +- lib/cucover/recorder.rb | 37 ++++++++ lib/cucover/recording.rb | 88 +++++++++++-------- lib/cucover/recording/covered_file.rb | 97 ++++++++++----------- lib/cucover/recording/data.rb | 59 ------------- lib/cucover/recording/recorder.rb | 40 --------- lib/cucover/recording/store.rb | 80 ----------------- lib/cucover/store.rb | 70 +++++++++++++++ spec/cucover/controller_spec.rb | 23 ++--- spec/cucover/recording/covered_file_spec.rb | 2 +- spec/cucover/{recording => }/store_spec.rb | 4 +- 16 files changed, 277 insertions(+), 297 deletions(-) create mode 100644 lib/cucover/recorder.rb delete mode 100644 lib/cucover/recording/data.rb delete mode 100644 lib/cucover/recording/recorder.rb delete mode 100644 lib/cucover/recording/store.rb create mode 100644 lib/cucover/store.rb rename spec/cucover/{recording => }/store_spec.rb (91%) diff --git a/lib/cucover.rb b/lib/cucover.rb index 9567775..f09eea9 100644 --- a/lib/cucover.rb +++ b/lib/cucover.rb @@ -1,19 +1,60 @@ $:.unshift(File.dirname(__FILE__)) require 'dependencies' +require 'cucover/logging_config' require 'cucover/cli_commands/coverage_of' require 'cucover/cli_commands/cucumber' require 'cucover/cli_commands/show_recordings' -require 'cucover/logging_config' +require 'cucover/controller' +require 'cucover/cli' require 'cucover/monkey' require 'cucover/rails' require 'cucover/recording' -require 'cucover/controller' -require 'cucover/cli' +require 'cucover/recorder' +require 'cucover/store' module Cucover class << self def logger Logging::Logger['Cucover'] - end + end + + def should_execute?(scenario_or_table_row) + controller(scenario_or_table_row).should_execute? + end + + def start_recording!(scenario_or_table_row) + raise("Already recording. Please call stop first.") if recording? + + @current_recorder = Recorder.new(scenario_or_table_row) + @current_recorder.start! + record_file(scenario_or_table_row.file_colon_line.split(':').first) # TODO: clean this by extending the feature element + end + + def stop_recording! + return unless recording? + + @current_recorder.stop! + store.keep!(@current_recorder.recording) + @current_recorder = nil + end + + def record_file(source_file) + Cucover.logger.debug("Recording extra source file #{source_file}") + @current_recorder.record_file!(source_file) + end + + private + + def controller(scenario_or_table_row) + Controller.new(scenario_or_table_row.file_colon_line, store) + end + + def recording? + !!@current_recorder + end + + def store + @store ||= Store.new + end end end diff --git a/lib/cucover/cli_commands/coverage_of.rb b/lib/cucover/cli_commands/coverage_of.rb index b94dcdb..eba3ddc 100644 --- a/lib/cucover/cli_commands/coverage_of.rb +++ b/lib/cucover/cli_commands/coverage_of.rb @@ -5,7 +5,7 @@ class CoverageOf def initialize(cli_args) @filespec = cli_args[1] - @store = ::Cucover::Recording::Store.new + @store = Store.new end def execute diff --git a/lib/cucover/cli_commands/show_recordings.rb b/lib/cucover/cli_commands/show_recordings.rb index 7b9d167..3d0db95 100644 --- a/lib/cucover/cli_commands/show_recordings.rb +++ b/lib/cucover/cli_commands/show_recordings.rb @@ -2,7 +2,7 @@ module Cucover module CliCommands class ShowRecordings def initialize(cli_args) - @store = Cucover::Recording::Store.new + @store = Store.new end def execute diff --git a/lib/cucover/controller.rb b/lib/cucover/controller.rb index 35d7e4e..09e8199 100644 --- a/lib/cucover/controller.rb +++ b/lib/cucover/controller.rb @@ -1,14 +1,8 @@ module Cucover class Controller - class << self - def [](scenario_or_table_row) - new(scenario_or_table_row.file_colon_line) - end - end - - def initialize(file_colon_line) + def initialize(file_colon_line, store) @file_colon_line = file_colon_line - @store = Recording::Store.new + @store = store end def should_execute? diff --git a/lib/cucover/cucumber_hooks.rb b/lib/cucover/cucumber_hooks.rb index a3b9a45..5a265c3 100644 --- a/lib/cucover/cucumber_hooks.rb +++ b/lib/cucover/cucumber_hooks.rb @@ -16,8 +16,8 @@ def file Cucover.logger.info("Starting #{scenario_or_table_row.class} #{scenario_or_table_row.file_colon_line}") Cucover::Rails.patch_if_necessary - if Cucover::Controller[scenario_or_table_row].should_execute? - Cucover::Recording.start(scenario_or_table_row) + if Cucover.should_execute?(scenario_or_table_row) + Cucover.start_recording!(scenario_or_table_row) else announce "[ Cucover - Skipping clean scenario ]" scenario_or_table_row.skip_invoke! @@ -25,5 +25,5 @@ def file end After do - Cucover::Recording.stop + Cucover.stop_recording! end diff --git a/lib/cucover/rails.rb b/lib/cucover/rails.rb index 594b95d..06a7f6b 100644 --- a/lib/cucover/rails.rb +++ b/lib/cucover/rails.rb @@ -5,15 +5,14 @@ def patch_if_necessary return if @patched return unless defined?(ActionView) - Monkey.extend_every ActionView::Template => Cucover::Rails::RecordsRenders - + Monkey.extend_every ActionView::Template => Cucover::Rails::RecordsRenders @patched = true end end module RecordsRenders def render - Cucover::Recording.record_file(@filename) + Cucover.record_file(@filename) super end end diff --git a/lib/cucover/recorder.rb b/lib/cucover/recorder.rb new file mode 100644 index 0000000..299b7ca --- /dev/null +++ b/lib/cucover/recorder.rb @@ -0,0 +1,37 @@ +module Cucover + class Recorder + def initialize(scenario_or_table_row) + @scenario_or_table_row = scenario_or_table_row + @analyzer = Rcov::CodeCoverageAnalyzer.new + @additional_covered_files = [] + end + + def record_file!(source_file) + unless @additional_covered_files.include?(source_file) + @additional_covered_files << source_file + end + end + + def start! + @start_time = Time.now + @analyzer.install_hook + end + + def stop! + @end_time = Time.now + @analyzer.remove_hook + Cucover.logger.info("Finished recording #{@scenario_or_table_row.file_colon_line}.") + Cucover.logger.debug("Covered files: #{@analyzer.analyzed_files.join(',')}") + Cucover.logger.debug("Additional Covered files: #{@additional_covered_files.join(',')}") + end + + def recording + Recording.new( + @scenario_or_table_row.file_colon_line, + @scenario_or_table_row.exception, + @additional_covered_files, + @analyzer, + @start_time, @end_time) + end + end +end \ No newline at end of file diff --git a/lib/cucover/recording.rb b/lib/cucover/recording.rb index 16d086c..d80f3e3 100644 --- a/lib/cucover/recording.rb +++ b/lib/cucover/recording.rb @@ -1,44 +1,64 @@ -require 'cucover/recording/recorder' -require 'cucover/recording/data' -require 'cucover/recording/covered_file' -require 'cucover/recording/store' - module Cucover - module Recording - class << self - def start(scenario_or_table_row) - raise("Already recording. Please call stop first.") if recording? - - @current_recorder = Recording::Recorder.new(scenario_or_table_row) - @current_recorder.start - record_file(scenario_or_table_row.file_colon_line.split(':').first) # TODO: clean this by extending the feature element - end + class Recording < Struct.new( + :file_colon_line, + :exception, + :additional_covered_files, + :analyzer, + :start_time, :end_time) - def stop - return unless recording? - @current_recorder.stop - store.keep(@current_recorder.to_data) - @current_recorder = nil - end - - def record_file(source_file) - Cucover.logger.debug("Recording extra source file #{source_file}") - @current_recorder.record_file(source_file) - end + def feature_filename + file_colon_line.split(':').first + end + + def covers_file?(source_file) + covered_files.include?(source_file) + end + + def covers_line?(source_file, line_number) + covered_files.detect{ |f| f.file == source_file }.covers_line?(line_number) + end + + def covered_files + @covered_files ||= analyzed_covered_files + additional_covered_files + end - def record_exception(exception) - @current_recorder.fail!(exception) + def failed? + !!exception + end + + private + + def additional_covered_files + super.map do |filename| + CoveredFile.new(filename, nil, self) end - - private + end - def recording? - !!@current_recorder + def analyzed_covered_files + filtered_analyzed_files.map do |filename| + lines, marked_info, count_info = analyzer.data(filename) + CoveredFile.new(filename, marked_info, self) end + end - def store - store ||= Recording::Store.new + def boring?(file) + [ + /gem/, + /vendor/, + /lib\/ruby/, + /cucover/ + ].any? do |expression| + file.match expression end end + + def filtered_analyzed_files + analyzer.analyzed_files.reject{ |f| boring?(f) } + end + + def normalized_files + cleaned_analyzed_files + additional_covered_files + end end -end \ No newline at end of file +end +require 'cucover/recording/covered_file' \ No newline at end of file diff --git a/lib/cucover/recording/covered_file.rb b/lib/cucover/recording/covered_file.rb index 2ec5b14..2dd5e6f 100644 --- a/lib/cucover/recording/covered_file.rb +++ b/lib/cucover/recording/covered_file.rb @@ -1,56 +1,53 @@ module Cucover - module Recording - class CoveredFile - - module HasLineNumberDetail - def covered_lines - return @covered_lines if @covered_lines - @covered_lines = [] - @marked_info.each_with_index do |covered, index| - line_number = index + 1 - @covered_lines << line_number if covered - end - @covered_lines - end - end - - attr_reader :file - - def initialize(full_filename, marked_info, recording) - @recording = recording - @full_filename = full_filename - @marked_info = marked_info - @file = File.expand_path(full_filename).gsub(/^#{Dir.pwd}\//, '') - - extend HasLineNumberDetail if @marked_info - end - - def dirty? - Cucover.logger.debug("#{file} last modified at #{File.mtime(@full_filename)}") - Cucover.logger.debug("#{file} recording started at #{@recording.start_time}") - result = File.mtime(@full_filename).to_i >= @recording.start_time.to_i - Cucover.logger.debug(result ? "dirty" : "not dirty") - result - end - - def covers_line?(line_number) - covered_lines.include?(line_number) - end - - def ==(other) - other == file - end - - def to_s - "#{file}:#{covered_lines.join(':')}" - end - - private - + class Recording::CoveredFile + attr_reader :file + + def initialize(full_filename, rcov_marked_info, recording) + @full_filename = full_filename + @rcov_marked_info = rcov_marked_info + @recording = recording + @file = File.expand_path(full_filename).gsub(/^#{Dir.pwd}\//, '') + + extend HasLineNumberDetail if @rcov_marked_info + end + + def dirty? + Cucover.logger.debug("#{file} last modified at #{File.mtime(@full_filename)}") + Cucover.logger.debug("#{file} recording started at #{@recording.start_time}") + result = File.mtime(@full_filename).to_i >= @recording.start_time.to_i + Cucover.logger.debug("verdict: #{(result ? "dirty" : "not dirty")}") + result + end + + def covers_line?(line_number) + covered_lines.include?(line_number) + end + + def ==(other) + other == file + end + + def to_s + "#{file}:#{covered_lines.join(':')}" + end + + private + + def covered_lines + [''] + end + + module HasLineNumberDetail def covered_lines - [''] + return @covered_lines if @covered_lines + + @covered_lines = [] + @rcov_marked_info.each_with_index do |covered, index| + line_number = index + 1 + @covered_lines << line_number if covered + end + @covered_lines end - end end end \ No newline at end of file diff --git a/lib/cucover/recording/data.rb b/lib/cucover/recording/data.rb deleted file mode 100644 index 0c34845..0000000 --- a/lib/cucover/recording/data.rb +++ /dev/null @@ -1,59 +0,0 @@ -module Cucover - module Recording - class Data < Struct.new(:file_colon_line, :exception, :additional_covered_files, :analyzer, :start_time, :end_time) - def feature_filename - file_colon_line.split(':').first - end - - def covers_file?(source_file) - covered_files.include?(source_file) - end - - def covers_line?(source_file, line_number) - covered_files.detect{ |f| f.file == source_file }.covers_line?(line_number) - end - - def covered_files - @covered_files ||= analyzed_covered_files + additional_covered_files - end - - def failed? - !!exception - end - - private - - def additional_covered_files - super.map do |filename| - CoveredFile.new(filename, nil, self) - end - end - - def analyzed_covered_files - filtered_analyzed_files.map do |filename| - lines, marked_info, count_info = analyzer.data(filename) - CoveredFile.new(filename, marked_info, self) - end - end - - def boring?(file) - [ - /gem/, - /vendor/, - /lib\/ruby/, - /cucover/ - ].any? do |expression| - file.match expression - end - end - - def filtered_analyzed_files - analyzer.analyzed_files.reject{ |f| boring?(f) } - end - - def normalized_files - cleaned_analyzed_files + additional_covered_files - end - end - end -end diff --git a/lib/cucover/recording/recorder.rb b/lib/cucover/recording/recorder.rb deleted file mode 100644 index 05c7250..0000000 --- a/lib/cucover/recording/recorder.rb +++ /dev/null @@ -1,40 +0,0 @@ -module Cucover - module Recording - class Recorder - def initialize(scenario_or_table_row) - @scenario_or_table_row = scenario_or_table_row - @analyzer = Rcov::CodeCoverageAnalyzer.new - @additional_covered_files = [] - end - - def record_file(source_file) - unless @additional_covered_files.include?(source_file) - @additional_covered_files << source_file - end - end - - def start - @start_time = Time.now - @analyzer.install_hook - end - - def stop - @end_time = Time.now - @analyzer.remove_hook - Cucover.logger.info("Finished recording #{@scenario_or_table_row.file_colon_line}.") - Cucover.logger.debug("Covered files: #{@analyzer.analyzed_files.join(',')}") - Cucover.logger.debug("Additional Covered files: #{@additional_covered_files.join(',')}") - end - - def to_data - Data.new( - @scenario_or_table_row.file_colon_line, - @scenario_or_table_row.exception, - @additional_covered_files, - @analyzer, - @start_time, @end_time) - end - - end - end -end \ No newline at end of file diff --git a/lib/cucover/recording/store.rb b/lib/cucover/recording/store.rb deleted file mode 100644 index 395fa16..0000000 --- a/lib/cucover/recording/store.rb +++ /dev/null @@ -1,80 +0,0 @@ -module Cucover - module Recording - class Store - def initialize(cache = DiskCache.new) - @cache = cache - end - - def latest_recordings - recordings.keys.map{ |file_colon_line| latest_recording(file_colon_line) } - end - - def latest_recording(file_colon_line) - recordings[file_colon_line].sort{ |x, y| x.end_time <=> y.end_time }.last - end - - def keep(recording_data) - Cucover.logger.debug("Storing recording of #{recording_data.file_colon_line}") - recordings[recording_data.file_colon_line] << recording_data - @cache.save(@recordings) - end - - def recordings_covering(source_file) - latest_recordings.select { |r| r.covers_file?(source_file) } - end - - def recordings - ensure_recordings_loaded! - @recordings - end - - private - - def ensure_recordings_loaded! - return if @recordings - - if @recordings = @cache.load - Cucover.logger.debug("Loaded #{@recordings.length} recording(s)") - else - Cucover.logger.debug("Starting with no existing coverage data.") - @recordings = Recordings.new - end - end - - class Recordings < Hash - def [](key) - self[key] = [] if super.nil? - super - end - - def dump - dump = [] - each do |file_colon_line, recordings| - dump << "#{file_colon_line} #{recordings.map{ |r| r.to_s }}" - end - dump - end - end - - class DiskCache - def save(recordings) - Cucover.logger.debug("Saving #{recordings.length} recording(s) to #{data_file}") - File.open(data_file, 'w') { |f| f.puts Marshal.dump(recordings) } - end - - def load - return unless File.exists?(data_file) - - Cucover.logger.debug("Reading existing coverage data from #{data_file}") - File.open(data_file) { |f| Marshal.load(f) } - end - - private - - def data_file - Dir.pwd + '/cucover.data' - end - end - end - end -end \ No newline at end of file diff --git a/lib/cucover/store.rb b/lib/cucover/store.rb new file mode 100644 index 0000000..a42c82f --- /dev/null +++ b/lib/cucover/store.rb @@ -0,0 +1,70 @@ +module Cucover + class Store + def initialize(cache = DiskCache.new) + @cache = cache + end + + def latest_recordings + recordings.keys.map{ |file_colon_line| latest_recording(file_colon_line) } + end + + def latest_recording(file_colon_line) + recordings[file_colon_line].sort{ |x, y| x.end_time <=> y.end_time }.last + end + + def keep!(recording_data) + Cucover.logger.debug("Storing recording of #{recording_data.file_colon_line}") + recordings[recording_data.file_colon_line] << recording_data + @cache.save(@recordings) + end + + def recordings_covering(source_file) + latest_recordings.select { |r| r.covers_file?(source_file) } + end + + def recordings + ensure_recordings_loaded! + @recordings + end + + private + + def ensure_recordings_loaded! + return if @recordings + + if @recordings = @cache.load + Cucover.logger.debug("Loaded #{@recordings.length} recording(s)") + else + Cucover.logger.debug("Starting with no existing coverage data.") + @recordings = Recordings.new + end + end + + class Recordings < Hash + def [](key) + self[key] = [] if super.nil? + super + end + end + + class DiskCache + def save(recordings) + Cucover.logger.debug("Saving #{recordings.length} recording(s) to #{data_file}") + File.open(data_file, 'w') { |f| f.puts Marshal.dump(recordings) } + end + + def load + return unless File.exists?(data_file) + + Cucover.logger.debug("Reading existing coverage data from #{data_file}") + File.open(data_file) { |f| Marshal.load(f) } + end + + private + + def data_file + Dir.pwd + '/cucover.data' + end + end + end +end \ No newline at end of file diff --git a/spec/cucover/controller_spec.rb b/spec/cucover/controller_spec.rb index c5faf88..ea83918 100644 --- a/spec/cucover/controller_spec.rb +++ b/spec/cucover/controller_spec.rb @@ -1,15 +1,16 @@ require File.dirname(__FILE__) + '/../spec_helper' -describe Cucover::Controller do - describe "#should_execute?" do - before(:each) do - @store = mock(Cucover::Recording::Store) - Cucover::Recording::Store.stub!(:new).and_return(@store) - @example = mock('Example', :file_colon_line => 'foo.feature:123') - end - it "when no previous recording exists, it should always return true" do - @store.should_receive(:latest_recording).with(@example.file_colon_line).and_return(nil) - Cucover::Controller[@example].should_execute?.should be_true +module Cucover + describe Controller do + describe "#should_execute?" do + before(:each) do + @store = mock(Store) + @example_id = 'foo.feature:123' + end + it "when no previous recording exists, it should always return true" do + @store.should_receive(:latest_recording).with(@example_id).and_return(nil) + Controller.new(@example_id, @store).should_execute?.should be_true + end end end -end +end \ No newline at end of file diff --git a/spec/cucover/recording/covered_file_spec.rb b/spec/cucover/recording/covered_file_spec.rb index 8092b44..95d6f22 100644 --- a/spec/cucover/recording/covered_file_spec.rb +++ b/spec/cucover/recording/covered_file_spec.rb @@ -1,6 +1,6 @@ require File.dirname(__FILE__) + '/../../spec_helper' -module Cucover::Recording +class Cucover::Recording describe CoveredFile do describe "with a marked info" do before(:each) do diff --git a/spec/cucover/recording/store_spec.rb b/spec/cucover/store_spec.rb similarity index 91% rename from spec/cucover/recording/store_spec.rb rename to spec/cucover/store_spec.rb index c24dfc5..3c70416 100644 --- a/spec/cucover/recording/store_spec.rb +++ b/spec/cucover/store_spec.rb @@ -1,6 +1,6 @@ -require File.dirname(__FILE__) + '/../../spec_helper' +require File.dirname(__FILE__) + '/../spec_helper' -module Cucover::Recording +module Cucover describe Store do before(:each) do @cache = mock('cache', :load => nil)