(maint) Report format abstraction & JUnit support

.rubocop.yml

@@ -6,7 +6,7 @@ AllCops:
   Exclude:
     # binstubs, and other utilities
     - bin/**/*
-    - vendor/bundle/**/*
+    - vendor/**/*
 
 Layout/IndentHeredoc:
   Description: The `squiggly` style would be preferable, but is only available from ruby 2.3. We'll enable this when we can.
@@ -58,6 +58,14 @@ RSpec/HookArgument:
   Description: Prefer explicit :each argument, matching existing module's style
   EnforcedStyle: each
 
+RSpec/NestedGroups:
+  Description: Nested groups can lead to cleaner tests with less duplication
+  Max: 10
+
+RSpec/ExampleLength:
+  Description: Forcing short examples leads to the creation of one-time use let() helpers
+  Enabled: False
+
 # Style Cops
 Style/AsciiComments:
   Description: Names, non-english speaking communities.

.rubocop_todo.yml

@@ -32,7 +32,6 @@ RSpec/FilePath:
     - 'spec/generators/puppet_object_spec.rb'
     - 'spec/logger_spec.rb'
     - 'spec/module/metadata_spec.rb'
-    - 'spec/report_spec.rb'
     - 'spec/template_file_spec.rb'
     - 'spec/util/bundler_spec.rb'
     - 'spec/validate_spec.rb'

lib/pdk/cli/util/option_normalizer.rb

@@ -7,34 +7,59 @@ def self.comma_separated_list_to_array(list, _options = {})
           list.split(',').compact
         end
 
-        # Parse one or more format:target pairs.
-        # @return [Array<Report>] An array of one or more Reports.
-        def self.report_formats(formats, _options = {})
-          reports = []
-          formats.each do |f|
-            if f.include?(':')
-              format, target = f.split(':')
-            else
-              format = f
-              target = PDK::Report.default_target
+        # Parse one or more format:target pairs into report format
+        # specifications.
+        #
+        # Each specification is a Hash with two values:
+        #   :method => The name of the method to call on the PDK::Report object
+        #              to render the report.
+        #   :target => The target to write the report to. This can be either an
+        #              IO object that implements #write, or a String filename
+        #              that will be opened for writing.
+        #
+        # If the target given is "stdout" or "stderr", this will convert those
+        # strings into the appropriate IO object.
+        #
+        # @return [Array<Hash{Symbol=>Object}>] An array of one or more report
+        #   format specifications
+        def self.report_formats(formats)
+          formats.map do |f|
+            format, target = f.split(':', 2)
+
+            begin
+              OptionValidator.enum(format, PDK::Report.formats)
+            rescue
+              raise PDK::CLI::FatalError, _("'%{name}' is not a valid report format (%{valid})") % {
+                name:  format,
+                valid: PDK::Report.formats.join(', '),
+              }
             end
 
-            reports << Report.new(target, format)
-          end
+            case target
+            when 'stdout'
+              target = $stdout
+            when 'stderr'
+              target = $stderr
+            end
 
-          reports
+            { method: "to_#{format}".to_sym, target: target }
+          end
         end
 
         def self.parameter_specification(value)
           param_name, param_type = value.split(':', 2)
           param_type = 'String' if param_type.nil?
 
           unless PDK::CLI::Util::OptionValidator.valid_param_name?(param_name)
-            raise PDK::CLI::FatalError, _("'%{name}' is not a valid parameter name") % { name: param_name }
+            raise PDK::CLI::FatalError, _("'%{name}' is not a valid parameter name") % {
+              name: param_name,
+            }
           end
 
           unless PDK::CLI::Util::OptionValidator.valid_data_type?(param_type)
-            raise PDK::CLI::FatalError, _("'%{type}' is not a valid data type") % { type: param_type }
+            raise PDK::CLI::FatalError, _("'%{type}' is not a valid data type") % {
+              type: param_type,
+            }
           end
 
           { name: param_name, type: param_type }

lib/pdk/cli/validate.rb

@@ -13,7 +13,6 @@ module PDK::CLI
       validator_names = PDK::Validate.validators.map { |v| v.name }
       validators = PDK::Validate.validators
       targets = []
-      reports = nil
 
       if opts[:list]
         PDK.logger.info(_('Available validators: %{validator_names}') % { validator_names: validator_names.join(', ') })
@@ -50,31 +49,27 @@ module PDK::CLI
       # Subsequent arguments are targets.
       targets.concat(args[1..-1]) if args.length > 1
 
