Please review pull request #149: (#12613) Add Pending Test Method opened by (justinstoller)

Description:

This patch adds a pending_test method to the dsl, prints total and average test times in the summaries, and many minor clean ups to the TestCase and TestSuite classes.

Minor cleanups include:
move Puppet specific helpers to a dedicated module,
declaring blocks that are yielded to in the method signatures,
forcing nearly every line to stay under 80 characters,
removing duplicated means of county test_case's status,
removing unneeded error checking, and
factoring out needed error checking into its own method.

  • Opened: Mon Feb 20 19:06:37 UTC 2012
  • Based on: puppetlabs:master (3b758e933b43488cf9fab30bf278547bdb86f3d6)
  • Requested merge: justinstoller:bug/master/12613-pending_test_method (3a7af04ac27e562912288723075a5caba6df7e11)

Diff follows:

diff --git a/lib/puppet_commands.rb b/lib/puppet_commands.rb
new file mode 100644
index 0000000..d4459ff
--- /dev/null
+++ b/lib/puppet_commands.rb
@@ -0,0 +1,180 @@
+module PuppetCommands
+
+  def facter(*args)
+    FacterCommand.new(*args)
+  end
+
+  def puppet(*args)
+    PuppetCommand.new(*args)
+  end
+
+  def puppet_resource(*args)
+    PuppetCommand.new(:resource,*args)
+  end
+
+  def puppet_doc(*args)
+    PuppetCommand.new(:doc,*args)
+  end
+
+  def puppet_kick(*args)
+    PuppetCommand.new(:kick,*args)
+  end
+
+  def puppet_cert(*args)
+    PuppetCommand.new(:cert,*args)
+  end
+
+  def puppet_apply(*args)
+    PuppetCommand.new(:apply,*args)
+  end
+
+  def puppet_master(*args)
+    PuppetCommand.new(:master,*args)
+  end
+
+  def puppet_agent(*args)
+    PuppetCommand.new(:agent,*args)
+  end
+
+  def puppet_filebucket(*args)
+    PuppetCommand.new(:filebucket,*args)
+  end
+
+  def host_command(command_string)
+    HostCommand.new(command_string)
+  end
+
+  # method apply_manifest_on
+  # runs a 'puppet apply' command on a remote host
+  # parameters:
+  # [host] an instance of Host which contains the info about the host that this command should be run on
+  # [manifest] a string containing a puppet manifest to apply
+  # [options] an optional hash containing options; legal values include:
+  #   :acceptable_exit_codes => an array of integer exit codes that should be considered acceptable.  an error will be
+  #     thrown if the exit code does not match one of the values in this list.
+  #   :parseonly => any value.  If this key exists in the Hash, the "--parseonly" command line parameter will be
+  #     passed to the 'puppet apply' command.
+  #   :environment => a Hash containing string->string key value pairs.  These will be treated as extra environment
+  #     variables that should be set before running the puppet command.
+  # [&block] this method will yield to a block of code passed by the caller; this can be used for additional validation,
+  #     etc.
+  def apply_manifest_on(host,manifest,options={},&block)
+    _on_options_ = {:stdin => manifest + "\n"}
+    on_options[:acceptable_exit_codes] = options.delete(:acceptable_exit_codes) if options.keys.include?(:acceptable_exit_codes)
+    args = ["--verbose"]
+    args << "--parseonly" if options[:parseonly]
+
+    # Not really thrilled with this implementation, might want to improve it later.  Basically, there is a magic
+    # trick in the constructor of PuppetCommand which allows you to pass in a Hash for the last value in the *args
+    # Array; if you do so, it will be treated specially.  So, here we check to see if our caller passed us a hash
+    # of environment variables that they want to set for the puppet command.  If so, we set the final value of
+    # *args to a new hash with just one entry (the value of which is our environment variables hash)
+    args << { :environment => options[:environment]} if options.has_key?(:environment)
+
+    on host, puppet_apply(*args), on_options, &block
+  end
+
+  def run_agent_on(host, arg='--no-daemonize --verbose --onetime --test', options={}, &block)
+    if host.is_a? Array
+      host.each { |h| run_agent_on h, arg, options, &block }
+    else
+      on host, puppet_agent(arg), options, &block
+    end
+  end
+
+  def run_cron_on(host, action, user, entry="", &block)
+    platform = host['platform']
+    if platform.include? 'solaris'
+      case action
+        when :list   then args = '-l'
+        when :remove then args = '-r'
+        when :add
+          on(host, "echo '#{entry}' > /var/spool/cron/crontabs/#{user}", &block)
+      end
+    else         # default for GNU/Linux platforms
+      case action
+        when :list   then args = '-l -u'
+        when :remove then args = '-r -u'
+        when :add
+           on(host, "echo '#{entry}' > /tmp/#{user}.cron && crontab -u #{user} /tmp/#{user}.cron", &block)
+      end
+    end
+
+    if args
+      case action
+        when :list, :remove then on(host, "crontab #{args} #{user}", &block)
+      end
+    end
+  end
+
+  # This method performs the following steps:
+  # 1. issues start command for puppet master on specified host
+  # 2. polls until it determines that the master has started successfully
+  # 3. yields to a block of code passed by the caller
+  # 4. runs a "kill" command on the master's pid (on the specified host)
+  # 5. polls until it determines that the master has shut down successfully.
+  #
+  # Parameters:
+  # [host] the master host
+  # [arg] a string containing all of the command line arguments that you would like for the puppet master to
+  #     be started with.  Defaults to '--daemonize'.  NOTE: the following values will be added to the argument list
+  #     if they are not explicitly set in your 'args' parameter:
+  # * --daemonize
+  # * --logdest="#{host['puppetvardir']}/log/puppetmaster.log"
+  # * --dns_alt_names="puppet, $(hostname -s), $(hostname -f)"
+  def with_master_running_on(host, arg='--daemonize', &block)
+    # they probably want to run with daemonize.  If they pass some other arg/args but forget to re-include
+    # daemonize, we'll check and make sure they didn't explicitly specify "no-daemonize", and, failing that,
+    # we'll add daemonize to the arg string
+    if (arg !~ /(?:--daemonize)|(?:--no-daemonize)/) then arg << " --daemonize" end
+
+    if (arg !~ /--logdest/) then arg << " --logdest=\"#{master['puppetvardir']}/log/puppetmaster.log\"" end
+    if (arg !~ /--dns_alt_names/) then arg << " --dns_alt_names=\"puppet, $(hostname -s), $(hostname -f)\"" end
+
+    on hosts, host_command('rm -rf #{host["puppetpath"]}/ssl')
+    agents.each do |agent|
+      if vardir = agent['puppetvardir']
+        # we want to remove everything except the log directory
+        on agent, "if [ -e \"#{vardir}\" ]; then for f in #{vardir}/*; do if [ \"$f\" != \"#{vardir}/log\" ]; then rm -rf \"$f\"; fi; done; fi"
+      end
+    end
+
+    on host, puppet_master('--configprint pidfile')
+    pidfile = stdout.chomp
+    on host, puppet_master(arg)
+    poll_master_until(host, :start)
+    master_started = true
+    yield if block
+  ensure
+    if master_started
+      on host, "kill $(cat #{pidfile})"
+      poll_master_until(host, :stop)
+    end
+  end
+
+  def poll_master_until(host, verb)
+    timeout = 30
+    verb_exit_codes = {:start => 0, :stop => 7}
+
+    Log.debug "Wait for master to #{verb}"
+
+    agent = agents.first
+    wait_start = Time.now
+    done = false
+
+    until done or Time.now - wait_start > timeout
+      on(agent, "curl -k https://#{master}:8140 >& /dev/null", :acceptable_exit_codes => (0..255))
+      done = exit_code == verb_exit_codes[verb]
+      sleep 1 unless done
+    end
+
+    wait_finish = Time.now
+    elapsed = wait_finish - wait_start
+
+    if done
+      Log.debug "Slept for #{elapsed} seconds waiting for Puppet Master to #{verb}"
+    else
+      Log.error "Puppet Master failed to #{verb} after #{elapsed} seconds"
+    end
+  end
+end
diff --git a/lib/test_case.rb b/lib/test_case.rb
index 1032ca5..8de615f 100644
--- a/lib/test_case.rb
+++ b/lib/test_case.rb
@@ -3,11 +3,16 @@ class TestCase
   require 'tempfile'
   require 'benchmark'
   require 'stringio'
