(PDK-402, PDK-403, PDK-404) Update validators to handle targets better.

Updates validators to handle targets consistently and disallows input of
explicit targets that are incompatible with the validator's defined
filter pattern. Also updates the spinner text to be informative about what
kind of files the validator is validating against.

lib/pdk/cli/validate.rb

@@ -93,7 +93,7 @@ module PDK::CLI
         exit_code = exec_group.exit_code
       else
         validators.each do |validator|
-          validator_exit_code = validator.invoke(report, options)
+          validator_exit_code = validator.invoke(report, options.dup)
           exit_code = validator_exit_code if validator_exit_code != 0
         end
       end

lib/pdk/report/event.rb

@@ -96,7 +96,7 @@ def to_text
         location = nil if location.empty?
 
         # TODO: maybe add trace
-        [location, severity, message].compact.join(': ')
+        [severity, source, location, message].compact.join(': ')
       end
 
       # Renders the event as a JUnit XML testcase.
@@ -170,7 +170,9 @@ def sanitise_file(value)
 
         if path.absolute?
           module_root = Pathname.new(PDK::Util.module_root)
-          path.relative_path_from(module_root).to_path
+          path = path.relative_path_from(module_root).to_path
+          path << '/' if path == '.'
+          path
         else
           path.to_path
         end

lib/pdk/validators/base_validator.rb

@@ -16,6 +16,17 @@ def self.cmd_path
         File.join(PDK::Util.module_root, 'bin', cmd)
       end
 
+      # Parses the target strings provided from the CLI
+      #
+      # @param options [Hash] A Hash containing the input options from the CLI.
+      #
+      # @return targets [Array] An Array of Strings containing target file paths
+      #                         for the validator to validate.
+      # @return skipped [Array] An Array of Strings containing targets
+      #                         that are skipped due to target not containing
+      #                         any files that can be validated by the validator.
+      # @return invalid [Array] An Array of Strings containing targets that do
+      #                         not exist, and will not be run by validator.
       def self.parse_targets(options)
         # If no targets are specified, then we will run validations from the
         # base module directory.
@@ -25,17 +36,32 @@ def self.parse_targets(options)
                     options[:targets]
                   end
 
-        targets.map { |target|
+        skipped = []
+        invalid = []
+        matched = targets.map { |target|
           if respond_to?(:pattern)
             if File.directory?(target)
-              Array[pattern].flatten.map { |p| Dir.glob(File.join(target, p)) }
+              target_list = Array[pattern].flatten.map { |p| Dir.glob(File.join(target, p)) }
+              skipped << target if target_list.flatten.empty?
+              target_list
+            elsif File.file?(target)
+              if target.eql? pattern
+                target
+              elsif Array[pattern].flatten.map { |p| File.fnmatch(p, File.expand_path(target)) }.include? true
+                target
+              else
+                skipped << target
+                next
+              end
             else
-              target
+              invalid << target
+              next
             end
           else
             target
           end
-        }.flatten
+        }.compact.flatten
+        [matched, skipped, invalid]
       end
 
       def self.parse_options(_options, targets)
@@ -46,8 +72,37 @@ def self.spinner_text(_targets = nil)
         _('Invoking %{cmd}') % { cmd: cmd }
       end
 
+      def self.process_skipped(report, skipped = [])
+        skipped.each do |skipped_target|
+          PDK.logger.debug(_('%{validator}: Skipped \'%{target}\'. Target does not contain any files to validate (%{pattern}).') % { validator: name, target: skipped_target, pattern: pattern })
+          report.add_event(
+            file:     skipped_target,
+            source:   name,
+            message:  _('Target does not contain any files to validate (%{pattern}).') % { pattern: pattern },
+            severity: :info,
+            state:    :skipped,
+          )
+        end
+      end
+
+      def self.process_invalid(report, invalid = [])
+        invalid.each do |invalid_target|
+          PDK.logger.debug(_('%{validator}: Skipped \'%{target}\'. Target file not found.') % { validator: name, target: invalid_target })
+          report.add_event(
+            file:     invalid_target,
+            source:   name,
+            message:  'File does not exist.',
+            severity: :error,
+            state:    :error,
+          )
+        end
+      end
+
       def self.invoke(report, options = {})
