Merge pull request #205 from dylanratcliffe/formatting

Improved Formatting

Gemfile

@@ -2,6 +2,8 @@ source 'https://rubygems.org'
 
 gemspec
 
+gem 'pry-coolline', '> 0.0', '< 1.0.0'
+
 if ENV['PUPPET_VERSION']
   gem 'puppet', ENV['PUPPET_VERSION']
 end

features/formatting.feature

@@ -0,0 +1,36 @@
+@formatting
+Feature: Format errors nicely
+  I would like Onceover to format errors nicely so that they are easily read
+
+  Background:
+    Given the OnceoverFormatter
+
+  Scenario: Parse a missing class error
+    When Puppet throws the error: "error during compilation: Evaluation Error: Error while evaluating a Function Call, Could not find class ::role::websevrer for server01.foo.com (line: 17, column: 1) on node server01.foo.com"
+    Then the error should parse successfully
+    And it should find 1 error
+    And the parsed error should contain the following keys: text, line, column
+
+  Scenario: Parse an incorrect parameter error
+    When Puppet throws the error: "error during compilation: Evaluation Error: Error while evaluating a Resource Statement, Class[Docker]: has no parameter named 'package_name' (file: /some/path/tp/the/file/module/manifests/profile/docker.pp, line: 8, column: 1) on node server01.foo.com"
+    Then the error should parse successfully
+    And it should find 1 error
+    And the parsed error should contain the following keys: text, file, line, column
+
+  Scenario: Parse a duplication declaration error
+    When Puppet throws the error: "error during compilation: Evaluation Error: Error while evaluating a Resource Statement, Duplicate declaration: Package[virtualenv] is already declared at (file: /some/path/tp/the/file/foo/manifests/profile/ti2.pp, line: 31); cannot redeclare (file: /some/path/tp/the/file/python/manifests/install.pp, line: 54) (file: /some/path/tp/the/file/python/manifests/install.pp, line: 54, column: 3) on node server01.foo.co"
+    Then the error should parse successfully
+    And it should find 2 errors
+    And the parsed errors should contain the following keys: text, file, line, column
+
+  Scenario: Parse a lookup failure error
+    When Puppet throws the error: "error during compilation: Function lookup() did not find a value for the name 'archvsync::repositories' on node server01.foo.com"
+    Then the error should parse successfully
+    And it should find 1 errors
+    And the parsed errors should contain the following keys: text
+
+  Scenario: Fail to parse an error and survive anyway
+    When Puppet throws the error: "Derp derp derp. This error is 💩"
+    Then the error should parse successfully
+    And it should find 1 errors
+    And the parsed errors should contain the following keys: text

features/step_definitions/formatter.rb

@@ -0,0 +1,41 @@
+Given(/^the OnceoverFormatter$/) do
+  require 'rspec'
+  require 'onceover/rspec/formatters'
+
+  RSpec.configure do |c|
+    # Create onceover settings to be accessed by formatters
+    c.add_setting :onceover_tempdir
+    c.add_setting :onceover_root
+    c.add_setting :onceover_environmentpath
+
+    c.onceover_tempdir         = "/Users/foo/git/controlrepo/.onceover"
+    c.onceover_root            = "/Users/foo/git/controlrepo"
+    c.onceover_environmentpath = "etc/puppetlabs/code/environments"
+  end
+
+  @formatter = OnceoverFormatter.new(STDOUT)
+end
+
+When(/^Puppet throws the error: "(.*)"$/) do |error|
+  @error = error
+end
+
+Then(/^the error should parse successfully$/) do
+  expect do
+    @parsed_error = @formatter.parse_errors(@error)
+  end.to_not raise_error
+end
+
+Then(/^it should find (\d+) errors?$/) do |number|
+  expect(@parsed_error.length).to be(number.to_i)
+end
+
+Then(/^the parsed errors? should contain the following keys: (.*)$/) do |keys|
+  # Split the keys into an array
+  keys = keys.split(',').map(&:strip)
+  @parsed_error.each do |error|
+    keys.each do |k|
+      expect(error).to have_key(k.to_sym)
+    end
+  end
+end

lib/onceover/cli/run.rb