+  require 'lib/puppet_commands'
 
   include Test::Unit::Assertions
+  include PuppetCommands
+
+  class PendingTest < Exception; end
+
+  attr_reader :version, :config, :options, :path, :fail_flag, :usr_home,
+              :test_status, :exception, :runtime, :result
 
-  attr_reader :version, :config, :options, :path, :fail_flag, :usr_home, :test_status, :exception
-  attr_reader :runtime
   def initialize(hosts, config, options={}, path=nil)
     @version = config['VERSION']
     @config  = config['CONFIG']
@@ -31,6 +36,8 @@ def run_test
             rescue Test::Unit::AssertionFailedError => e
               @test_status = :fail
               @exception   = e
+            rescue PendingTest
+              @test_status = :pending
             rescue StandardError, ScriptError => e
               Log.error(e.inspect)
               e.backtrace.each { |line| Log.error(line) }
@@ -53,78 +60,58 @@ def to_hash
     end
     hash
   end
-  #
-  # Identify hosts
-  #
-  def hosts(desired_role=nil)
-    @hosts.select { |host| desired_role.nil? or host['roles'].include?(desired_role) }
-  end
-  def agents
-    hosts 'agent'
-  end
-  def master
-    masters = hosts 'master'
-    fail "There must be exactly one master" unless masters.length == 1
-    masters.first
-  end
-  def dashboard
-    dashboards = hosts 'dashboard'
-    Log.warn "There is no dashboard host configured" if dashboards.empty?
-    fail "Cannot have more than one dashboard host" if dashboards.length > 1
-    dashboards.first
+
+  def with_standard_output_to_logs(&block)
+    stdout = ''
+    old_stdout = $stdout
+    $stdout = StringIO.new(stdout, 'w')
+
+    stderr = ''
+    old_stderr = $stderr
+    $stderr = StringIO.new(stderr, 'w')
+
+    result = yield if block_given?
+
+    $stdout = old_stdout
+    $stderr = old_stderr
+
+    stdout.each { |line| Log.notify(line) }
+    stderr.each { |line| Log.warn(line) }
+
+    return result
   end