-      # Note: Reporting may be delegated to the validation tool itself.
-      if opts[:format]
-        reports = Util::OptionNormalizer.report_formats(opts.fetch(:format))
-      end
+      report = PDK::Report.new
+      report_formats = if opts[:format]
+                         PDK::CLI::Util::OptionNormalizer.report_formats(opts[:format])
+                       else
+                         [{
+                           method: PDK::Report.default_format,
+                           target: PDK::Report.default_target,
+                         }]
+                       end
 
       options = targets.empty? ? {} : { targets: targets }
 
       exit_code = 0
 
       validators.each do |validator|
-        results = validator.invoke(options)
-
-        results.each do |_validator_name, result|
-          exit_code = 1 unless result[:exit_code].zero?
+        exit_code = validator.invoke(report, options)
+        break if exit_code != 0
+      end
 
-          if reports
-            reports.each do |r|
-              r.write(result)
-            end
-          else
-            # TODO: not sure if we want this long term
-            $stdout.puts(result[:stdout])
-            $stderr.puts(result[:stderr])
-          end
-        end
+      report_formats.each do |format|
+        report.send(format[:method], format[:target])
       end
 
       exit exit_code

lib/pdk/report.rb

@@ -1,41 +1,86 @@
+require 'rexml/document'
+require 'time'
+require 'pdk/report/event'
+
 module PDK
   class Report
-    attr_reader :format
-    attr_reader :path
-
-    def initialize(path, format = nil)
-      @path = path
-      @format = format || self.class.default_format
-    end
-
+    # @return [Array<String>] the list of supported report formats.
     def self.formats
       @report_formats ||= %w[junit text].freeze
     end
 
+    # @return [Symbol] the method name of the default report format.
     def self.default_format
-      'text'
+      :to_text
     end
 
+    # @return [#write] the default target to write the report to.
     def self.default_target
-      'stdout' # TODO: actually write to stdout
+      $stdout
     end
 
-    def write(text)
-      if @format == 'junit'
-        report = prepare_junit(text)
-      elsif @format == 'text'
-        report = prepare_text(text)
-      end
+    # Memoised access to the report event storage hash.
+    #
+    # The keys of the Hash are the source names of the Events (see
+    # PDK::Report::Event#source).
+    #
+    # @example accessing events from the puppet-lint validator
+    #   report = PDK::Report.new
+    #   report.events['puppet-lint']
+    #
+    # @return [Hash{String=>Array<PDK::Report::Event>}] the events stored in
+    #   the repuort.
+    def events
+      @events ||= {}
+    end
 
-      File.open(@path, 'a') { |f| f.write(report) }
+    # Create a new PDK::Report::Event from a hash of values and add it to the
+    # report.
+    #
+    # @param data [Hash] (see PDK::Report::Event#initialize)
+    def add_event(data)
+      (events[data[:source]] ||= []) << PDK::Report::Event.new(data)
     end
 
-    def prepare_junit(text)
-      "junit: #{text}"
+    # Renders the report as a JUnit XML document.
+    #
+    # @param target [#write] an IO object that the report will be written to.
+    #   Defaults to PDK::Report.default_target.
+    def to_junit(target = self.class.default_target)
+      document = REXML::Document.new
+      document << REXML::XMLDecl.new
+      testsuites = REXML::Element.new('testsuites')
+
+      events.each do |testsuite_name, testcases|
+        testsuite = REXML::Element.new('testsuite')
+        testsuite.attributes['name'] = testsuite_name
+        testsuite.attributes['tests'] = testcases.length
+        testsuite.attributes['errors'] = testcases.select(&:error?).length
+        testsuite.attributes['failures'] = testcases.select(&:failure?).length
+        testsuite.attributes['time'] = 0
+        testsuite.attributes['timestamp'] = Time.now.xmlschema
+        testcases.each { |r| testsuite.elements << r.to_junit }
+
+        testsuites.elements << testsuite
+      end
+
+      document.elements << testsuites
+      document.write(target, 2)
     end
 
-    def prepare_text(text)
-      "text: #{text}"
+    # Renders the report as plain text.
+    #
+    # This report is designed for interactive use by a human and so excludes
+    # all passing events in order to be consise.
+    #
+    # @param target [#write] an IO object that the report will be written to.
+    #   Defaults to PDK::Report.default_target.
+    def to_text(target = self.class.default_target)
+      events.each do |_tool, tool_events|
+        tool_events.each do |event|
+          target.puts(event.to_text) unless event.pass?
+        end
+      end
     end
   end
 end