@@ -41,7 +41,7 @@ def self.command
             summary 'Runs spec tests'
 
             optional :p, :parallel, 'Runs spec tests in parallel. This increases speed at the cost of poorly formatted logs and irrelevant junit output.'
-            optional nil, :format, 'Which RSpec formatter to use, valid options are: documentation, progress, FailureCollector. You also specify this multiple times', multiple: true, default: :defaults
+            optional nil, :format, 'Which RSpec formatter to use, valid options are: documentation, progress, FailureCollector, OnceoverFormatter. You also specify this multiple times', multiple: true, default: :defaults
 
             run do |opts, args, cmd|
               repo = Onceover::Controlrepo.new(opts)

lib/onceover/rspec/formatters.rb

@@ -0,0 +1,239 @@
+require 'pathname'
+
+class OnceoverFormatter
+  RSpec::Core::Formatters.register self, :example_group_started,
+    :example_passed, :example_failed, :example_pending, :dump_failures#, :dump_summary
+
+  COMPILATION_ERROR      = %r{error during compilation: (?<error>.*)}
+  ERROR_WITH_LOCATION    = %r{(?<error>.*?)\s(at )?(\((file: (?<file>.*?), )?line: (?<line>\d+)(, column: (?<column>\d+))?\))(; )?}
+  ERROR_WITHOUT_LOCATION = %r{(?<error>.*?)\son node}
+
+  def initialize output
+    @output        = output
+    @previous_role = nil
+  end
+
+  def example_group_started notification
+    if notification.group.parent_groups == [notification.group]
+      # If this is the highest level group (The role)
+      role = notification.group.description
+      if role != @previous_role
+        @output << "\n"
+        @output << class_name("#{notification.group.description}:")
+
+        # Calculate the padding required
+        padding = (longest_group - role.length) + 1
+        # Create padding
+        padding.times { @output << ' ' }
+
+        # Save the role name
+        @previous_role = role
+      end
+    else
+      # If not then this will be a test for that role
+      @output << '? '
+    end
+  end
+
+  def example_passed notification
+    @output << "\b\b"
+    @output << "#{green('P')} "
+  end
+
+  def example_failed notification
+    @output << "\b\b"
+    @output << "#{red('F')} "
+  end
+
+  def example_pending notification
+    @output << "\b\b"
+    @output << "#{yellow('?')} "
+  end
+
+  def dump_failures notification
+    require 'onceover/controlrepo'
+
+    # Group by role
+    grouped = notification.failed_examples.group_by { |e| e.metadata[:example_group][:parent_example_group][:description]}
+
+    # Further group by error
+    grouped.each do |role, failures|
+      grouped[role] = failures.uniq { |f| f.metadata[:execution_result].exception.to_s }
+    end
+
+    # Put some spacing before the results
+    @output << "\n\n\n"
+
+    grouped.each do |role, failures|
+      role = {
+        name: role,
+        errors: failures.map { |f| parse_errors(f.metadata[:execution_result].exception.to_s)}.flatten,
+      }
+
+      @output << Onceover::Controlrepo.evaluate_template('error_summary.yaml.erb', binding)
+    end
+  end
+
+  def parse_errors(raw_error)
+    # Check if the error is a compilation error
+    match = COMPILATION_ERROR.match(raw_error)
+    if match
+      compilation_error = match['error']
+      # Check if we car parse it
+      if ERROR_WITH_LOCATION.match(compilation_error)
+        scanned_errors = match['error'].scan(ERROR_WITH_LOCATION)
+
+        # Delete any matches where there was no error text
+        scanned_errors.delete_if { |e| e.first.empty? }
+
+        scanned_errors.map do |error_matches|
+          {
+            text:   error_matches[0],
+            file:   calculate_relative_source(error_matches[1]),
+            line:   error_matches[2],
+            column: error_matches[3],
+          }
+        end
+      elsif ERROR_WITHOUT_LOCATION.match(compilation_error)
+        scanned_errors = match['error'].scan(ERROR_WITHOUT_LOCATION)
+
+        # Delete any matches where there was no error text
+        scanned_errors.delete_if { |e| e.first.empty? }
+
+        scanned_errors.map do |error_matches|
+          {
+            text: error_matches[0],
+          }
+        end
+      else
+        [{
+          text: raw_error,
+        }]
+      end
+    else
+      [{
+        text: raw_error,
+      }]
+    end
+  end
+
+  # This method calculates where the original source file is relative to the
+  # user's current location. This is more compliacted than it sounds because
+  # if we are running from the root of the controlrepo and we have an error in:
+  #
+  # /Users/dylan/git/puppet_controlrepo/.onceover/etc/puppetlabs/code/environments/production/site/role/manifests/lb.pp
+  #
+  # We need that to end up pointing at the original source file not the cached
+  # one i.e.
+  #
+  # site/role/manifests/lb.pp
+  #
+  def calculate_relative_source(file)
+    return nil if file.nil?
+
+    file            = Pathname.new(file)
+    tempdir         = Pathname.new(RSpec.configuration.onceover_tempdir)
+    root            = Pathname.new(RSpec.configuration.onceover_root)
+    environmentpath = Pathname.new(RSpec.configuration.onceover_environmentpath)
+
+    # Calculate the full relative path
+    file.relative_path_from(tempdir + environmentpath + "production").to_s
+  end
+
+  private
+
+  # Below are defined the styles for the output
+  def class_name(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :bold)
+  end
+
+  def black(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :black)
+  end
+
+  def red(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :red)
+  end
+
+  def green(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :green)
+  end
+
+  def yellow(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :yellow)
+  end
+
+  def blue(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :blue)
+  end
+
+  def magenta(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :magenta)
+  end
+
+  def cyan(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :cyan)
+  end
+
+  def white(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :white)
+  end
+
+  def bold(text)
+    RSpec::Core::Formatters::ConsoleCodes.wrap(text, :bold)
+  end
+
+  def longest_group
+    RSpec.configuration.world.example_groups.max { |a,b| a.description.length <=> b.description.length}.description.length
+  end
+
+end
+
+# class OnceoverFormatterParallel < OnceoverFormatter
+#   require 'yaml'
+
+#   def example_group_started notification
+#     # Do nothing
+#   end
+
+#   def example_passed notification
+#     @output << green('P')
+#   end
+
+#   def example_failed notification
+#     @output << red('F')
+#   end
+
+#   def example_pending notification
+#     @output << yellow('?')
+#   end
+
+#   def dump_failures
+#     # TODO: This should write to a file and then get picked up and formatted by onceover itself
+#     # might need to use a module for the formatting
+#     require 'pry'
+#     binding.pry
+#     RSpec.configuration.onceover_tempdir
+#   end
+
+# end
+
+class FailureCollector
+  RSpec::Core::Formatters.register self, :dump_failures
+
+  def initialize(output)
+    FileUtils.touch(File.expand_path("#{RSpec.configuration.onceover_tempdir}/failures.out"))
+  end
+
+  def dump_failures(failures)
+    open(File.expand_path("#{RSpec.configuration.onceover_tempdir}/failures.out"), 'a') { |f|
+      failures.failed_examples.each do |fe|
+        f.puts
+        f.puts "#{fe.metadata[:description]}"
+        f.puts "#{fe.metadata[:execution_result].exception.to_s}"
+        f.puts "#{fe.metadata[:file_path]}:#{fe.metadata[:line_number]}"
+        f.puts "------------------------------------------------------"
+      end
+    }
+  end
+end

lib/onceover/testconfig.rb

@@ -53,7 +53,7 @@ def initialize(file, opts = {})
 
       # Set dynamic defaults for format
       if opts[:format] == [:defaults]
-        @formatters = opts[:parallel] ? ['documentation', 'FailureCollector'] : ['documentation']
+        @formatters = opts[:parallel] ? ['documentation', 'FailureCollector'] : ['OnceoverFormatter']
       else
         @formatters = opts[:format]
       end

templates/error_summary.yaml.erb

@@ -0,0 +1,14 @@
+<%= bold(role[:name]) %>: <%= red('failed') %>
+  errors:
+<% role[:errors].each do |error| -%>
+    <%= red(error[:text]) %>
+<% if error[:file] -%>
+      file: <%= bold(error[:file]) %>
+<% end -%>
+<% if error[:line] -%>
+      line: <%= bold(error[:line]) %>
+<% end -%>
+<% if error[:column] -%>
+      column: <%= bold(error[:column]) %>
+<% end -%>
+<% end -%>