+
   #
-  # Annotations
+  # Test Structure
   #
-  def step(step_name,&block)
+  def step(step_name, &block)
     Log.notify "  * #{step_name}"
     yield if block
   end
 
-  def test_name(test_name,&block)
+  def test_name(test_name, &block)
     Log.notify test_name
     yield if block
   end
-  #
-  # Basic operations
-  #
-  attr_reader :result
-  def on(host, command, options={}, &block)
-    if command.is_a? String
-      command = Command.new(command)
-    end
-    if host.is_a? Array
-      host.map { |h| on h, command, options, &block }
-    else
-      @result = command.exec(host, options)
-
-      # Also, let additional checking be performed by the caller.
-      yield if block_given?
-
-      return @result
-    end
-  end
-
-  def scp_to(host,from_path,to_path,options={})
-    if host.is_a? Array
-      host.each { |h| scp_to h,from_path,to_path,options }
-    else
-      @result = host.do_scp(from_path, to_path)
-      result.log
-      raise "scp exited with #{result.exit_code}" if result.exit_code != 0
-    end
-  end
 
   def pass_test(msg)
     Log.notify msg
   end
+
   def skip_test(msg)
     Log.notify "Skip: #{msg}"
     @test_status = :skip
   end
+
   def fail_test(msg)
     flunk(msg + "\n" + Log.pretty_backtrace() + "\n")
   end
+
+  def pending_test(msg = "WIP: #{@test_name}")
+    Log.warn msg
+    raise PendingTest
+  end
+
   #
   # result access
   #
@@ -132,199 +119,43 @@ def stdout
     return nil if result.nil?
     result.stdout
   end
+
   def stderr
     return nil if result.nil?
     result.stderr
   end
+
   def exit_code
     return nil if result.nil?
     result.exit_code
   end
+
   #
-  # Macros
+  # Networking Helpers
   #
