Merge pull request #879 from chef/ksubrama/json_fix

Generate test labels for multi-test controls

lib/inspec/rspec_json_formatter.rb

@@ -8,10 +8,17 @@ require 'rspec/core/formatters/json_formatter'
 # Vanilla RSpec JSON formatter with a slight extension to show example IDs.
 # TODO: Remove these lines when RSpec includes the ID natively
 class InspecRspecVanilla < RSpec::Core::Formatters::JsonFormatter
-  RSpec::Core::Formatters.register self, :message, :dump_summary, :dump_profile, :stop, :close
+  RSpec::Core::Formatters.register self
 
   private
 
+  # We are cheating and overriding a private method in RSpec's core JsonFormatter.
+  # This is to avoid having to repeat this id functionality in both dump_summary
+  # and dump_profile (both of which call format_example).
+  # See https://github.com/rspec/rspec-core/blob/master/lib/rspec/core/formatters/json_formatter.rb
+  #
+  # rspec's example id here corresponds to an inspec test's control name -
+  # either explicitly specified or auto-generated by rspec itself.
   def format_example(example)
     res = super(example)
     res[:id] = example.metadata[:id]
@@ -22,8 +29,11 @@ end
 # Minimal JSON formatter for inspec. Only contains limited information about
 # examples without any extras.
 class InspecRspecMiniJson < RSpec::Core::Formatters::JsonFormatter
-  RSpec::Core::Formatters.register self, :message, :dump_summary, :dump_profile, :stop, :close
+  # Don't re-register all the call-backs over and over - we automatically
+  # inherit all callbacks registered by the parent class.
+  RSpec::Core::Formatters.register self, :dump_summary, :stop
 
+  # Called after stop has been called and the run is complete.
   def dump_summary(summary)
     @output_hash[:version] = Inspec::VERSION
     @output_hash[:summary] = {
@@ -34,7 +44,12 @@ class InspecRspecMiniJson < RSpec::Core::Formatters::JsonFormatter
     }
   end
 
+  # Called at the end of a complete RSpec run.
   def stop(notification)
+    # This might be a bit confusing. The results are not actually organized
+    # by control. It is organized by test. So if a control has 3 tests, the
+    # output will have 3 control entries, each one with the same control id
+    # and different test results. An rspec example maps to an inspec test.
     @output_hash[:controls] = notification.examples.map do |example|
       format_example(example).tap do |hash|
         e = example.exception
@@ -72,19 +87,30 @@ class InspecRspecMiniJson < RSpec::Core::Formatters::JsonFormatter
 end
 
 class InspecRspecJson < InspecRspecMiniJson
-  RSpec::Core::Formatters.register self, :message, :dump_summary, :dump_profile, :stop, :close
+  RSpec::Core::Formatters.register self, :start, :stop
   attr_writer :backend
 
   def initialize(*args)
     super(*args)
     @profiles = []
+    # Will be valid after "start" state is reached.
+    @profiles_info = nil
     @backend = nil
   end
 
+  # Called by the runner during example collection.
   def add_profile(profile)
     @profiles.push(profile)
   end
 
+  # Called after all examples have been collected but before rspec
+  # test execution has begun.
+  def start(_notification)
+    # Note that the default profile may have no name - therefore
+    # the hash may have a valid nil => entry.
+    @profiles_info ||= Hash[@profiles.map { |x| profile_info(x) }]
+  end
+
   def dump_one_example(example, control)
     control[:results] ||= []
     example.delete(:id)
@@ -92,29 +118,28 @@ class InspecRspecJson < InspecRspecMiniJson
     control[:results].push(example)
   end
 
-  def profile_info(profile)
-    info = profile.info.dup
-    [info[:name], info]
-  end
-
-  def dump_summary(summary)
-    super(summary)
+  def stop(notification)
+    super(notification)
     examples = @output_hash.delete(:controls)
-    profiles = Hash[@profiles.map { |x| profile_info(x) }]
     missing = []
 
     examples.each do |example|
-      control = example2control(example, profiles)
+      control = example2control(example, @profiles_info)
       next missing.push(example) if control.nil?
       dump_one_example(example, control)
     end
 
-    @output_hash[:profiles] = profiles
+    @output_hash[:profiles] = @profiles_info
     @output_hash[:other_checks] = missing
   end
 
   private
 
