diff --git a/.rubocop.yml b/.rubocop.yml index 1eff7eb5a..77e819a75 100644 --- a/.rubocop.yml +++ b/.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. diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 74c02f057..a04adb70e 100644 --- a/.rubocop_todo.yml +++ b/.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' diff --git a/lib/pdk/cli/exec.rb b/lib/pdk/cli/exec.rb index 5fe3f7786..1b154ea40 100644 --- a/lib/pdk/cli/exec.rb +++ b/lib/pdk/cli/exec.rb @@ -126,7 +126,7 @@ def run_process! begin @process.start rescue ChildProcess::LaunchError => e - raise PDK::CLI::FatalError, _("Failed to execute '%{command}': %{message}") % { command: @process.argv.join(' '), message: e.message } + raise PDK::CLI::FatalError, _("Failed to execute '%{command}': %{message}") % { command: argv.join(' '), message: e.message } end if timeout diff --git a/lib/pdk/cli/util/option_normalizer.rb b/lib/pdk/cli/util/option_normalizer.rb index 85a285222..d27345225 100644 --- a/lib/pdk/cli/util/option_normalizer.rb +++ b/lib/pdk/cli/util/option_normalizer.rb @@ -7,22 +7,43 @@ def self.comma_separated_list_to_array(list, _options = {}) list.split(',').compact end - # Parse one or more format:target pairs. - # @return [Array] 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 [ArrayObject}>] 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) @@ -30,11 +51,15 @@ def self.parameter_specification(value) 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 } diff --git a/lib/pdk/cli/validate.rb b/lib/pdk/cli/validate.rb index 5bf58ca9e..d5bef0476 100644 --- a/lib/pdk/cli/validate.rb +++ b/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,18 +49,23 @@ 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 } validators.each do |validator| - result = validator.invoke(options) - next unless reports - reports.each do |r| - r.write(result) - end + validator.invoke(report, options) + end + + report_formats.each do |format| + report.send(format[:method], format[:target]) end end end diff --git a/lib/pdk/report.rb b/lib/pdk/report.rb index 48e7be307..9c7f4ffb0 100644 --- a/lib/pdk/report.rb +++ b/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] 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}] 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 diff --git a/lib/pdk/report/event.rb b/lib/pdk/report/event.rb new file mode 100644 index 000000000..22bf2ae51 --- /dev/null +++ b/lib/pdk/report/event.rb @@ -0,0 +1,265 @@ +require 'rexml/document' +require 'pathname' + +module PDK + class Report + class Event + # @return [String] The path to the file that the event is in reference + # to. + attr_reader :file + + # @return [Integer] The line number in the file that the event is in + # reference to. + attr_reader :line + + # @return [Integer] The column number in the line of the file that the + # event is in reference to. + attr_reader :column + + # @return [String] The name of the source of the event (usually the name + # of the validation or testing tool that generated the event). + attr_reader :source + + # @return [String] A freeform String containing a human readable message + # describing the event. + attr_reader :message + + # @return [String] The severity of the event as reported by the + # underlying tool. + attr_reader :severity + + # @return [String] The name of the test that generated the event. + attr_reader :test + + # @return [Symbol] The state of the event. :passed, :failure, :error, or + # :skipped. + attr_reader :state + + # Initailises a new PDK::Report::Event object. + # + # @param data [Hash{Symbol=>Object} + # @option data [String] :file (see #file) + # @option data [Integer] :line (see #line) + # @option data [Integer] :column (see #column) + # @option data [String] :source (see #source) + # @option data [String] :message (see #message) + # @option data [String] :severity (see #severity) + # @option data [String] :test (see #test) + # @option data [Symbol] :state (see #state) + # + # @raise [ArgumentError] (see #sanitise_data) + def initialize(data) + sanitise_data(data).each do |key, value| + instance_variable_set("@#{key}", value) + end + end + + # Checks if the event is the result of a passing test. + # + # @return [Boolean] true if the test passed, otherwise false. + def pass? + state == :passed + end + + # Checks if the event is the result of a test that could not complete due + # to an error. + # + # @return [Boolean] true if the test did not complete, otherwise false. + def error? + state == :error + end + + # Checks if the event is the result of a failing test. + # + # @return [Boolean] true if the test failed, otherwise false. + def failure? + state == :failure + end + + # Checks if the event is the result of test that was not run. + # + # @return [Boolean] true if the test was skipped, otherwise false. + def skipped? + state == :skipped + end + + # Renders the event in a clang style text format. + # + # @return [String] The rendered event. + def to_text + location = [file, line, column].compact.join(':') + + [location, severity, message].compact.join(': ') + end + + # Renders the event as a JUnit XML testcase. + # + # @return [REXML::Element] The rendered event. + def to_junit + testcase = REXML::Element.new('testcase') + testcase.attributes['classname'] = [source, test].compact.join('.') + testcase.attributes['name'] = [file, line, column].compact.join(':') + testcase.attributes['time'] = 0 + + if failure? + failure = REXML::Element.new('failure') + failure.attributes['message'] = severity + failure.text = to_text + testcase.elements << failure + elsif skipped? + testcase.add_element('skipped') + end + + testcase + end + + private + + # Processes the data hash used to initialise the event, validating and + # munging the values as necessary. + # + # @param data [Hash{Symbol => Object}] (see #initialize) + # + # @return [Hash{Symbol => String}] A copy of the data hash passed to the + # method with sanitised values. + # + # @raise [ArgumentError] (see #sanitise_file) + # @raise [ArgumentError] (see #sanitise_state) + # @raise [ArgumentError] (see #sanitise_source) + def sanitise_data(data) + result = data.dup + data.each do |key, value| + key = key.to_sym unless key.is_a?(Symbol) + method = "sanitise_#{key}" + result[key] = send(method, value) if respond_to?(method, true) + end + + result + end + + # Munges and validates the file path used to instantiate the event. + # + # If the path is an absolute path, it will be rewritten so that it is + # relative to the module root instead. + # + # @param value [String] The path to the file that the event is + # describing. + # + # @return [String] The path to the file, relative to the module root. + # + # @raise [ArgumentError] if the value is nil, an empty String, or not + # a String. + def sanitise_file(value) + if value.nil? || (value.is_a?(String) && value.empty?) + raise ArgumentError, _('file not specified') + end + + unless value.is_a?(String) + raise ArgumentError, _('file must be a String') + end + + path = Pathname.new(value) + + if path.absolute? + module_root = Pathname.new(PDK::Util.module_root) + path.relative_path_from(module_root).to_path + else + path.to_path + end + end + + # Munges and validates the state of the event. + # + # The valid event states are: + # :passed - The event represents a passing test. + # :error - The event represents a test that could not be completed due + # to an unexpected error. + # :failure - The event represents a failing test. + # :skipped - The event represents a test that was skipped. + # + # @param value [Symbol, String] The state of the event. If passed as + # a String, it will be turned into a Symbol before validation. + # + # @return [Symbol] The sanitised state type. + # + # @raise [ArgumentError] if the value is nil, an empty String, or not + # a String or Symbol representation of a valid state. + def sanitise_state(value) + if value.nil? || (value.is_a?(String) && value.empty?) + raise ArgumentError, _('state not specified') + end + + value = value.to_sym if value.is_a?(String) + unless value.is_a?(Symbol) + raise ArgumentError, _('state must be a Symbol, not %{type}') % { type: value.class } + end + + valid_states = [:passed, :error, :failure, :skipped] + unless valid_states.include?(value) + raise ArgumentError, _('Invalid state %{state}, valid states are: %{valid}') % { + state: value.inspect, + valid: valid_states.map(&:inspect).join(', '), + } + end + + value + end + + # Validates the source of the event. + # + # @param value [String, Symbol] The name of the source of the event. + # + # @return [String] the value passed to the event, converted to a String + # if necessary. + # + # @raise [ArgumentError] if the value is nil or an empty String. + def sanitise_source(value) + if value.nil? || (value.is_a?(String) && value.empty?) + raise ArgumentError, _('source not specified') + end + + value.to_s + end + + # Munges the line number of the event into an Integer. + # + # @param value [Integer, String, Fixnum] The line number. + # + # @return [Integer] the provided value, converted into an Integer if + # necessary. + def sanitise_line(value) + return nil if value.nil? + + unless [String, Integer, Fixnum].include?(value.class) # rubocop:disable Lint/UnifiedInteger + raise ArgumentError, _('line must be an Integer or a String representation of an Integer') + end + + if value.is_a?(String) && value !~ %r{\A[0-9]+\Z} + raise ArgumentError, _('the line number can only contain the digits 0-9') + end + + value.to_i + end + + # Munges the column number of the event into an Integer. + # + # @param value [Integer, String, Fixnum] The column number. + # + # @return [Integer] the provided value, converted into an Integer if + # necessary. + def sanitise_column(value) + return nil if value.nil? + + unless [String, Integer, Fixnum].include?(value.class) # rubocop:disable Lint/UnifiedInteger + raise ArgumentError, _('column must be an Integer or a String representation of an Integer') + end + + if value.is_a?(String) && value !~ %r{\A[0-9]+\Z} + raise ArgumentError, _('the column number can only contain the digits 0-9') + end + + value.to_i + end + end + end +end diff --git a/lib/pdk/validators/base_validator.rb b/lib/pdk/validators/base_validator.rb index 0d637125f..03a696767 100644 --- a/lib/pdk/validators/base_validator.rb +++ b/lib/pdk/validators/base_validator.rb @@ -18,13 +18,17 @@ def self.parse_options(_options, targets) targets end - def self.invoke(options = {}) + def self.invoke(report, options = {}) targets = parse_targets(options) cmd_options = parse_options(options, targets) PDK.logger.debug(_('Running %{cmd} with options: %{options}') % { cmd: cmd, options: cmd_options }) result = PDK::CLI::Exec.execute(cmd, *cmd_options) - result + begin + parse_output(report, JSON.parse(result[:stdout])) + rescue JSON::ParserError + # error messages to stdout :( + end end end end diff --git a/lib/pdk/validators/puppet/puppet_lint.rb b/lib/pdk/validators/puppet/puppet_lint.rb index 5de23c052..d2fc760bb 100644 --- a/lib/pdk/validators/puppet/puppet_lint.rb +++ b/lib/pdk/validators/puppet/puppet_lint.rb @@ -14,15 +14,30 @@ def self.cmd 'puppet-lint' end - def self.parse_options(options, targets) - cmd_options = [] + def self.pattern + '**/*.pp' + end - if options[:format] && options[:format] == 'junit' - cmd_options << '--json' - end + def self.parse_options(_options, targets) + cmd_options = ['--json'] cmd_options.concat(targets) end + + def self.parse_output(report, json_data) + json_data.each do |offense| + report.add_event( + file: offense['path'], + source: 'puppet-lint', + line: offense['line'], + column: offense['column'], + message: offense['message'], + test: offense['check'], + severity: offense['kind'], + state: :failure, + ) + end + end end end end diff --git a/lib/pdk/validators/puppet_validator.rb b/lib/pdk/validators/puppet_validator.rb index a1a41396b..9b6920a65 100644 --- a/lib/pdk/validators/puppet_validator.rb +++ b/lib/pdk/validators/puppet_validator.rb @@ -1,8 +1,8 @@ require 'pdk' require 'pdk/cli/exec' require 'pdk/validators/base_validator' -require 'pdk/validators/puppet/puppet_lint.rb' -require 'pdk/validators/puppet/puppet_parser.rb' +require 'pdk/validators/puppet/puppet_lint' +require 'pdk/validators/puppet/puppet_parser' module PDK module Validate @@ -12,16 +12,13 @@ def self.name end def self.puppet_validators - [PuppetLint, PuppetParser] + [PuppetLint] end - def self.invoke(options = {}) - results = {} + def self.invoke(report, options = {}) puppet_validators.each do |validator| - output = validator.invoke(options) - results.merge!(validator.name.to_s => output) + validator.invoke(report, options) end - results end end end diff --git a/lib/pdk/validators/ruby/rubocop.rb b/lib/pdk/validators/ruby/rubocop.rb index e30ff1f68..9cc37611e 100644 --- a/lib/pdk/validators/ruby/rubocop.rb +++ b/lib/pdk/validators/ruby/rubocop.rb @@ -14,15 +14,40 @@ def self.cmd 'rubocop' end - def self.parse_options(options, targets) - cmd_options = if options[:format] && options[:format] == 'junit' - ['--format', 'json'] - else - ['--format', 'clang'] - end + def self.parse_options(_options, targets) + cmd_options = ['--format', 'json'] cmd_options.concat(targets) end + + def self.parse_output(report, json_data) + return unless json_data.key?('files') + + json_data['files'].each do |file_info| + next unless file_info.key?('offenses') + result = { + file: file_info['path'], + source: 'rubocop', + } + + if file_info['offenses'].empty? + report.add_event(result.merge(state: :passed, severity: :ok)) + else + file_info['offenses'].each do |offense| + report.add_event( + result.merge( + line: offense['location']['line'], + column: offense['location']['column'], + message: offense['message'], + severity: offense['severity'], + test: offense['cop'], + state: :failure, + ), + ) + end + end + end + end end end end diff --git a/lib/pdk/validators/ruby_validator.rb b/lib/pdk/validators/ruby_validator.rb index 9bb12c11f..ce105aaef 100644 --- a/lib/pdk/validators/ruby_validator.rb +++ b/lib/pdk/validators/ruby_validator.rb @@ -14,13 +14,10 @@ def self.ruby_validators [Rubocop] end - def self.invoke(options = {}) - results = {} + def self.invoke(report, options = {}) ruby_validators.each do |validator| - output = validator.invoke(options) - results.merge!(validator.name.to_s => output) + validator.invoke(report, options) end - results end end end diff --git a/spec/acceptance/validate_ruby_spec.rb b/spec/acceptance/validate_ruby_spec.rb new file mode 100644 index 000000000..86853adaa --- /dev/null +++ b/spec/acceptance/validate_ruby_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper_acceptance' + +describe 'When running ruby validation' do + before(:all) do + system('pdk new module foo --skip-interview') || raise + + # Remove these files before the test to prevent them from influencing the + # test results + FileUtils.rm(File.join('foo', 'Gemfile')) + FileUtils.rm(File.join('foo', 'spec', 'spec_helper.rb')) + end + + after(:all) do + FileUtils.rm_rf('foo') + end + + context 'on a file with a style issue' do + describe command('pdk validate ruby') do + before(:each) do + File.open(File.join('foo', 'failing_test.rb'), 'w') do |f| + f.puts 'foo = {:bar => :baz}' + end + Dir.chdir('foo') + end + + after(:each) do + Dir.chdir('..') + FileUtils.rm(File.join('foo', 'failing_test.rb')) + end + + its(:stdout) { is_expected.to match(%r{^failing_test.rb.*hash syntax}) } + its(:stderr) { is_expected.to match(%r{\A\Z}) } + end + end + + context 'on a perfect ruby file' do + describe command('pdk validate ruby') do + before(:each) do + File.open(File.join('foo', 'passing_test.rb'), 'w') do |f| + f.puts 'exit 0' + end + Dir.chdir('foo') + end + + after(:each) do + Dir.chdir('..') + FileUtils.rm(File.join('foo', 'passing_test.rb')) + end + + its(:stdout) { is_expected.to match(%r{\A\Z}) } + its(:stderr) { is_expected.to match(%r{\A\Z}) } + end + end +end diff --git a/spec/cli/option_normalizer_spec.rb b/spec/cli/option_normalizer_spec.rb index 5e479a89b..88f26b1e4 100644 --- a/spec/cli/option_normalizer_spec.rb +++ b/spec/cli/option_normalizer_spec.rb @@ -13,30 +13,50 @@ end context 'when parsing report formats and targets' do - context 'when a single format is specified' do - it 'returns a single Report with the default target when target is not specified' do + context 'and given a single format with no target' do + it 'returns a single format specification with no target' do reports = described_class.report_formats(['text']) expect(reports.length).to eq(1) - expect(reports[0].instance_variable_get(:@path)).to eq('stdout') - expect(reports[0].instance_variable_get(:@format)).to eq('text') + expect(reports[0][:method]).to eq(:to_text) + expect(reports[0][:target]).to eq(nil) end + end - it 'returns a single Report with the specified target when provided' do + context 'and given a single format with a target' do + it 'returns a single format specification with target' do reports = described_class.report_formats(['text:foo.txt']) expect(reports.length).to eq(1) - expect(reports[0].instance_variable_get(:@path)).to eq('foo.txt') - expect(reports[0].instance_variable_get(:@format)).to eq('text') + expect(reports[0][:method]).to eq(:to_text) + expect(reports[0][:target]).to eq('foo.txt') + end + + context 'and the target is stdout' do + it 'returns the $stdout IO object as the target' do + reports = described_class.report_formats(['text:stdout']) + expect(reports.length).to eq(1) + expect(reports[0][:method]).to eq(:to_text) + expect(reports[0][:target]).to eq($stdout) + end + end + + context 'and the target is stderr' do + it 'returns the $stderr IO object as the target' do + reports = described_class.report_formats(['text:stderr']) + expect(reports.length).to eq(1) + expect(reports[0][:method]).to eq(:to_text) + expect(reports[0][:target]).to eq($stderr) + end end end - context 'when multiple report formats are specified' do - it 'returns two Reports with default and specified targets where appropriate' do + context 'and multiple report formats are specified' do + it 'returns multiple format specifications with targets when appropriate' do reports = described_class.report_formats(['text', 'junit:foo.junit']) expect(reports.length).to eq(2) - expect(reports[0].instance_variable_get(:@path)).to eq('stdout') - expect(reports[0].instance_variable_get(:@format)).to eq('text') - expect(reports[1].instance_variable_get(:@path)).to eq('foo.junit') - expect(reports[1].instance_variable_get(:@format)).to eq('junit') + expect(reports[0][:method]).to eq(:to_text) + expect(reports[0][:target]).to eq(nil) + expect(reports[1][:method]).to eq(:to_junit) + expect(reports[1][:target]).to eq('foo.junit') end end end diff --git a/spec/cli/validate_spec.rb b/spec/cli/validate_spec.rb index 6601a2d1d..92fbdebd9 100644 --- a/spec/cli/validate_spec.rb +++ b/spec/cli/validate_spec.rb @@ -10,7 +10,7 @@ context 'when no arguments or options are provided' do it 'invokes each validator with no report and no options' do validators.each do |validator| - expect(validator).to receive(:invoke).with({}) + expect(validator).to receive(:invoke).with(instance_of(PDK::Report), {}) end expect(logger).to receive(:info).with('Running all available validators...') @@ -33,8 +33,8 @@ end context 'when validators are provided as arguments' do - it 'onlies invoke a single validator when only one is provided' do - expect(PDK::Validate::Metadata).to receive(:invoke).with({}) + it 'only invokes a single validator when only one is provided' do + expect(PDK::Validate::Metadata).to receive(:invoke).with(instance_of(PDK::Report), {}) validators.reject { |r| r == PDK::Validate::Metadata }.each do |validator| expect(validator).not_to receive(:invoke) @@ -52,7 +52,7 @@ ] invoked_validators.each do |validator| - expect(validator).to receive(:invoke).with({}) + expect(validator).to receive(:invoke).with(instance_of(PDK::Report), {}) end (validators | invoked_validators).each do |validator| @@ -66,7 +66,7 @@ it 'warns about unknown validators' do expect(logger).to receive(:warn).with("Unknown validator 'bad-val'. Available validators: #{validator_names}") - expect(PDK::Validate::PuppetValidator).to receive(:invoke).with({}) + expect(PDK::Validate::PuppetValidator).to receive(:invoke).with(instance_of(PDK::Report), {}) expect { PDK::CLI.run(['validate', 'puppet,bad-val']) @@ -76,7 +76,7 @@ context 'when targets are provided as arguments' do pending '`validate` not implemented yet' it 'invokes the specified validator with the target as an option' do - expect(PDK::Validate::Metadata).to receive(:invoke).with(targets: ['lib/', 'manifests/']) + expect(PDK::Validate::Metadata).to receive(:invoke).with(instance_of(PDK::Report), targets: ['lib/', 'manifests/']) expect { PDK::CLI.run(['validate', 'metadata', 'lib/', 'manifests/']) @@ -89,7 +89,7 @@ pending '`validate` not implemented yet' it 'invokes all validators with the target as an option' do validators.each do |validator| - expect(validator).to receive(:invoke).with(targets: ['lib/', 'manifests/']) + expect(validator).to receive(:invoke).with(instance_of(PDK::Report), targets: ['lib/', 'manifests/']) end expect(logger).to receive(:info).with('Running all available validators...') diff --git a/spec/pdk/report/event_spec.rb b/spec/pdk/report/event_spec.rb new file mode 100644 index 000000000..298bd4793 --- /dev/null +++ b/spec/pdk/report/event_spec.rb @@ -0,0 +1,475 @@ +require 'spec_helper' + +describe PDK::Report::Event do + subject(:junit_event) { event.to_junit } + + subject(:text_event) { event.to_text } + + subject { event } + + let(:event) { described_class.new(default_data.merge(data)) } + + let(:default_data) do + { + file: 'testfile.rb', + source: 'test-validator', + state: :passed, + } + end + + let(:data) { {} } + + context 'when validating arguments' do + context 'and passed an absolute path to the file being tested' do + before(:each) do + expect(PDK::Util).to receive(:module_root).and_return('/path/to/test/module') + end + + let(:data) do + { + file: '/path/to/test/module/lib/some/file.rb', + } + end + + it 'converts the path to one relative to the module root' do + is_expected.to have_attributes(file: 'lib/some/file.rb') + end + end + + context 'and not passed a file path' do + let(:data) do + { + file: nil, + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{file not specified}) + end + end + + context 'and passed an empty string as the file path' do + let(:data) do + { + file: '', + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{file not specified}) + end + end + + context 'and not passed a source' do + let(:data) do + { + source: nil, + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{source not specified}) + end + end + + context 'and passed an empty string as the source' do + let(:data) do + { + source: '', + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{source not specified}) + end + end + + context 'and not passed a state' do + let(:data) do + { + state: nil, + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{state not specified}) + end + end + + context 'and passed an empty string as the state' do + let(:data) do + { + state: '', + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{state not specified}) + end + end + + methods = [:pass, :error, :failure, :skipped] + + { + passed: :pass, + error: :error, + failure: :failure, + skipped: :skipped, + }.each do |state_sym, state_method| + [state_sym, state_sym.to_s].each do |state| + context "and passed #{state.inspect} as the state" do + let(:data) do + { + state: state, + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + + methods.each do |method| + if method == state_method + it { is_expected.to send("be_#{method}") } + else + it { is_expected.not_to send("be_#{method}") } + end + end + end + end + end + + context 'and passed a state that is not a String or Symbol' do + let(:data) do + { + state: %r{passed}, + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{state must be a Symbol}) + end + end + + context 'and passed an unknown state' do + let(:data) do + { + state: :maybe, + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{Invalid state :maybe}) + end + end + + context 'and passed an Integer line number' do + let(:data) do + { + line: 123, + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + end + + context 'and passed a String line number containing only digits' do + let(:data) do + { + line: '123', + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + + it 'converts the line number to an Integer' do + expect(event).to have_attributes(line: 123) + end + end + + context 'and passed a String line number containing non-digit characters' do + let(:data) do + { + line: 'line 123', + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{only contain the digits 0-9}) + end + end + + context 'and passed a line number that is not a String or Integer' do + let(:data) do + { + line: [123], + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{must be an Integer or a String}) + end + end + + context 'and passed a nil line number' do + let(:data) do + { + line: nil, + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + + it 'sets the line number to nil' do + expect(event).to have_attributes(line: nil) + end + end + + context 'and passed an Integer column number' do + let(:data) do + { + column: 456, + } + end + + it 'does not raise an ArgumentError' do + expect { event }.not_to raise_error + end + end + + context 'and passed a String column number containing only digits' do + let(:data) do + { + column: 456, + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + + it 'converts the column number to an Integer' do + expect(event).to have_attributes(column: 456) + end + end + + context 'and passed a String column number containing non-digit characters' do + let(:data) do + { + column: 'column 456', + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{only contain the digits 0-9}) + end + end + + context 'and passed a column numbre that is not a String or Integer' do + let(:data) do + { + column: [456], + } + end + + it 'raises an ArgumentError' do + expect { event }.to raise_error(ArgumentError, %r{must be an Integer or a String}) + end + end + + context 'and passed a nil column number' do + let(:data) do + { + column: nil, + } + end + + it 'does not raise an error' do + expect { event }.not_to raise_error + end + + it 'sets the column number to nil' do + expect(event).to have_attributes(column: nil) + end + end + end + + context 'when generating text output' do + it 'contains the file name at the start of the string' do + expect(text_event).to match(%r{\Atestfile\.rb}) + end + + context 'and a line number is provided' do + let(:data) do + { + line: 123, + } + end + + it 'appends the line number to the file name' do + expect(text_event).to match(%r{\Atestfile\.rb:123}) + end + + context 'and a column number is provided' do + let(:data) do + { + line: 123, + column: 456, + } + end + + it 'appends the column number to the line number' do + expect(text_event).to match(%r{\Atestfile\.rb:123:456}) + end + end + end + + context 'and a severity is provided' do + let(:data) do + { + severity: 'ok', + } + end + + it 'includes the message after the file name' do + expect(text_event).to match(%r{\Atestfile\.rb: ok\Z}) + end + end + + context 'and a message is provided' do + let(:data) do + { + message: 'test message', + } + end + + it 'includes the message at the end of the string' do + expect(text_event).to match(%r{\Atestfile\.rb: test message\Z}) + end + + context 'and a severity is provided' do + let(:data) do + { + message: 'test message', + severity: 'critical', + } + end + + it 'includes the severity before the message' do + expect(text_event).to match(%r{\Atestfile\.rb: critical: test message\Z}) + end + end + end + end + + context 'when generating junit output' do + it 'sets the classname attribute to the event source' do + expect(junit_event.attributes['classname']).to eq('test-validator') + end + + context 'and a test name is provided' do + let(:data) do + { + test: 'test-method', + } + end + + it 'adds the test name to the classname attribute' do + expect(junit_event.attributes['classname']).to eq('test-validator.test-method') + end + end + + it 'sets the testcase name to the file name' do + expect(junit_event.attributes['name']).to eq('testfile.rb') + end + + context 'and a line number is provided' do + let(:data) do + { + line: 123, + } + end + + it 'adds the line number to the testcase name' do + expect(junit_event.attributes['name']).to eq('testfile.rb:123') + end + end + + context 'and a column number is provided' do + let(:data) do + { + column: 456, + } + end + + it 'adds the column number to the testcase name' do + expect(junit_event.attributes['name']).to eq('testfile.rb:456') + end + end + + context 'and both line and column numbers are provided' do + let(:data) do + { + line: 123, + column: 456, + } + end + + it 'adds the line number and then the column number to the testcase name' do + expect(junit_event.attributes['name']).to eq('testfile.rb:123:456') + end + end + + context 'for a passing test case' do + it 'creates an xml testcase with no children' do + expect(junit_event.children).to eq([]) + end + end + + context 'for a skipped test case' do + let(:data) do + { + state: :skipped, + } + end + + it 'creates an xml testcase with a "skipped" child element' do + expect(junit_event.children.length).to eq(1) + expect(junit_event.children.first.name).to eq('skipped') + expect(junit_event.children.first.children).to eq([]) + expect(junit_event.children.first.attributes).to eq({}) + end + end + + context 'for a failing test case' do + let(:data) do + { + state: :failure, + line: 123, + column: 456, + severity: 'critical', + message: 'some message', + } + end + + it 'creates an xml testcase with a failure child element' do + expect(junit_event.children.length).to eq(1) + expect(junit_event.children.first.name).to eq('failure') + end + + it 'sets the message attribute to the severity' do + expect(junit_event.children.first.attributes['message']).to eq('critical') + end + + it 'puts a textual representation of the event into the failure element' do + expect(junit_event.children.first.text).to eq(text_event) + end + end + end +end diff --git a/spec/pdk/report_spec.rb b/spec/pdk/report_spec.rb new file mode 100644 index 000000000..abf5c8373 --- /dev/null +++ b/spec/pdk/report_spec.rb @@ -0,0 +1,15 @@ +require 'spec_helper' +require 'tmpdir' + +describe PDK::Report do + %w[junit text].each do |report_format| + it "can format the report as #{report_format}" do + expect(described_class.formats).to include(report_format) + is_expected.to respond_to("to_#{report_format}") + end + end + + it 'defaults to the text format' do + expect(described_class.default_format).to eq(:to_text) + end +end diff --git a/spec/report_spec.rb b/spec/report_spec.rb deleted file mode 100644 index 1fcd884f6..000000000 --- a/spec/report_spec.rb +++ /dev/null @@ -1,38 +0,0 @@ -require 'spec_helper' -require 'tmpdir' - -describe PDK::Report do - def tmpdir - @tmpdir ||= Dir.mktmpdir - end - - after(:each) do - FileUtils.remove_entry_secure(@tmpdir) if @tmpdir - end - - it 'includes formats junit and text' do - expect(described_class.formats).to eq(%w[junit text]) - end - - it 'has a default format of junit' do - expect(described_class.default_format).to eq('text') - end - - context 'when no format is specified' do - let(:report) { described_class.new(File.join(tmpdir, 'report.txt')) } - - it 'instantiates its format to text' do - expect(report).to receive(:prepare_text).with('cmd output') - report.write('cmd output') - end - end - - context 'when a format is specified' do - let(:report) { described_class.new(File.join(tmpdir, 'report.xml'), 'junit') } - - it 'instantiates its format to junit' do - expect(report).to receive(:prepare_junit).with('cmd output') - report.write('cmd output') - end - end -end