-
-  def facter(*args)
-    FacterCommand.new(*args)
-  end
-
-  def puppet(*args)
-    PuppetCommand.new(*args)
-  end
-
-  def puppet_resource(*args)
-    PuppetCommand.new(:resource,*args)
-  end
-
-  def puppet_doc(*args)
-    PuppetCommand.new(:doc,*args)
-  end
-
-  def puppet_kick(*args)
-    PuppetCommand.new(:kick,*args)
-  end
-
-  def puppet_cert(*args)
-    PuppetCommand.new(:cert,*args)
-  end
-
-  def puppet_apply(*args)
-    PuppetCommand.new(:apply,*args)
-  end
-
-  def puppet_master(*args)
-    PuppetCommand.new(:master,*args)
-  end
-
-  def puppet_agent(*args)
-    PuppetCommand.new(:agent,*args)
-  end
-
-  def puppet_filebucket(*args)
-    PuppetCommand.new(:filebucket,*args)
-  end
-
-  def host_command(command_string)
-    HostCommand.new(command_string)
-  end
-
-  # method apply_manifest_on
-  # runs a 'puppet apply' command on a remote host
-  # parameters:
-  # [host] an instance of Host which contains the info about the host that this command should be run on
-  # [manifest] a string containing a puppet manifest to apply
-  # [options] an optional hash containing options; legal values include:
-  #   :acceptable_exit_codes => an array of integer exit codes that should be considered acceptable.  an error will be
-  #     thrown if the exit code does not match one of the values in this list.
-  #   :parseonly => any value.  If this key exists in the Hash, the "--parseonly" command line parameter will be
-  #     passed to the 'puppet apply' command.
-  #   :environment => a Hash containing string->string key value pairs.  These will be treated as extra environment
-  #     variables that should be set before running the puppet command.
-  # [&block] this method will yield to a block of code passed by the caller; this can be used for additional validation,
-  #     etc.
-  def apply_manifest_on(host,manifest,options={},&block)
-    _on_options_ = {:stdin => manifest + "\n"}
-    on_options[:acceptable_exit_codes] = options.delete(:acceptable_exit_codes) if options.keys.include?(:acceptable_exit_codes)
-    args = ["--verbose"]
-    args << "--parseonly" if options[:parseonly]
-
-    # Not really thrilled with this implementation, might want to improve it later.  Basically, there is a magic
-    # trick in the constructor of PuppetCommand which allows you to pass in a Hash for the last value in the *args
-    # Array; if you do so, it will be treated specially.  So, here we check to see if our caller passed us a hash
-    # of environment variables that they want to set for the puppet command.  If so, we set the final value of
-    # *args to a new hash with just one entry (the value of which is our environment variables hash)
-    args << { :environment => options[:environment]} if options.has_key?(:environment)
-
-    on host, puppet_apply(*args), on_options, &block
-  end
-
-  def run_script_on(host, script, &block)
-    remote_path=File.join("", "tmp", File.basename(script))
-    scp_to host, script, remote_path
-    on host, remote_path, &block
-  end
-
-  def run_agent_on(host, arg='--no-daemonize --verbose --onetime --test', options={}, &block)
+  def on(host, command, options={}, &block)
+    if command.is_a? String
+      command = Command.new(command)
+    end
     if host.is_a? Array
-      host.each { |h| run_agent_on h, arg, options, &block }
+      host.map { |h| on h, command, options, &block }
     else
-      on host, puppet_agent(arg), options, &block
-    end
-  end
-
-  def run_cron_on(host, action, user, entry="", &block)
-    platform = host['platform']
-    if platform.include? 'solaris'
-      case action
-        when :list   then args = '-l'
-        when :remove then args = '-r'
-        when :add
-          on(host, "echo '#{entry}' > /var/spool/cron/crontabs/#{user}", &block)
-      end
-    else         # default for GNU/Linux platforms
-      case action
-        when :list   then args = '-l -u'
-        when :remove then args = '-r -u'
-        when :add
-           on(host, "echo '#{entry}' > /tmp/#{user}.cron && crontab -u #{user} /tmp/#{user}.cron", &block)
-      end
-    end
-   
-    if args
-      case action
-        when :list, :remove then on(host, "crontab #{args} #{user}", &block)
-      end
-    end
-  end
-
-  # This method performs the following steps:
-  # 1. issues start command for puppet master on specified host
-  # 2. polls until it determines that the master has started successfully
-  # 3. yields to a block of code passed by the caller
-  # 4. runs a "kill" command on the master's pid (on the specified host)
-  # 5. polls until it determines that the master has shut down successfully.
-  #
-  # Parameters:
-  # [host] the master host
-  # [arg] a string containing all of the command line arguments that you would like for the puppet master to
-  #     be started with.  Defaults to '--daemonize'.  NOTE: the following values will be added to the argument list
-  #     if they are not explicitly set in your 'args' parameter:
-  # * --daemonize
-  # * --logdest="#{host['puppetvardir']}/log/puppetmaster.log"
-  # * --dns_alt_names="puppet, $(hostname -s), $(hostname -f)"
-  def with_master_running_on(host, arg='--daemonize', &block)
-    # they probably want to run with daemonize.  If they pass some other arg/args but forget to re-include
-    # daemonize, we'll check and make sure they didn't explicitly specify "no-daemonize", and, failing that,
-    # we'll add daemonize to the arg string
-    if (arg !~ /(?:--daemonize)|(?:--no-daemonize)/) then arg << " --daemonize" end
-
-    if (arg !~ /--logdest/) then arg << " --logdest=\"#{master['puppetvardir']}/log/puppetmaster.log\"" end
-    if (arg !~ /--dns_alt_names/) then arg << " --dns_alt_names=\"puppet, $(hostname -s), $(hostname -f)\"" end
+      @result = command.exec(host, options)
 
