Merge pull request #139 from rodjek/validator_unit_tests

(maint) Finish PDK::Validate::* unit tests

lib/pdk/validators/metadata/metadata_json_lint.rb

@@ -36,18 +36,33 @@ def self.parse_options(_options, targets)
       end
 
       def self.parse_output(report, result, targets)
-        begin
-          json_data = JSON.parse(result[:stdout])
-        rescue JSON::ParserError
-          json_data = []
-        end
+        raise ArgumentError, 'More than 1 target provided to PDK::Validate::MetadataJSONLint' if targets.count > 1
 
-        raise ArgumentError, 'More that 1 target provided to PDK::Validate::MetadataJSONLint' if targets.count > 1
+        if result[:stdout].strip.empty?
+          # metadata-json-lint will print nothing if there are no problems with
+          # the file being linted. This should be handled separately to
+          # metadata-json-lint generating output that can not be parsed as JSON
+          # (unhandled exception in metadata-json-lint).
+          json_data = {}
+        else
+          begin
+            json_data = JSON.parse(result[:stdout])
+          rescue JSON::ParserError
+            report.add_event(
+              file:     targets.first,
+              source:   name,
+              state:    :error,
+              severity: :error,
+              message:  result[:stdout],
+            )
+            return
+          end
+        end
 
         if json_data.empty?
           report.add_event(
             file:     targets.first,
-            source:   cmd,
+            source:   name,
             state:    :passed,
             severity: :ok,
           )