-        targets = parse_targets(options)
+        targets, skipped, invalid = parse_targets(options)
+
+        process_skipped(report, skipped)
+        process_invalid(report, invalid)
 
         return 0 if targets.empty?
 

lib/pdk/validators/metadata/metadata_syntax.rb

@@ -42,7 +42,10 @@ def self.stop_spinner(exit_code)
       end
 
       def self.invoke(report, options = {})
-        targets = parse_targets(options)
+        targets, skipped, invalid = parse_targets(options)
+
+        process_skipped(report, skipped)
+        process_invalid(report, invalid)
 
         return 0 if targets.empty?
 
@@ -57,18 +60,6 @@ def self.invoke(report, options = {})
         JSON.parser = JSON::Pure::Parser
 
         targets.each do |target|
-          unless File.file?(target)
-            report.add_event(
-              file:     target,
-              source:   name,
-              state:    :failure,
-              severity: 'error',
-              message:  _('not a file'),
-            )
-            return_val = 1
-            next
-          end
-
           unless File.readable?(target)
             report.add_event(
               file: target,

lib/pdk/validators/metadata_validator.rb

@@ -18,11 +18,6 @@ def self.metadata_validators
       def self.invoke(report, options = {})
         exit_code = 0
 
-        if options[:targets] && options[:targets] != []
-          PDK.logger.info(_('metadata validator only checks metadata.json. The specified files will be ignored: %{targets}') % { targets: options[:targets].join(', ') })
-          options.delete(:targets)
-        end
-
         metadata_validators.each do |validator|
           exit_code = validator.invoke(report, options)
           break if exit_code != 0

spec/acceptance/report_spec.rb

@@ -25,7 +25,7 @@ class foo { }
 
       describe file('report.txt') do
         it { is_expected.to exist }
-        its(:content) { is_expected.to match %r{^#{Regexp.escape(init_pp)}.*warning.*class not documented} }
+        its(:content) { is_expected.to match %r{^warning:.*#{Regexp.escape(init_pp)}.*class not documented} }
       end
     end
 
@@ -34,7 +34,7 @@ class foo { }
       its(:exit_status) { is_expected.to eq(0) }
       its(:stderr) { is_expected.to match(%r{Checking Puppet manifest syntax}i) }
       its(:stderr) { is_expected.to match(%r{Checking Puppet manifest style}i) }
-      its(:stdout) { is_expected.to match(%r{^#{Regexp.escape(init_pp)}.*warning.*class not documented}) }
+      its(:stdout) { is_expected.to match(%r{^warning:.*#{Regexp.escape(init_pp)}.*class not documented}) }
 
       describe file('stdout') do
         it { is_expected.not_to exist }
@@ -47,7 +47,7 @@ class foo { }
       its(:stdout) { is_expected.to match(%r{\A\Z}) }
       its(:stderr) { is_expected.to match(%r{Checking Puppet manifest syntax}i) }
       its(:stderr) { is_expected.to match(%r{Checking Puppet manifest style}i) }
-      its(:stderr) { is_expected.to match(%r{^#{Regexp.escape(init_pp)}.*warning.*class not documented}) }
+      its(:stderr) { is_expected.to match(%r{^warning:.*#{Regexp.escape(init_pp)}.*class not documented}) }
 
       describe file('stderr') do
         it { is_expected.not_to exist }

spec/acceptance/validate_all_spec.rb

@@ -60,9 +60,9 @@ class validate_all {
       its(:stderr) { is_expected.to match(%r{Checking metadata syntax \(metadata\.json\)}i) }
       its(:stderr) { is_expected.to match(%r{Checking metadata style \(metadata\.json\)}i) }
       its(:stderr) { is_expected.to match(%r{Checking Puppet manifest syntax}i) }
-      its(:stdout) { is_expected.to match(%r{Error: This Name has no effect}i) }
-      its(:stdout) { is_expected.to match(%r{Error: This Type-Name has no effect}i) }
-      its(:stdout) { is_expected.to match(%r{Error: Language validation logged 2 errors. Giving up}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*This Name has no effect}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*This Type-Name has no effect}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*Language validation logged 2 errors. Giving up}i) }
       its(:stderr) { is_expected.to match(%r{Checking Ruby code style}i) }
     end
 

spec/acceptance/validate_metadata_spec.rb

@@ -16,7 +16,7 @@
 
     describe command('pdk validate metadata') do
       its(:exit_status) { is_expected.not_to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^metadata\.json:.+warning.+open ended dependency}) }
+      its(:stdout) { is_expected.to match(%r{^warning:.*metadata\.json:.+open ended dependency}) }
       its(:stderr) { is_expected.to match(spinner_text) }
     end
 
@@ -72,17 +72,17 @@
 
     describe command('pdk validate metadata --format junit broken.json') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stderr) { is_expected.to match(spinner_text) }
-      its(:stderr) { is_expected.to match(%r{will be ignored.*broken.json}) }
+      its(:stderr) { is_expected.not_to match(spinner_text) }
 
       its(:stdout) do
-        is_expected.to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]/testcase').with_attributes(
-          'name' => 'metadata.json',
+        is_expected.to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]').with_attributes(
+          'tests' => '1',
+          'skipped' => '1',
         )
       end
 
       its(:stdout) do
-        is_expected.not_to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]/testcase').with_attributes(
+        is_expected.to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]/testcase').with_attributes(
           'name' => 'broken.json',
         )
       end
@@ -99,25 +99,18 @@
       end
 
       describe command('pdk validate metadata --format junit broken.json') do
-        its(:exit_status) { is_expected.not_to eq(0) }
-        its(:stderr) { is_expected.to match(spinner_text) }
-        its(:stderr) { is_expected.to match(%r{will be ignored.*broken.json}) }
+        its(:exit_status) { is_expected.to eq(0) }
+        its(:stderr) { is_expected.not_to match(spinner_text) }
 
         its(:stdout) do
           is_expected.to have_junit_testsuite('metadata-json-lint').with_attributes(
-            'failures' => eq(1),
-            'tests'    => eq(1),
+            'skipped' => eq(1),
+            'tests' => eq(1),
           )
         end
 
         its(:stdout) do
           is_expected.to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]/testcase').with_attributes(
-            'name' => 'metadata.json',
-          )
-        end
-
-        its(:stdout) do
-          is_expected.not_to have_xpath('/testsuites/testsuite[@name="metadata-json-lint"]/testcase').with_attributes(
             'name' => 'broken.json',
           )
         end

spec/acceptance/validate_puppet_spec.rb

@@ -16,7 +16,7 @@
       its(:exit_status) { is_expected.to eq(0) }
       its(:stderr) { is_expected.not_to match(syntax_spinner_text) }
       its(:stderr) { is_expected.not_to match(lint_spinner_text) }
-      its(:stdout) { is_expected.to match(empty_string) }
+      its(:stdout) { is_expected.to match(%r{Target does not contain any files to validate}) }
     end
 
     describe command('pdk validate puppet --format junit') do
@@ -25,8 +25,8 @@
       its(:stderr) { is_expected.not_to match(lint_spinner_text) }
 
       its(:stdout) { is_expected.to pass_validation(junit_xsd) }
-      its(:stdout) { is_expected.not_to have_junit_testsuite('puppet-syntax') }
-      its(:stdout) { is_expected.not_to have_junit_testsuite('puppet-lint') }
+      its(:stdout) { is_expected.to have_junit_testcase.in_testsuite('puppet-syntax').that_was_skipped }
+      its(:stdout) { is_expected.to have_junit_testcase.in_testsuite('puppet-lint').that_was_skipped }
     end
   end
 
@@ -82,8 +82,8 @@ class foo {
       its(:stderr) { is_expected.to match(syntax_spinner_text) }
       its(:stderr) { is_expected.to match(lint_spinner_text) }
 
-      its(:stdout) { is_expected.to match(%r{Warning: Unrecognized escape sequence \'\\\[\'}i) }
-      its(:stdout) { is_expected.to match(%r{Warning: Unrecognized escape sequence \'\\\]\'}i) }
+      its(:stdout) { is_expected.to match(%r{Warning:.*Unrecognized escape sequence \'\\\[\'}i) }
+      its(:stdout) { is_expected.to match(%r{Warning:.*Unrecognized escape sequence \'\\\]\'}i) }
     end
 
     describe command('pdk validate puppet --format junit') do
@@ -149,7 +149,7 @@ class foo {
 
     describe command('pdk validate puppet') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^#{Regexp.escape(init_pp)}.+class not documented}i) }
+      its(:stdout) { is_expected.to match(%r{^warning:.*#{Regexp.escape(init_pp)}.+class not documented}i) }
       its(:stderr) { is_expected.to match(syntax_spinner_text) }
       its(:stderr) { is_expected.to match(lint_spinner_text) }
     end
@@ -211,9 +211,9 @@ class foo {
       its(:stderr) { is_expected.to match(syntax_spinner_text) }
       its(:stderr) { is_expected.not_to match(lint_spinner_text) }
 
-      its(:stdout) { is_expected.to match(%r{Error: This Name has no effect}i) }
-      its(:stdout) { is_expected.to match(%r{Error: This Type-Name has no effect}i) }
-      its(:stdout) { is_expected.to match(%r{Error: Language validation logged 2 errors. Giving up}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*This Name has no effect}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*This Type-Name has no effect}i) }
+      its(:stdout) { is_expected.to match(%r{Error:.*Language validation logged 2 errors. Giving up}i) }
     end
 
     describe command('pdk validate puppet --format junit') do
@@ -356,8 +356,8 @@ class foo {
         its(:exit_status) { is_expected.not_to eq(0) }
         its(:stderr) { is_expected.to match(syntax_spinner_text) }
         its(:stderr) { is_expected.to match(lint_spinner_text) }
-        its(:stdout) { is_expected.to match(%r{^#{Regexp.escape(another_problem_pp)}}) }
-        its(:stdout) { is_expected.not_to match(%r{^#{Regexp.escape(example_pp)}}) }
+        its(:stdout) { is_expected.to match(%r{#{Regexp.escape(another_problem_pp)}}) }
+        its(:stdout) { is_expected.not_to match(%r{#{Regexp.escape(example_pp)}}) }
       end
 
       describe command("pdk validate puppet --format junit #{another_problem_dir}") do
@@ -401,12 +401,12 @@ class foo {
 
     describe command('pdk validate puppet') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^#{Regexp.escape(example_pp)}.*warning.*double quoted string}) }
+      its(:stdout) { is_expected.to match(%r{^warning:.*#{Regexp.escape(example_pp)}.*double quoted string}) }
     end
 
     describe command('pdk validate puppet --auto-correct') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^#{Regexp.escape(example_pp)}.*corrected.*double quoted string}) }
+      its(:stdout) { is_expected.to match(%r{^corrected:.*#{Regexp.escape(example_pp)}.*double quoted string}) }
     end
 
     describe command('pdk validate puppet') do

spec/acceptance/validate_ruby_spec.rb

@@ -87,12 +87,12 @@
 
     describe command('pdk validate ruby') do
       its(:exit_status) { is_expected.not_to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^test\.rb.*space inside (\{|\}) missing}i) }
+      its(:stdout) { is_expected.to match(%r{test\.rb.*space inside (\{|\}) missing}i) }
     end
 
     describe command('pdk validate ruby --auto-correct') do
       its(:exit_status) { is_expected.to eq(0) }
-      its(:stdout) { is_expected.to match(%r{^test\.rb.*corrected.*space inside (\{|\}) missing}i) }
+      its(:stdout) { is_expected.to match(%r{^corrected:.*test\.rb.*space inside (\{|\}) missing}i) }
     end
 
     describe command('pdk validate ruby') do