+  def profile_info(profile)
+    info = profile.info.dup
+    [info[:name], info]
+  end
+
   def example2control(example, profiles)
     profile = profiles[example[:profile_id]]
     return nil if profile.nil? || profile[:controls].nil?
@@ -130,7 +155,7 @@ class InspecRspecJson < InspecRspecMiniJson
 end
 
 class InspecRspecCli < InspecRspecJson # rubocop:disable Metrics/ClassLength
-  RSpec::Core::Formatters.register self, :message, :dump_summary, :dump_profile, :stop, :close
+  RSpec::Core::Formatters.register self, :close
 
   STATUS_TYPES = {
     'unknown'  => -3,
@@ -169,6 +194,8 @@ class InspecRspecCli < InspecRspecJson # rubocop:disable Metrics/ClassLength
     'empty'    => '     ',
   }.freeze
 
+  MULTI_TEST_CONTROL_SUMMARY_MAX_LEN = 60
+
   def initialize(*args)
     @colors = COLORS
     @indicators = INDICATORS
@@ -181,10 +208,6 @@ class InspecRspecCli < InspecRspecJson # rubocop:disable Metrics/ClassLength
     super(*args)
   end
 
-  def start(_notification)
-    @profiles_info ||= Hash[@profiles.map { |x| profile_info(x) }]
-  end
-
   def close(_notification)
     flush_current_control
     output.puts('') unless @current_control.nil?
@@ -236,24 +259,50 @@ class InspecRspecCli < InspecRspecJson # rubocop:disable Metrics/ClassLength
     [fails, skips, STATUS_TYPES.key(summary_status)]
   end
 
-  def current_control_summary(fails, skips)
-    sum_info = [
-      (fails.length > 0) ? "#{fails.length} failed" : nil,
-      (skips.length > 0) ? "#{skips.length} skipped" : nil,
-    ].compact
-
-    summary = @current_control[:title]
-    unless summary.nil?
-      return summary + ' (' + sum_info.join(' ') + ')' unless sum_info.empty?
-      return summary
+  def current_control_title
+    title = @current_control[:title]
+    res = @current_control[:results]
+    if title
+      title
+    elsif res.length == 1
+      # If it's an anonymous control, just go with the only description
+      # available for the underlying test.
+      res[0][:code_desc].to_s
+    elsif res.length == 0
+      # Empty control block - if it's anonymous, there's nothing we can do.
+      # Is this case even possible?
+      'Empty anonymous control'
+    else
+      # Multiple tests - but no title. Do our best and generate some form of
+      # identifier or label or name.
+      title = (res.map { |r| r[:code_desc] }).join('; ')
+      max_len = MULTI_TEST_CONTROL_SUMMARY_MAX_LEN
+      title = title[0..(max_len-1)] + '...' if title.length > max_len
+      title
+    end
   end
 
-    return sum_info.join(' ') if @current_control[:results].length != 1
-
+  def current_control_summary(fails, skips)
+    title = current_control_title
+    res = @current_control[:results]
+    suffix =
+      if res.length == 1
+        # Single test - be nice and just print the exception message if the test
+        # failed. No need to say "1 failed".
         fails.clear
         skips.clear
-    c = @current_control[:results][0]
-    c[:code_desc].to_s + c[:message].to_s
+        res[0][:message].to_s
+      else
+        [
+          (fails.length > 0) ? "#{fails.length} failed" : nil,
+          (skips.length > 0) ? "#{skips.length} skipped" : nil,
+        ].compact.join(' ')
+      end
+    if suffix == ''
+      title
+    else
+      title + ' (' + suffix + ')'
+    end
   end
 
   def format_line(fields)

test/functional/inspec_exec_test.rb

@@ -60,11 +60,12 @@ Target:  local://
 
 \e[32m  ✔  working should eq \"working\"\e[0m
 \e[37m  ○  skippy This will be skipped intentionally.\e[0m
-\e[31m  ✖  failing should eq \"as intended\"
+\e[31m  ✖  failing should eq \"as intended\" (
      expected: \"as intended\"
           got: \"failing\"
-     \n     (compared using ==)
-     \e[0m
+     
+     (compared using ==)
+     )\e[0m
 
 Summary: \e[32m1 successful\e[0m, \e[31m1 failures\e[0m, \e[37m1 skipped\e[0m
 "