@@ -62,7 +77,7 @@ def self.parse_output(report, result, targets)
 
               report.add_event(
                 file:     targets.first,
-                source:   cmd,
+                source:   name,
                 message:  offense['msg'],
                 test:     offense['check'],
                 severity: event_type,

lib/pdk/validators/puppet/puppet_lint.rb

@@ -43,7 +43,7 @@ def self.parse_output(report, result, targets)
         targets.reject { |target| json_data.any? { |j| j['path'] == target } }.each do |target|
           report.add_event(
             file:     target,
-            source:   'puppet-lint',
+            source:   name,
             severity: 'ok',
             state:    :passed,
           )
@@ -52,7 +52,7 @@ def self.parse_output(report, result, targets)
         json_data.each do |offense|
           report.add_event(
             file:     offense['path'],
-            source:   'puppet-lint',
+            source:   name,
             line:     offense['line'],
             column:   offense['column'],
             message:  offense['message'],

lib/pdk/validators/puppet/puppet_syntax.rb

@@ -44,7 +44,7 @@ def self.parse_output(report, result, targets)
           attributes = {
             source:  name,
             message: message.strip,
-            state:  'failure',
+            state:  :failure,
           }
           attributes[:severity] = severity.strip unless severity.nil?
 
@@ -57,14 +57,14 @@ def self.parse_output(report, result, targets)
           results_data << attributes
         end
 
-        # puppet-lint does not include files without problems in its JSON
+        # puppet parser validate does not include files without problems in its
         # output, so we need to go through the list of targets and add passing
-        # events to the report for any target not listed in the JSON output.
+        # events to the report for any target not listed in the output.
         targets.reject { |target| results_data.any? { |j| j[:file] == target } }.each do |target|
           report.add_event(
             file:     target,
-            source:   'puppet-syntax',
-            severity: 'ok',
+            source:   name,
+            severity: :ok,
             state:    :passed,
           )
         end

spec/pdk/validate/metadata_json_lint_spec.rb

@@ -1,9 +1,198 @@
 require 'spec_helper'
 
 describe PDK::Validate::MetadataJSONLint do
-  it 'invokes `metadata-json-lint`' do
-    allow(PDK::Util).to receive(:module_root).and_return('/')
+  let(:module_root) { File.join('path', 'to', 'test', 'module') }
 
-    expect(described_class.cmd).to match(%r{metadata-json-lint$})
+  before(:each) do
+    allow(PDK::Util).to receive(:module_root).and_return(module_root)
+    allow(File).to receive(:directory?).with(module_root).and_return(true)
+  end
+
+  it 'defines the base validator attributes' do
+    expect(described_class).to have_attributes(
+      name: 'metadata-json-lint',
+      cmd:  'metadata-json-lint',
+    )
+  end
+
+  describe '.spinner_text' do
+    subject(:spinner_text) { described_class.spinner_text(targets) }
+
+    context 'when given a relative path to the target' do
+      let(:targets) { ['foo/metadata.json'] }
+
+      it 'includes the path to the target in the spinner text' do
+        expect(spinner_text).to match(%r{checking metadata \(#{Regexp.escape(targets.first)}\)}i)
+      end
+    end
+
+    context 'when given an absolute path to the target' do
+      let(:targets) do
+        if Gem.win_platform?
+          ['C:/path/to/module/metadata.json']
+        else
+          ['/path/to/module/metadata.json']
+        end
+      end
+
+      before(:each) do
+        pwd = Gem.win_platform? ? 'C:/path/to/module' : '/path/to/module'
+        allow(Pathname).to receive(:pwd).and_return(Pathname.new(pwd))
+      end
+
+      it 'includes the path to the target relative to the PWD in the spinner text' do
+        expect(spinner_text).to match(%r{checking metadata \(metadata\.json\)}i)
+      end
+    end
+  end
+
+  it_behaves_like 'it accepts metadata.json targets'
+
+  describe '.parse_options' do
+    subject(:command_args) { described_class.parse_options(options, targets) }
+
+    let(:options) { {} }
+    let(:targets) { %w[target1 target2.json] }
+
+    it 'sets the output format as JSON' do
+      expect(command_args.first(2)).to eq(['--format', 'json'])
+    end
+
+    it 'appends the targets to the command arguments' do
+      expect(command_args.last(targets.count)).to eq(targets)
+    end
+
+    context 'when auto-correct is enabled' do
+      let(:options) { { auto_correct: true } }
+
+      it 'has no effect' do
+        expect(command_args).to eq(['--format', 'json'].concat(targets))
+      end
+    end
+  end
+
+  describe '.parse_output' do
+    subject(:parse_output) do
+      described_class.parse_output(report, { stdout: metadata_json_lint_output }, targets)
+    end
+
+    let(:report) { PDK::Report.new }
+    let(:metadata_json_lint_output) { { result: 'something', errors: errors, warnings: warnings }.to_json }
+    let(:targets) { ['metadata.json'] }
+    let(:errors) { [] }
+    let(:warnings) { [] }
+
+    context 'when passed multiple targets' do
+      let(:targets) { ['metadata.json', 'another.json'] }
+
+      it 'raises an ArgumentError' do
+        expect { parse_output }.to raise_error(ArgumentError, a_string_matching(%r{more than 1 target provided}i))
+      end
+    end
+
+    context 'when metadata-json-lint generates no output' do
+      let(:metadata_json_lint_output) { '' }
+
+      it 'adds a passing event for the target to the report' do
+        expect(report).to receive(:add_event).with(
+          file:     targets.first,
+          source:   described_class.name,
+          state:    :passed,
+          severity: :ok,
+        )
+
+        parse_output
+      end
+    end
+
+    context 'when metadata-json-lint generates bad JSON' do
+      let(:metadata_json_lint_output) { 'some unhandled error' }
+
+      it 'adds an error event for the target to the report' do
+        expect(report).to receive(:add_event).with(
+          file:     targets.first,
+          source:   described_class.name,
+          state:    :error,
+          severity: :error,
+          message:  metadata_json_lint_output,
+        )
+
+        parse_output
+      end
+    end
+
+    context 'when metadata-json-lint finds problems in the target' do
+      let(:errors) { [{ 'msg' => 'some error', 'check' => 'error-check' }] }
+      let(:warnings) { [{ 'msg' => 'some warning', 'check' => 'warning-check' }] }
+
+      it 'adds a failure event to the report for each error' do
+        allow(report).to receive(:add_event)
+
+        errors.each do |error|
+          expect(report).to receive(:add_event).with(
+            file:     targets.first,
+            source:   described_class.name,
+            message:  error['msg'],
+            test:     error['check'],
+            severity: 'error',
+            state:    :failure,
+          )
+        end
+
+        parse_output
+      end
+
+      it 'adds a failure event to the report for each warning' do
+        allow(report).to receive(:add_event)
+
+        warnings.each do |warning|
+          expect(report).to receive(:add_event).with(
+            file:     targets.first,
+            source:   described_class.name,
+            message:  warning['msg'],
+            test:     warning['check'],
+            severity: 'warning',
+            state:    :failure,
+          )
+        end
+
+        parse_output
+      end
+    end
+  end
+
+  describe '.invoke' do
+    let(:targets) { ['metadata.json', 'test.json'] }
+
+    let(:expected_args) do
+      if Gem.win_platform?
+        %w[ruby -W0].concat([described_class.cmd_path, described_class.parse_options({}, [])])
+      else
+        [described_class.cmd_path, described_class.parse_options({}, [])]
+      end
+    end
+
+    let(:command_double) do
+      instance_double(PDK::CLI::Exec::Command,
+                      :context=    => true,
+                      :add_spinner => true,
+                      :execute!    => { stdout: '', stderr: '', exit_code: 0 })
+    end
+
+    before(:each) do
+      allow(PDK::Util::Bundler).to receive(:ensure_binstubs!).with(described_class.cmd)
+      targets.each do |target|
+        allow(File).to receive(:directory?).with(target).and_return(false)
+      end
+    end
+
+    it 'invokes metadata-json-lint once per target' do
+      targets.each do |target|
+        cmd_args = expected_args.dup.flatten << target
+        expect(PDK::CLI::Exec::Command).to receive(:new).with(*cmd_args).and_return(command_double)
+      end
+
+      described_class.invoke(PDK::Report.new, targets: targets)
+    end
   end
 end

spec/pdk/validate/metadata_syntax_spec.rb

@@ -8,80 +8,7 @@
     allow(File).to receive(:directory?).with(module_root).and_return(true)
   end
 
-  describe '.parse_targets' do
-    subject { described_class.parse_targets(targets: targets) }
-
-    context 'when given no targets' do
-      let(:module_metadata_json) { File.join(module_root, 'metadata.json') }
-      let(:glob_pattern) { File.join(module_root, described_class.pattern) }
-      let(:targets) { [] }
-
-      context 'and the module contains a metadata.json file' do
-        before(:each) do
-          allow(Dir).to receive(:glob).with(glob_pattern).and_return(module_metadata_json)
-        end
-
-        it 'returns the path to metadata.json in the module' do
-          is_expected.to eq([module_metadata_json])
-        end
-      end
-
-      context 'and the module does not contain a metadata.json file' do
-        before(:each) do
-          allow(Dir).to receive(:glob).with(glob_pattern).and_return([])
-        end
-
-        it 'returns no targets' do
-          is_expected.to eq([])
-        end
-      end
-    end
-
-    context 'when given specific target files' do
-      let(:targets) { ['target1', 'target2.json'] }
-
-      before(:each) do
-        targets.each do |target|
-          allow(File).to receive(:directory?).with(target).and_return(false)
-        end
-      end
-
-      it 'returns the targets' do
-        is_expected.to eq(targets)
-      end
-    end
-
-    context 'when given a specific target directory' do
-      let(:targets) { [File.join('path', 'to', 'target', 'directory')] }
-      let(:glob_pattern) { File.join(targets.first, described_class.pattern) }
-
-      before(:each) do
-        allow(File).to receive(:directory?).with(targets.first).and_return(true)
-      end
-
-      context 'and the directory contains a metadata.json file' do
-        let(:expected_targets) { [File.join(targets.first, 'metadata.json')] }
-
-        before(:each) do
-          allow(Dir).to receive(:glob).with(glob_pattern).and_return(expected_targets)
-        end
-
-        it 'returns the path to the metadata.json file in the target directory' do
-          is_expected.to eq(expected_targets)
-        end
-      end
-
-      context 'and the directory does not contain a metadata.json file' do
-        before(:each) do
-          allow(Dir).to receive(:glob).with(glob_pattern).and_return([])
-        end
-
-        it 'returns no targets' do
-          is_expected.to eq([])
-        end
-      end
-    end
-  end
+  it_behaves_like 'it accepts metadata.json targets'
 
   describe '.invoke' do
     subject(:return_value) { described_class.invoke(report, targets: targets.map { |r| r[:name] }) }