-    on hosts, host_command('rm -rf #{host["puppetpath"]}/ssl')
-    agents.each do |agent|
-      if vardir = agent['puppetvardir']
-        # we want to remove everything except the log directory
-        on agent, "if [ -e \"#{vardir}\" ]; then for f in #{vardir}/*; do if [ \"$f\" != \"#{vardir}/log\" ]; then rm -rf \"$f\"; fi; done; fi"
-      end
-    end
+      # Also, let additional checking be performed by the caller.
+      yield if block_given?
 
-    on host, puppet_master('--configprint pidfile')
-    pidfile = stdout.chomp
-    on host, puppet_master(arg)
-    poll_master_until(host, :start)
-    master_started = true
-    yield if block
-  ensure
-    if master_started
-      on host, "kill $(cat #{pidfile})"
-      poll_master_until(host, :stop)
+      return @result
     end
   end
 
-  def poll_master_until(host, verb)
-    timeout = 30
-    verb_exit_codes = {:start => 0, :stop => 7}
-
-    Log.debug "Wait for master to #{verb}"
-
-    agent = agents.first
-    wait_start = Time.now
-    done = false
-
-    until done or Time.now - wait_start > timeout
-      on(agent, "curl -k https://#{master}:8140 >& /dev/null", :acceptable_exit_codes => (0..255))
-      done = exit_code == verb_exit_codes[verb]
-      sleep 1 unless done
-    end
-
-    wait_finish = Time.now
-    elapsed = wait_finish - wait_start
-
-    if done
-      Log.debug "Slept for #{elapsed} seconds waiting for Puppet Master to #{verb}"
+  def scp_to(host, from_path, to_path, options={})
+    if host.is_a? Array
+      host.each { |h| scp_to h, from_path, to_path, options }
     else
-      Log.error "Puppet Master failed to #{verb} after #{elapsed} seconds"
+      @result = host.do_scp(from_path, to_path)
+      result.log
+      raise "scp exited with #{result.exit_code}" if result.exit_code != 0
     end
   end
 
@@ -336,24 +167,35 @@ def create_remote_file(hosts, file_path, file_content)
     end
   end
 
-  def with_standard_output_to_logs
-    stdout = ''
-    old_stdout = $stdout
-    $stdout = StringIO.new(stdout, 'w')
-
-    stderr = ''
-    old_stderr = $stderr
-    $stderr = StringIO.new(stderr, 'w')
-
-    result = yield
+  def run_script_on(host, script, &block)
+    remote_path = File.join("", "tmp", File.basename(script))
+    scp_to host, script, remote_path
+    on host, remote_path, &block
+  end
 
-    $stdout = old_stdout
-    $stderr = old_stderr
+  #
+  # Identify hosts
+  #
+  def hosts(desired_role = nil)
+    @hosts.select do |host|
+      desired_role.nil? or host['roles'].include?(desired_role)
+    end
+  end
 
-    stdout.each { |line| Log.notify(line) }
-    stderr.each { |line| Log.warn(line) }
+  def agents
+    hosts 'agent'
+  end
 
-    return result
+  def master
+    masters = hosts 'master'
+    fail "There must be exactly one master" unless masters.length == 1
+    masters.first
   end
 
+  def dashboard
+    dashboards = hosts 'dashboard'
+    Log.warn "There is no dashboard host configured" if dashboards.empty?
+    fail "Cannot have more than one dashboard host" if dashboards.length > 1
+    dashboards.first
+  end
 end
diff --git a/lib/test_suite.rb b/lib/test_suite.rb
index fd8664f..a276712 100644
--- a/lib/test_suite.rb
+++ b/lib/test_suite.rb
@@ -1,6 +1,11 @@
 # -*- coding: utf-8 -*-
 require 'rexml/document'
 
+# This Class is in need of some cleaning up beyond what can be quickly done.
+# Things to keep in mind:
+#   * Global State Change
+#   * File Creation Relative to CWD  -- Should be a config option
+#   * Better Method Documentation
 class TestSuite
   attr_reader :name, :options, :config, :stop_on_error
 
@@ -19,7 +24,9 @@ def initialize(name, hosts, options, config, stop_on_error=FALSE)
       if File.file? root then
         @test_files << root
       else
-        @test_files += Dir[File.join(root, "**/*.rb")].select { |f| File.file?(f) }
+        @test_files += Dir.glob(
+          File.join(root, "**/*.rb")
+        ).select { |f| File.file?(f) }
       end
     end
     fail "no test files found..." if @test_files.empty?
@@ -48,7 +55,8 @@ def run
       duration = Time.now - start
       @test_cases << test_case
 
-      msg = "#{test_file} #{test_case.test_status == :skip ? 'skipp' : test_case.test_status}ed in %.2f seconds" % duration.to_f
+      state = test_case.test_status == :skip ? 'skipp' : test_case.test_status
+      msg = "#{test_file} #{state}ed in %.2f seconds" % duration.to_f
       case test_case.test_status
       when :pass
         Log.success msg
@@ -80,53 +88,51 @@ def run_and_exit_on_failure
     exit 1
   end
 
-  def success?
+  def fail_without_test_run
     fail "you have not run the tests yet" unless @run
+  end
+
+  def success?
+    fail_without_test_run
     sum_failed == 0
   end
+
   def failed?
     !success?
   end
 
   def test_count
-    fail "you have not run the tests yet" unless @run
-    @test_cases.length
+    @test_count ||= @test_cases.length
   end
 
-  def test_errors
-    fail "you have not run the tests yet" unless @run
-    @test_cases.select { |c| c.test_status == :error } .length
+  def passed_tests
+    @passed_tests ||= @test_cases.select { |c| c.test_status == :pass }.length
   end
 
-  def test_failures
-    fail "you have not run the tests yet" unless @run
-    @test_cases.select { |c| c.test_status == :fail } .length
+  def errored_tests
+    @errored_tests ||= @test_cases.select { |c| c.test_status == :error }.length
   end
 
-  def test_skips
-    fail "you have not run the tests yet" unless @run
-    @test_cases.select { |c| c.test_status == :skip} .length
+  def failed_tests
+    @failed_tests ||= @test_cases.select { |c| c.test_status == :fail }.length
+  end
+
+  def skipped_tests
+    @skipped_tests ||= @test_cases.select { |c| c.test_status == :skip }.length
+  end
+
+  def pending_tests
+    @pending_tests ||= @test_cases.select {|c| c.test_status == :pending}.length
   end
 
   private
 
   def sum_failed
-    test_failed=0
-    test_passed=0
-    test_errored=0
-    test_skips=0
-    @test_cases.each do |test_case|
-      case test_case.test_status
-      when :pass  then test_passed  += 1
-      when :fail  then test_failed  += 1
-      when :error then test_errored += 1
-      when :skip  then test_skips   += 1
-      end
-    end
-    test_failed + test_errored
+    @sum_failed ||= failed_tests + errored_tests
   end
 
   def write_junit_xml
+    # This should be a configuration option
     File.directory?('junit') or FileUtils.mkdir('junit')
 
     begin
@@ -136,9 +142,10 @@ def write_junit_xml
       suite = REXML::Element.new('testsuite', doc)
       suite.add_attribute('name',     name)
       suite.add_attribute('tests',    test_count)
-      suite.add_attribute('errors',   test_errors)
-      suite.add_attribute('failures', test_failures)
-      suite.add_attribute('skip',     test_skips)
+      suite.add_attribute('errors',   errored_test)
+      suite.add_attribute('failures', failed_test)
+      suite.add_attribute('skip',     skipped_test)
+      suite.add_attribute('pending',  pending_tests)
 
       @test_cases.each do |test|
         item = REXML::Element.new('testcase', suite)
@@ -147,7 +154,7 @@ def write_junit_xml
         item.add_attribute('time',      test.runtime)
 
         # Did we fail?  If so, report that.
-        unless test.test_status == :pass || test.test_status == :skip then
+        if test.test_status == :fail || test.test_status == :error then
           status = REXML::Element.new('failure', item)
           status.add_attribute('type', test.test_status.to_s)
           if test.exception then
@@ -167,6 +174,8 @@ def write_junit_xml
         end
       end
 
+      # junit/name.xml will be created in a directory relative to the CWD
+      # --  JLS 2/12
       File.open("junit/#{name}.xml", 'w') { |fh| doc.write(fh) }
     rescue Exception => e
       Log.error "failure in XML output:\n#{e.to_s}\n" + e.backtrace.join("\n")
@@ -174,7 +183,7 @@ def write_junit_xml
   end
 
   def summarize
-    fail "you have not run the tests yet" unless @run
+    fail_without_test_run
 
     if Log.file then
       Log.file = log_path("#{name}-summary.txt")
@@ -189,40 +198,45 @@ def summarize
 
     TestConfig.dump(config)
 
-    test_failed=0
-    test_passed=0
-    test_errored=0
-    test_skips=0
-    @test_cases.each do |test_case|
-      case test_case.test_status
-      when :pass  then test_passed  += 1
-      when :fail  then test_failed  += 1
-      when :error then test_errored += 1
-      when :skip  then test_skips   += 1
-      end
-    end
-    grouped_summary = @test_cases.group_by{|test_case| test_case.test_status }
+    elapsed_time = @test_cases.inject(0.0) {|r, t| r + t.runtime.to_f }
+    average_test_time = elapsed_time / test_count
 
-    Log.notify <<-HEREDOC
+    Log.notify %Q[
 
-  - Test Case Summary -
-  Attempted: #{@test_cases.length}
-     Passed: #{test_passed}
-     Failed: #{test_failed}
-    Errored: #{test_errored}
-    Skipped: #{test_skips}
+          - Test Case Summary -
+   Total Suite Time: %.2f seconds
+  Average Test Time: %.2f seconds
+          Attempted: #{test_count}
+             Passed: #{passed_tests}
+             Failed: #{failed_tests}
+            Errored: #{errored_tests}
+            Skipped: #{skipped_tests}
+            Pending: #{pending_tests}
 
   - Specific Test Case Status -
-  HEREDOC
+    ] % [elapsed_time, average_test_time]
+
+    grouped_summary = @test_cases.group_by{|test_case| test_case.test_status }
 
     Log.notify "Failed Tests Cases:"
-    (grouped_summary[:fail] || []).each {|test_case| print_test_failure(test_case)}
+    (grouped_summary[:fail] || []).each do |test_case|
+      print_test_failure(test_case)
+    end
 
     Log.notify "Errored Tests Cases:"
-    (grouped_summary[:error] || []).each {|test_case| print_test_failure(test_case)}
+    (grouped_summary[:error] || []).each do |test_case|
+      print_test_failure(test_case)
+    end
 
     Log.notify "Skipped Tests Cases:"
-    (grouped_summary[:skip] || []).each {|test_case| print_test_failure(test_case)}
+    (grouped_summary[:skip] || []).each do |test_case|
+      print_test_failure(test_case)
+    end
+
+    Log.notify "Pending Tests Cases:"
+    (grouped_summary[:pending] || []).each do |test_case|
+      print_test_failure(test_case)
+    end
 
     Log.notify("\n\n")
 
@@ -231,7 +245,12 @@ def summarize
   end
 
   def print_test_failure(test_case)
-    Log.notify "  Test Case #{test_case.path} reported: #{test_case.exception.inspect}"
+    test_reported = if test_case.exception
+                      "reported: #{test_case.exception.inspect}"
+                    else
+                      test_case.test_status
+                    end
+    Log.notify "  Test Case #{test_case.path} #{test_reported}"
   end
 
   def log_path(name)

    

--
You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.

Reply via email to