diff --git a/Gemfile b/Gemfile index 624a0eab2..c3d3f5ee3 100644 --- a/Gemfile +++ b/Gemfile @@ -15,8 +15,8 @@ gemspec if ENV['BEAKER_PE_PR_REPO_URL'] lib = ENV['BEAKER_PE_PR_REPO_URL'].match(/\/([^\/]+)\.git$/)[1] - author = ENV['BEAKER_PE_PR_AUTHOR'] - ref = ENV['BEAKER_PE_PR_COMMIT'] + author = ENV.fetch('BEAKER_PE_PR_AUTHOR', nil) + ref = ENV.fetch('BEAKER_PE_PR_COMMIT', nil) gem lib, :git => "git@github.com:#{author}/#{lib}.git", :branch => ref end diff --git a/Rakefile b/Rakefile index ca9a973e2..fcf61c5a8 100644 --- a/Rakefile +++ b/Rakefile @@ -61,18 +61,16 @@ module HarnessOptions end def hosts_file_env - ENV['BEAKER_HOSTS'] + ENV.fetch('BEAKER_HOSTS', nil) end def hosts_opt(use_preserved_hosts = false) if use_preserved_hosts "--hosts=#{HOSTS_PRESERVED}" + elsif hosts_file_env + "--hosts=#{hosts_file_env}" else - if hosts_file_env - "--hosts=#{hosts_file_env}" - else - "--hosts=tmp/#{HOSTS_FILE}" - end + "--hosts=tmp/#{HOSTS_FILE}" end end @@ -110,7 +108,7 @@ def beaker_test(mode = :base, options = {}) end end - tests = ENV['TESTS'] || ENV['TEST'] + tests = ENV['TESTS'] || ENV.fetch('TEST', nil) tests_opt = "" tests_opt = "--tests=#{tests}" if tests @@ -142,17 +140,13 @@ namespace :test do puts line end output = stdout.to_s - if not wait_thr.value.success? - fail "Failed to 'bundle exec rspec' (exit status: #{wait_thr.value})" - end + fail "Failed to 'bundle exec rspec' (exit status: #{wait_thr.value})" if not wait_thr.value.success? exit_status = wait_thr.value end if exit_status != /0/ # check for deprecation warnings - if output.include?('Deprecation Warnings') - fail "DEPRECATION WARNINGS in spec generation, please fix!" - end + fail "DEPRECATION WARNINGS in spec generation, please fix!" if output.include?('Deprecation Warnings') end end end @@ -183,9 +177,7 @@ namespace :test do desc 'Generate Beaker Host Config File' task :gen_hosts do - if hosts_file_env - next - end + next if hosts_file_env arguments = [test_targets] arguments += ['--hypervisor', ENV['BEAKER_HYPERVISOR']] if ENV['BEAKER_HYPERVISOR'] @@ -208,9 +200,7 @@ namespace :history do Dir.chdir(__dir__) do output = `bundle exec ruby history.rb .` puts output - if !output.include?('success') - raise "History generation failed" - end + raise "History generation failed" if !output.include?('success') end end end @@ -227,9 +217,7 @@ FOREGROUND_SERVER = "bundle exec yard server --reload --verbose lib/beaker --doc def running?(cmdline) ps = `ps -ef` found = ps.lines.grep(/#{Regexp.quote(cmdline)}/) - if found.length > 1 - raise StandardError, "Found multiple YARD Servers. Don't know what to do." - end + raise StandardError, "Found multiple YARD Servers. Don't know what to do." if found.length > 1 yes = found.empty? ? false : true return yes, found.first @@ -259,9 +247,7 @@ namespace :docs do Dir.chdir(__dir__) do output = `bundle exec yard doc -o #{DOCS_DIR}` puts output - if /\[warn\]|\[error\]/.match?(output) - fail "Errors/Warnings during yard documentation generation" - end + fail "Errors/Warnings during yard documentation generation" if /\[warn\]|\[error\]/.match?(output) end end @@ -342,8 +328,6 @@ else task.options = ['--display-cop-names', '--display-style-guide', '--extra-details'] # Use Rubocop's Github Actions formatter if possible - if ENV['GITHUB_ACTIONS'] == 'true' - task.formatters << 'github' - end + task.formatters << 'github' if ENV['GITHUB_ACTIONS'] == 'true' end end diff --git a/acceptance/config/acceptance-options.rb b/acceptance/config/acceptance-options.rb index 6142c4425..bf458ac34 100644 --- a/acceptance/config/acceptance-options.rb +++ b/acceptance/config/acceptance-options.rb @@ -1,6 +1,6 @@ { :load_path => File.join('acceptance', 'lib'), :ssh => { - :keys => ["id_rsa_acceptance", "#{ENV['HOME']}/.ssh/id_rsa-acceptance"], + :keys => ["id_rsa_acceptance", "#{ENV.fetch('HOME', nil)}/.ssh/id_rsa-acceptance"], }, } diff --git a/acceptance/fixtures/module/spec/spec_helper_acceptance.rb b/acceptance/fixtures/module/spec/spec_helper_acceptance.rb index 78227468b..11c614543 100644 --- a/acceptance/fixtures/module/spec/spec_helper_acceptance.rb +++ b/acceptance/fixtures/module/spec/spec_helper_acceptance.rb @@ -12,9 +12,7 @@ end hosts.each do |host| - unless host.is_pe? - on host, "/bin/echo '' > #{host.puppet('hiera_config')}" - end + on host, "/bin/echo '' > #{host.puppet('hiera_config')}" unless host.is_pe? on host, "mkdir -p #{host['distmoduledir']}" end end diff --git a/acceptance/pre_suite/subcommands/05_install_ruby.rb b/acceptance/pre_suite/subcommands/05_install_ruby.rb index 3e5e4f615..6db88f5f3 100644 --- a/acceptance/pre_suite/subcommands/05_install_ruby.rb +++ b/acceptance/pre_suite/subcommands/05_install_ruby.rb @@ -1,4 +1,4 @@ -ruby_version, ruby_source = ENV['RUBY_VER'], "job parameter" +ruby_version, ruby_source = ENV.fetch('RUBY_VER', nil), "job parameter" unless ruby_version ruby_version = "2.4.1" ruby_source = "default" diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/backup_the_file_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/backup_the_file_test.rb index 3cdde60b2..2769caeb7 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/backup_the_file_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/backup_the_file_test.rb @@ -3,14 +3,14 @@ test_name "dsl::helpers::host_helpers #backup_the_file" do step "#backup_the_file CURRENTLY will return nil if the file does not exist in the source directory" do # NOTE: would expect this to fail with Beaker::Host::CommandFailure - remote_source = default.tmpdir() - remote_destination = default.tmpdir() + remote_source = default.tmpdir + remote_destination = default.tmpdir result = backup_the_file default, remote_source, remote_destination assert_nil result end step "#backup_the_file will fail if the destination directory does not exist" do - remote_source = default.tmpdir() + remote_source = default.tmpdir create_remote_file_from_fixture("simple_text_file", default, remote_source, "puppet.conf") assert_raises Beaker::Host::CommandFailure do @@ -19,10 +19,10 @@ end step "#backup_the_file copies `puppet.conf` from the source to the destination directory" do - remote_source = default.tmpdir() + remote_source = default.tmpdir _remote_filename, contents = create_remote_file_from_fixture("simple_text_file", default, remote_source, "puppet.conf") - remote_destination = default.tmpdir() + remote_destination = default.tmpdir remote_destination_filename = File.join(remote_destination, "puppet.conf.bak") result = backup_the_file default, remote_source, remote_destination @@ -33,10 +33,10 @@ end step "#backup_the_file copies a named file from the source to the destination directory" do - remote_source = default.tmpdir() + remote_source = default.tmpdir _remote_filename, contents = create_remote_file_from_fixture("simple_text_file", default, remote_source, "testfile.txt") - remote_destination = default.tmpdir() + remote_destination = default.tmpdir remote_destination_filename = File.join(remote_destination, "testfile.txt.bak") result = backup_the_file default, remote_source, remote_destination, "testfile.txt" @@ -47,9 +47,9 @@ end step "#backup_the_file CURRENTLY will fail if given a hosts array" do - remote_source = default.tmpdir() + remote_source = default.tmpdir create_remote_file_from_fixture("simple_text_file", default, remote_source, "testfile.txt") - remote_destination = default.tmpdir() + remote_destination = default.tmpdir assert_raises NoMethodError do backup_the_file hosts, remote_source, remote_destination diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/create_remote_file_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/create_remote_file_test.rb index 1e43e6690..042a6db80 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/create_remote_file_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/create_remote_file_test.rb @@ -8,9 +8,7 @@ def create_remote_file_with_backups host, remote_filename, contents, opts = {} result = create_remote_file( host, remote_filename, contents, opts ) # return of block is whether or not we're done repeating - if result.is_a?(Rsync::Result) || result.is_a?(Beaker::Result) - return result.success? - end + return result.success? if result.is_a?(Rsync::Result) || result.is_a?(Beaker::Result) result.all? { |individual_result| individual_result.success? } rescue Beaker::Host::CommandFailure => e diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/curl_on_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/curl_on_test.rb index ceedf0020..a1c46498b 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/curl_on_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/curl_on_test.rb @@ -17,7 +17,7 @@ def host_local_url(host, path) end step "#curl_on can retrieve the contents of a URL, using standard curl options" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_filename, contents = create_remote_file_from_fixture("simple_text_file", default, remote_tmpdir, "testfile.txt") remote_targetfilename = File.join remote_tmpdir, "outfile.txt" @@ -29,7 +29,7 @@ def host_local_url(host, path) end step "#curl_on can retrieve the contents of a URL, when given a hosts array" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir on hosts, "mkdir -p #{remote_tmpdir}" remote_filename = contents = nil diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/retry_on_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/retry_on_test.rb index f72b0a639..f6abbdc76 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/retry_on_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/retry_on_test.rb @@ -4,7 +4,7 @@ step "#retry_on CURRENTLY fails with a RuntimeError if command does not pass after all retries" do # NOTE: would have expected this to fail with Beaker::Hosts::CommandFailure - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_script_file = File.join(remote_tmpdir, "test.sh") create_remote_file_from_fixture("retry_script", default, remote_tmpdir, "test.sh") @@ -14,7 +14,7 @@ end step "#retry_on succeeds if command passes before retries are exhausted" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_script_file = File.join(remote_tmpdir, "test.sh") create_remote_file_from_fixture("retry_script", default, remote_tmpdir, "test.sh") @@ -27,7 +27,7 @@ # NOTE: would expect this to work across hosts, or be better documented and # to raise Beaker::Host::CommandFailure - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_script_file = File.join(remote_tmpdir, "test.sh") hosts.each do |host| diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/rsync_to_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/rsync_to_test.rb index ce7e55401..54b358d26 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/rsync_to_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/rsync_to_test.rb @@ -31,7 +31,7 @@ def rsync_to_with_backups hosts, local_filename, remote_dir step "#rsync_to CURRENTLY fails on windows systems" do Dir.mktmpdir do |local_dir| local_filename, _contents = create_local_file_from_fixture("simple_text_file", local_dir, "testfile.txt") - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir assert_raises Beaker::Host::CommandFailure do rsync_to default, local_filename, remote_tmpdir @@ -54,7 +54,7 @@ def rsync_to_with_backups hosts, local_filename, remote_dir confine :except, :hypervisor => 'docker' step "#rsync_to fails if the local file cannot be found" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir assert_raises IOError do rsync_to default, "/non/existent/file.txt", remote_tmpdir end @@ -116,7 +116,7 @@ def rsync_to_with_backups hosts, local_filename, remote_dir step "#rsync_to creates the file on the remote system" do Dir.mktmpdir do |local_dir| local_filename, contents = create_local_file_from_fixture("simple_text_file", local_dir, "testfile.txt") - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_filename = File.join(remote_tmpdir, "testfile.txt") result = rsync_to_with_backups default, local_filename, remote_tmpdir @@ -138,7 +138,7 @@ def rsync_to_with_backups hosts, local_filename, remote_dir step "#rsync_to creates the file on all remote systems when a host array is provided" do Dir.mktmpdir do |local_dir| local_filename, contents = create_local_file_from_fixture("simple_text_file", local_dir, "testfile.txt") - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir on hosts, "mkdir -p #{remote_tmpdir}" remote_filename = File.join(remote_tmpdir, "testfile.txt") diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/scp_from_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/scp_from_test.rb index 95a89a658..c1250b5f8 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/scp_from_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/scp_from_test.rb @@ -3,7 +3,7 @@ test_name "dsl::helpers::host_helpers #scp_from" do if test_scp_error_on_close? step "#scp_from fails if the local path cannot be found" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_filename, _contents = create_remote_file_from_fixture("simple_text_file", default, remote_tmpdir, "testfile.txt") assert_raises Beaker::Host::CommandFailure do @@ -22,7 +22,7 @@ step "#scp_from creates the file on the local system" do Dir.mktmpdir do |local_dir| - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_filename, contents = create_remote_file_from_fixture("simple_text_file", default, remote_tmpdir, "testfile.txt") scp_from default, remote_filename, local_dir @@ -37,7 +37,7 @@ # file repeatedly to generate an error Dir.mktmpdir do |local_dir| - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir remote_filename = File.join(remote_tmpdir, "testfile.txt") on hosts, "mkdir -p #{remote_tmpdir}" on hosts, %{echo "${RANDOM}:${RANDOM}:${RANDOM}" > #{remote_filename}} diff --git a/acceptance/tests/base/dsl/helpers/host_helpers/scp_to_test.rb b/acceptance/tests/base/dsl/helpers/host_helpers/scp_to_test.rb index 184958cad..29fe000d5 100644 --- a/acceptance/tests/base/dsl/helpers/host_helpers/scp_to_test.rb +++ b/acceptance/tests/base/dsl/helpers/host_helpers/scp_to_test.rb @@ -2,7 +2,7 @@ test_name "dsl::helpers::host_helpers #scp_to" do step "#scp_to fails if the local file cannot be found" do - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir assert_raises IOError do scp_to default, "/non/existent/file.txt", remote_tmpdir end @@ -24,7 +24,7 @@ step "#scp_to creates the file on the remote system" do Dir.mktmpdir do |local_dir| local_filename, contents = create_local_file_from_fixture("simple_text_file", local_dir, "testfile.txt") - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir scp_to default, local_filename, remote_tmpdir @@ -38,7 +38,7 @@ Dir.mktmpdir do |local_dir| local_filename, contents = create_local_file_from_fixture("simple_text_file", local_dir, "testfile.txt") - remote_tmpdir = default.tmpdir() + remote_tmpdir = default.tmpdir on hosts, "mkdir -p #{remote_tmpdir}" remote_filename = File.join(remote_tmpdir, "testfile.txt") diff --git a/acceptance/tests/base/dsl/platform_tag_confiner_test.rb b/acceptance/tests/base/dsl/platform_tag_confiner_test.rb index 7c708d17c..ccedd51f5 100644 --- a/acceptance/tests/base/dsl/platform_tag_confiner_test.rb +++ b/acceptance/tests/base/dsl/platform_tag_confiner_test.rb @@ -13,7 +13,7 @@ end step "#{pstc_method_name} can remove hosts from a test, or be skipped if empty" do - assert hosts.length() > 0, "#{pstc_method_name} did not have enough hosts to test" + assert hosts.length > 0, "#{pstc_method_name} did not have enough hosts to test" previous_hosts = hosts.dup options[:platform_tag_confines] = [ @@ -26,19 +26,16 @@ begin tag('tag1') rescue Beaker::DSL::Outcomes::SkipTest => e - if /^No\ suitable\ hosts\ found/.match?(e.message) - # SkipTest is raised in the case when there are no hosts leftover for a test - # after confining. It's a very common acceptance test case where all of the - # hosts involved are of the same platform, and are thus all confined - # away by the code being run here. In this case, the hosts object will not - # be altered, but should be considered a pass, since the fact that SkipTest - # is being raised confirms that a lower number of hosts are coming out of - # the confine (0) than came in (>0, according to our pre-condition assertion) - else - fail "#{pstc_method_name} raised unexpected SkipTest exception: #{e}" - end + fail "#{pstc_method_name} raised unexpected SkipTest exception: #{e}" unless /^No\ suitable\ hosts\ found/.match?(e.message) + # SkipTest is raised in the case when there are no hosts leftover for a test + # after confining. It's a very common acceptance test case where all of the + # hosts involved are of the same platform, and are thus all confined + # away by the code being run here. In this case, the hosts object will not + # be altered, but should be considered a pass, since the fact that SkipTest + # is being raised confirms that a lower number of hosts are coming out of + # the confine (0) than came in (>0, according to our pre-condition assertion) else - assert hosts.length() < previous_hosts.length(), "#{pstc_method_name} did not change hosts array" + assert hosts.length < previous_hosts.length, "#{pstc_method_name} did not change hosts array" end # cleanup diff --git a/acceptance/tests/install/from_file.rb b/acceptance/tests/install/from_file.rb index f7b6804ef..667128e3d 100644 --- a/acceptance/tests/install/from_file.rb +++ b/acceptance/tests/install/from_file.rb @@ -13,8 +13,6 @@ step 'install arbitrary dmg via url' do hosts.each do |host| - if host['platform'].include?('osx') - host.generic_install_dmg('https://releases.hashicorp.com/vagrant/1.8.4/vagrant_1.8.4.dmg', 'Vagrant', 'Vagrant.pkg') - end + host.generic_install_dmg('https://releases.hashicorp.com/vagrant/1.8.4/vagrant_1.8.4.dmg', 'Vagrant', 'Vagrant.pkg') if host['platform'].include?('osx') end end diff --git a/beaker.gemspec b/beaker.gemspec index a4299d2d9..96653bf6d 100644 --- a/beaker.gemspec +++ b/beaker.gemspec @@ -1,5 +1,3 @@ -# -*- encoding: utf-8 -*- - lib = File.expand_path("lib", __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) require 'beaker/version' diff --git a/lib/beaker/cli.rb b/lib/beaker/cli.rb index 40c809fa6..2c29d3973 100644 --- a/lib/beaker/cli.rb +++ b/lib/beaker/cli.rb @@ -106,9 +106,7 @@ def execute! end # Setup perf monitoring if needed - if /(aggressive)|(normal)/.match?(@options[:collect_perf_data].to_s) - @perf = Beaker::Perf.new(@hosts, @options) - end + @perf = Beaker::Perf.new(@hosts, @options) if /(aggressive)|(normal)/.match?(@options[:collect_perf_data].to_s) # pre acceptance phase run_suite(:pre_suite, :fast) @@ -142,9 +140,7 @@ def execute! # cleanup on error if /(never)|(onpass)/.match?(@options[:preserve_hosts].to_s) @logger.notify "Cleanup: cleaning up after failed run" - if @network_manager - @network_manager.cleanup - end + @network_manager.cleanup if @network_manager else preserve_hosts_file end @@ -165,16 +161,12 @@ def execute! # cleanup on success if /(never)|(onfail)/.match?(@options[:preserve_hosts].to_s) @logger.notify "Cleanup: cleaning up after successful run" - if @network_manager - @network_manager.cleanup - end + @network_manager.cleanup if @network_manager else preserve_hosts_file end - if @logger.is_debug? - print_reproduction_info(:debug) - end + print_reproduction_info(:debug) if @logger.is_debug? end end @@ -197,9 +189,7 @@ def run_suite(suite_name, failure_strategy = nil) def configured_options result = Beaker::Options::OptionsHash.new @attribution.each do |attribute, setter| - if setter != 'preset' - result[attribute] = @options[attribute] - end + result[attribute] = @options[attribute] if setter != 'preset' end result end @@ -272,8 +262,8 @@ def print_reproduction_info(log_level = :debug) def print_env_vars_affecting_beaker(log_level) non_beaker_env_vars = ['BUNDLE_PATH', 'BUNDLE_BIN', 'GEM_HOME', 'GEM_PATH', 'RUBYLIB', 'PATH'] env_var_map = non_beaker_env_vars.each_with_object({}) do |possibly_set_vars, memo| - set_var = Array(possibly_set_vars).detect { |possible_var| ENV[possible_var] } - memo[set_var] = ENV[set_var] if set_var + set_var = Array(possibly_set_vars).detect { |possible_var| ENV.fetch(possible_var, nil) } + memo[set_var] = ENV.fetch(set_var, nil) if set_var end env_var_map = env_var_map.merge(Beaker::Options::Presets.new.env_vars) @@ -304,24 +294,24 @@ def print_env_vars_affecting_beaker(log_level) def print_command_line(log_level = :debug) @logger.send(log_level, "\nYou can reproduce this run with:\n") @logger.send(log_level, @options[:command_line]) - if @options[:hosts_preserved_yaml_file] - set_docker_warning = false - has_supported_hypervisor = false - @hosts.each do |host| - case host[:hypervisor] - when /vagrant|fusion|vmpooler|vcloud/ - has_supported_hypervisor = true - when /docker/ - set_docker_warning = true - end - end - if has_supported_hypervisor - reproducing_command = build_hosts_preserved_reproducing_command(@options[:command_line], @options[:hosts_preserved_yaml_file]) - @logger.send(log_level, "\nYou can re-run commands against the already provisioned SUT(s) with:\n") - @logger.send(log_level, '(docker support is untested for this feature. please reference the docs for more info)') if set_docker_warning - @logger.send(log_level, reproducing_command) + return unless @options[:hosts_preserved_yaml_file] + + set_docker_warning = false + has_supported_hypervisor = false + @hosts.each do |host| + case host[:hypervisor] + when /vagrant|fusion|vmpooler|vcloud/ + has_supported_hypervisor = true + when /docker/ + set_docker_warning = true end end + return unless has_supported_hypervisor + + reproducing_command = build_hosts_preserved_reproducing_command(@options[:command_line], @options[:hosts_preserved_yaml_file]) + @logger.send(log_level, "\nYou can re-run commands against the already provisioned SUT(s) with:\n") + @logger.send(log_level, '(docker support is untested for this feature. please reference the docs for more info)') if set_docker_warning + @logger.send(log_level, reproducing_command) end # provides a new version of the command given, edited for re-use with a diff --git a/lib/beaker/command.rb b/lib/beaker/command.rb index 40524df1c..68c330799 100644 --- a/lib/beaker/command.rb +++ b/lib/beaker/command.rb @@ -55,9 +55,7 @@ def initialize command, args = [], options = {} # this is deprecated and will not allow you to use a command line # option of `--environment`, please use ENV instead. [:ENV, :environment, 'environment', 'ENV'].each do |k| - if @options[k].is_a?(Hash) - @environment = @environment.merge(@options.delete(k)) - end + @environment = @environment.merge(@options.delete(k)) if @options[k].is_a?(Hash) end end @@ -128,8 +126,8 @@ def args_string args = @args class PuppetCommand < Command def initialize *args command = "puppet #{args.shift}" - opts = args.last.is_a?(Hash) ? args.pop : Hash.new - opts['ENV'] ||= Hash.new + opts = args.last.is_a?(Hash) ? args.pop : {} + opts['ENV'] ||= {} opts[:cmdexe] = true super(command, args, opts) end @@ -162,7 +160,7 @@ def initialize platform, expression, filename, opts = {} command << " > #{temp_file} && mv #{temp_file} #{filename} && rm -f #{temp_file}" end args = [] - opts['ENV'] ||= Hash.new + opts['ENV'] ||= {} super(command, args, opts) end end diff --git a/lib/beaker/dsl/helpers.rb b/lib/beaker/dsl/helpers.rb index e8bfa3102..945b86fd4 100644 --- a/lib/beaker/dsl/helpers.rb +++ b/lib/beaker/dsl/helpers.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - ['host', 'test', 'web', 'hocon'].each do |lib| require "beaker/dsl/helpers/#{lib}_helpers" end diff --git a/lib/beaker/dsl/helpers/hocon_helpers.rb b/lib/beaker/dsl/helpers/hocon_helpers.rb index a9486e5e5..89e8012cb 100644 --- a/lib/beaker/dsl/helpers/hocon_helpers.rb +++ b/lib/beaker/dsl/helpers/hocon_helpers.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - require 'hocon/parser/config_document_factory' require 'hocon/config_value_factory' @@ -20,9 +18,7 @@ module HoconHelpers # @raise ArgumentError if arguments are missing or incorrect # @return [Hocon::ConfigValueFactory] parsed hocon file def hocon_file_read_on(host, filename) - if filename.nil? || filename.empty? - raise ArgumentError, '#hocon_file_edit_on requires a filename' - end + raise ArgumentError, '#hocon_file_edit_on requires a filename' if filename.nil? || filename.empty? file_contents = on(host, "cat #{filename}").stdout Hocon::Parser::ConfigDocumentFactory.parse_string(file_contents) diff --git a/lib/beaker/dsl/helpers/host_helpers.rb b/lib/beaker/dsl/helpers/host_helpers.rb index 6e74db9eb..302aa1882 100644 --- a/lib/beaker/dsl/helpers/host_helpers.rb +++ b/lib/beaker/dsl/helpers/host_helpers.rb @@ -63,9 +63,7 @@ def on(host, command, opts = {}, &block) if command.is_a? String cmd_opts = {} # add any additional environment variables to the command - if opts[:environment] - cmd_opts['ENV'] = opts[:environment] - end + cmd_opts['ENV'] = opts[:environment] if opts[:environment] command_object = Command.new(command.to_s, [], cmd_opts) elsif command.is_a? Command if opts[:environment] @@ -236,9 +234,7 @@ def archive_file_from(host, from_path, opts = {}, archive_root = 'archive/sut-fi FileUtils.mkdir_p(targetdir) scp_from(host, from_path, targetdir, opts) # scp_from does succeed on a non-existant file, checking if the file/folder actually exists - if not File.exist?(filename) - raise IOError, "No such file or directory - #{filename}" - end + raise IOError, "No such file or directory - #{filename}" if not File.exist?(filename) create_tarball(archive_root, archive_name) end @@ -376,13 +372,11 @@ def upgrade_package host, package_name # # @return nil def add_system32_hosts_entry(host, opts = {}) - if host.is_powershell? - hosts_file = 'C:\Windows\System32\Drivers\etc\hosts' - host_entry = "#{opts['ip']}`t`t#{opts['name']}" - on host, powershell("\$text = \\\"#{host_entry}\\\"; Add-Content -path '#{hosts_file}' -value \$text") - else - raise "nothing to do for #{host.name} on #{host['platform']}" - end + raise "nothing to do for #{host.name} on #{host['platform']}" unless host.is_powershell? + + hosts_file = 'C:\Windows\System32\Drivers\etc\hosts' + host_entry = "#{opts['ip']}`t`t#{opts['name']}" + on host, powershell("\$text = \\\"#{host_entry}\\\"; Add-Content -path '#{hosts_file}' -value \$text") end # Back up the given file in the current_dir to the new_dir diff --git a/lib/beaker/dsl/helpers/web_helpers.rb b/lib/beaker/dsl/helpers/web_helpers.rb index 9b71dffc2..b2f422fae 100644 --- a/lib/beaker/dsl/helpers/web_helpers.rb +++ b/lib/beaker/dsl/helpers/web_helpers.rb @@ -67,11 +67,9 @@ def fetch_http_file(base_url, file_name, dst_dir) end end rescue OpenURI::HTTPError => e - if /404.*/.match?(e.message) - raise "Failed to fetch_remote_file '#{src}' (#{e.message})" - else - raise e - end + raise "Failed to fetch_remote_file '#{src}' (#{e.message})" if /404.*/.match?(e.message) + + raise e end end return dst @@ -90,9 +88,7 @@ def fetch_http_file(base_url, file_name, dst_dir) # @!visibility private def fetch_http_dir(url, dst_dir) logger.notify "fetch_http_dir (url: #{url}, dst_dir #{dst_dir})" - if !url.end_with?('/') - url += '/' - end + url += '/' if !url.end_with?('/') url = URI.parse(url) chunks = url.path.split('/') dst = File.join(dst_dir, chunks.last) @@ -109,9 +105,7 @@ def fetch_http_dir(url, dst_dir) stdout_and_stderr_str.each_line do |line| logger.debug(line) end - unless status.success? - raise "Failed to fetch_remote_dir '#{url}' (exit code #{$?})" - end + raise "Failed to fetch_remote_dir '#{url}' (exit code #{$?})" unless status.success? dst end diff --git a/lib/beaker/dsl/roles.rb b/lib/beaker/dsl/roles.rb index 460ab3ee2..8f3537bc9 100644 --- a/lib/beaker/dsl/roles.rb +++ b/lib/beaker/dsl/roles.rb @@ -173,19 +173,15 @@ def add_role_def role role.each do |r| add_role_def r end - else - if not respond_to? role - if !/\A[[:alpha:]]+[a-zA-Z0-9_]*[!?=]?\Z/.match?(role) - raise ArgumentError, "Role name format error for '#{role}'. Allowed characters are: \na-Z\n0-9 (as long as not at the beginning of name)\n'_'\n'?', '!' and '=' (only as individual last character at end of name)" - end + elsif not respond_to? role + if !/\A[[:alpha:]]+[a-zA-Z0-9_]*[!?=]?\Z/.match?(role) + raise ArgumentError, "Role name format error for '#{role}'. Allowed characters are: \na-Z\n0-9 (as long as not at the beginning of name)\n'_'\n'?', '!' and '=' (only as individual last character at end of name)" + end - self.class.send :define_method, role.to_s do - hosts_with_role = hosts_as role.to_sym - if hosts_with_role.length == 1 - hosts_with_role = hosts_with_role.pop - end - hosts_with_role - end + self.class.send :define_method, role.to_s do + hosts_with_role = hosts_as role.to_sym + hosts_with_role = hosts_with_role.pop if hosts_with_role.length == 1 + hosts_with_role end end end diff --git a/lib/beaker/dsl/structure.rb b/lib/beaker/dsl/structure.rb index 615ce6f1c..63db723be 100644 --- a/lib/beaker/dsl/structure.rb +++ b/lib/beaker/dsl/structure.rb @@ -37,32 +37,30 @@ module Structure def step step_name, &block logger.notify "\n* #{step_name}\n" set_current_step_name(step_name) - if block - begin - logger.with_indent do - yield - end - rescue Exception => e - if @options.has_key?(:debug_errors) && @options[:debug_errors] == true + return unless block + + begin + logger.with_indent(&block) + rescue Exception => e + if @options.has_key?(:debug_errors) && @options[:debug_errors] == true + begin + require 'pry' + rescue LoadError begin - require 'pry' + require 'debug' rescue LoadError - begin - require 'debug' - rescue LoadError - logger.exception('Unable to load pry and debug while debug_errors was true') - else - logger.info("Exception raised during step execution and debug-errors option is set, entering debug. Exception was: #{e.inspect}") - binding.break # rubocop:disable Lint/Debugger - end + logger.exception('Unable to load pry and debug while debug_errors was true') else - logger.info("Exception raised during step execution and debug-errors option is set, entering pry. Exception was: #{e.inspect}") - logger.info("HINT: Use the pry 'backtrace' and 'up' commands to navigate to the test code") - binding.pry # rubocop:disable Lint/Debugger + logger.info("Exception raised during step execution and debug-errors option is set, entering debug. Exception was: #{e.inspect}") + binding.break # rubocop:disable Lint/Debugger end + else + logger.info("Exception raised during step execution and debug-errors option is set, entering pry. Exception was: #{e.inspect}") + logger.info("HINT: Use the pry 'backtrace' and 'up' commands to navigate to the test code") + binding.pry # rubocop:disable Lint/Debugger end - raise e end + raise e end end @@ -90,12 +88,10 @@ def manual_step step_name fail_message = '' loop do fail_message = Readline.readline('What was the reason for failure? ', true).squeeze(" ").strip - if fail_message == '' - # if nothing is entered we tell the user to enter something - puts "No reason for failure given, please enter reason for failure." - else - break - end + break unless fail_message == '' + + # if nothing is entered we tell the user to enter something + puts "No reason for failure given, please enter reason for failure." end raise Beaker::DSL::FailTest, fail_message else @@ -128,14 +124,12 @@ def manual_test manual_test_name, &block # @param [String] my_name The name of the test to be logged. # @param [Proc] block The actions to be performed during this test. # - def test_name my_name + def test_name my_name, &block logger.notify "\n#{my_name}\n" set_current_test_name(my_name) - if block_given? - logger.with_indent do - yield - end - end + return unless block + + logger.with_indent(&block) end # Declare a teardown process that will be called after a test case is @@ -339,18 +333,14 @@ def confine_block(type, criteria, host_array = nil) # should return true if the host matches this additional criteria. # # @return [Array] Returns an array of hosts that meet the provided criteria - def select_hosts(criteria, host_array = nil) + def select_hosts(criteria, host_array = nil, &block) hosts_to_select_from = host_array || hosts criteria.each_pair do |property, value| hosts_to_select_from = hosts_to_select_from.select do |host| inspect_host host, property, value end end - if block_given? - hosts_to_select_from = hosts_to_select_from.select do |host| - yield host - end - end + hosts_to_select_from = hosts_to_select_from.select(&block) if block hosts_to_select_from end diff --git a/lib/beaker/host.rb b/lib/beaker/host.rb index dd9b16c74..3bb9d67da 100644 --- a/lib/beaker/host.rb +++ b/lib/beaker/host.rb @@ -109,7 +109,7 @@ def wait_for_port(port, attempts = 15) start = Time.now done = repeat_fibonacci_style_for(attempts) { port_open?(port) } if done - @logger.debug('connected in %0.2f seconds' % (Time.now - start)) + @logger.debug(format('connected in %0.2f seconds', (Time.now - start))) else @logger.debug('timeout') end @@ -227,16 +227,14 @@ def get_public_ip return self[:instance].ip_address elsif self[:hypervisor] == 'openstack' && self[:ip] return self[:ip] - else + elsif self.instance_of?(Windows::Host) # In the case of using ec2 instances with the --no-provision flag, the ec2 # instance object does not exist and we should just use the curl endpoint # specified here: # http://docs.aws.amazon.com/AWSEC2/latest/UserGuide/using-instance-addressing.html - if self.instance_of?(Windows::Host) - execute("wget http://169.254.169.254/latest/meta-data/public-ipv4").strip - else - execute("curl http://169.254.169.254/latest/meta-data/public-ipv4").strip - end + execute("wget http://169.254.169.254/latest/meta-data/public-ipv4").strip + else + execute("curl http://169.254.169.254/latest/meta-data/public-ipv4").strip end end end @@ -263,15 +261,9 @@ def connection self['user'], self['ssh'], { :logger => @logger, :ssh_connection_preference => self[:ssh_connection_preference] }) # update connection information - if self['ip'] && (@connection.ip != self['ip']) - @connection.ip = self['ip'] - end - if self['vmhostname'] && (@connection.vmhostname != self['vmhostname']) - @connection.vmhostname = self['vmhostname'] - end - if @name && (@connection.hostname != @name) - @connection.hostname = @name - end + @connection.ip = self['ip'] if self['ip'] && (@connection.ip != self['ip']) + @connection.vmhostname = self['vmhostname'] if self['vmhostname'] && (@connection.vmhostname != self['vmhostname']) + @connection.hostname = @name if @name && (@connection.hostname != @name) @connection end @@ -323,9 +315,7 @@ def exec command, options = {} end end - if not options[:silent] - @logger.debug "\n#{log_prefix} executed in %0.2f seconds" % seconds - end + @logger.debug "\n#{log_prefix} executed in %0.2f seconds" % seconds if not options[:silent] if options[:reset_connection] # Expect the connection to fail hard and possibly take a long time timeout. @@ -410,9 +400,7 @@ def do_scp_to source, target_path, options end # either a single file, or a directory with no ignores - if not File.file?(source) and not File.directory?(source) - raise IOError, "No such file or directory - #{source}" - end + raise IOError, "No such file or directory - #{source}" if not File.file?(source) and not File.directory?(source) if File.file?(source) or (File.directory?(source) and not has_ignore) source_file = source @@ -496,9 +484,7 @@ def do_rsync_to from_path, to_path, opts = {} rsync_args = [] ssh_args = [] - if not File.file?(from_path) and not File.directory?(from_path) - raise IOError, "No such file or directory - #{from_path}" - end + raise IOError, "No such file or directory - #{from_path}" if not File.file?(from_path) and not File.directory?(from_path) # We enable achieve mode and compression rsync_args << "-az" @@ -525,43 +511,33 @@ def do_rsync_to from_path, to_path, opts = {} if filesystem_ssh_config ssh_args << "-F #{filesystem_ssh_config}" - else - if ssh_opts.has_key?('keys') and - ssh_opts.has_key?('auth_methods') and - ssh_opts['auth_methods'].include?('publickey') - - # find the first SSH key that exists - key = Array(ssh_opts['keys']).find do |k| - File.exist?(k) - end + elsif ssh_opts.has_key?('keys') and + ssh_opts.has_key?('auth_methods') and + ssh_opts['auth_methods'].include?('publickey') + key = Array(ssh_opts['keys']).find do |k| + File.exist?(k) + end - if key - # rsync doesn't always play nice with tilde, so be sure to expand first - ssh_args << "-i #{File.expand_path(key)}" - end + if key + # rsync doesn't always play nice with tilde, so be sure to expand first + ssh_args << "-i #{File.expand_path(key)}" end - end - if ssh_opts.has_key?(:port) - ssh_args << "-p #{ssh_opts[:port]}" + # find the first SSH key that exists end + ssh_args << "-p #{ssh_opts[:port]}" if ssh_opts.has_key?(:port) + # We disable prompt when host isn't known ssh_args << "-o 'StrictHostKeyChecking no'" - if not ssh_args.empty? - rsync_args << "-e \"ssh #{ssh_args.join(' ')}\"" - end + rsync_args << "-e \"ssh #{ssh_args.join(' ')}\"" if not ssh_args.empty? - if opts.has_key?(:ignore) and not opts[:ignore].empty? - rsync_args << opts[:ignore].map { |value| "--exclude '#{value}'" }.join(' ') - end + rsync_args << opts[:ignore].map { |value| "--exclude '#{value}'" }.join(' ') if opts.has_key?(:ignore) and not opts[:ignore].empty? # We assume that the *contents* of the directory 'from_path' needs to be # copied into the directory 'to_path' - if File.directory?(from_path) and not from_path.end_with?('/') - from_path += '/' - end + from_path += '/' if File.directory?(from_path) and not from_path.end_with?('/') @logger.notify "rsync: localhost:#{from_path} to #{hostname_with_user}:#{to_path} {:ignore => #{opts[:ignore]}}" result = Rsync.run(from_path, to_path, rsync_args) diff --git a/lib/beaker/host/aix/exec.rb b/lib/beaker/host/aix/exec.rb index 512cd213d..3117078f3 100644 --- a/lib/beaker/host/aix/exec.rb +++ b/lib/beaker/host/aix/exec.rb @@ -24,6 +24,6 @@ def ssh_service_restart # (from {#ssh_service_restart}). def ssh_permit_user_environment exec(Beaker::Command.new("echo '\nPermitUserEnvironment yes' >> /etc/ssh/sshd_config")) - ssh_service_restart() + ssh_service_restart end end diff --git a/lib/beaker/host/cisco.rb b/lib/beaker/host/cisco.rb index 182263a5a..a45444c6c 100644 --- a/lib/beaker/host/cisco.rb +++ b/lib/beaker/host/cisco.rb @@ -33,9 +33,7 @@ def skip_set_env? # # @return nil def scp_post_operations(scp_file_actual, scp_file_target) - if self[:platform].include?('cisco_nexus') - execute("mv #{scp_file_actual} #{scp_file_target}") - end + execute("mv #{scp_file_actual} #{scp_file_target}") if self[:platform].include?('cisco_nexus') nil end @@ -89,12 +87,8 @@ def prepend_commands(command = '', user_pc = '', _opts = {}) prepend_cmds = 'source /etc/profile;' prepend_cmds << " sudo -E sh -c \"" if self[:user] != 'root' - if self[:vrf] - prepend_cmds << "ip netns exec #{self[:vrf]} " - end - if user_pc && !user_pc.empty? - prepend_cmds << "#{user_pc} " - end + prepend_cmds << "ip netns exec #{self[:vrf]} " if self[:vrf] + prepend_cmds << "#{user_pc} " if user_pc && !user_pc.empty? prepend_cmds.strip end @@ -147,25 +141,19 @@ def environment_string env def validate_setup msg = nil if self[:platform].include?('cisco_nexus') - if !self[:vrf] - msg = 'Cisco Nexus hosts must be provided with a :vrf value.' - end - if !self[:user] - msg = 'Cisco hosts must be provided with a :user value' - end + msg = 'Cisco Nexus hosts must be provided with a :vrf value.' if !self[:vrf] + msg = 'Cisco hosts must be provided with a :user value' if !self[:user] end if self[:platform].include?('cisco_ios_xr') - if !self[:user] - msg = 'Cisco hosts must be provided with a :user value' - end + msg = 'Cisco hosts must be provided with a :user value' if !self[:user] end - if msg - msg << <<-EOF + return unless msg + + msg << <<-EOF Check https://github.com/puppetlabs/beaker/blob/master/docs/hosts/cisco.md for more info.' - EOF - raise ArgumentError, msg - end + EOF + raise ArgumentError, msg end end end diff --git a/lib/beaker/host/mac/exec.rb b/lib/beaker/host/mac/exec.rb index 7a8bd1eb7..5b9bc79fd 100644 --- a/lib/beaker/host/mac/exec.rb +++ b/lib/beaker/host/mac/exec.rb @@ -24,7 +24,7 @@ def ssh_permit_user_environment ssh_config_file = '/private/etc/ssh/sshd_config' if /^osx-/.match?(self['platform']) exec(Beaker::Command.new("echo '\nPermitUserEnvironment yes' >> #{ssh_config_file}")) - ssh_service_restart() + ssh_service_restart end #  Checks if selinux is enabled diff --git a/lib/beaker/host/mac/group.rb b/lib/beaker/host/mac/group.rb index 83b32233e..906922593 100644 --- a/lib/beaker/host/mac/group.rb +++ b/lib/beaker/host/mac/group.rb @@ -31,7 +31,7 @@ def group_list def group_get(name) execute("dscacheutil -q group -a name #{name}") do |result| fail_test "failed to get group #{name}" unless /^name: #{name}/.match?(result.stdout) - gi = Hash.new # group info + gi = {} # group info result.stdout.each_line do |line| pieces = line.split(': ') gi[pieces[0].to_sym] = pieces[1].strip if pieces[1] != nil diff --git a/lib/beaker/host/pswindows/exec.rb b/lib/beaker/host/pswindows/exec.rb index 09100fa13..791bbe200 100644 --- a/lib/beaker/host/pswindows/exec.rb +++ b/lib/beaker/host/pswindows/exec.rb @@ -46,9 +46,7 @@ def modified_at(file, timestamp = nil) result = execute("powershell Test-Path #{file} -PathType Leaf") - if result.include? 'False' - execute("powershell New-Item -ItemType file #{file}") - end + execute("powershell New-Item -ItemType file #{file}") if result.include? 'False' execute("powershell (gci #{file}).LastWriteTime = Get-Date " \ "-Year '#{time.year}'" \ "-Month '#{time.month}'" \ @@ -93,9 +91,7 @@ def ping target, attempts = 5 try = 0 while try < attempts do result = exec(Beaker::Command.new("ping -n 1 #{target}"), :accept_all_exit_codes => true) - if result.exit_code == 0 - return true - end + return true if result.exit_code == 0 try += 1 end @@ -141,12 +137,12 @@ def delete_env_var key, val # get the current value of the key cur_val = get_env_var(key, true) subbed_val = (cur_val.split(';') - [val.gsub(/'|"/, '')]).join(';') - if subbed_val != cur_val - # remove the current key value - self.clear_env_var(key) - # set to the truncated value - self.add_env_var(key, subbed_val) - end + return unless subbed_val != cur_val + + # remove the current key value + self.clear_env_var(key) + # set to the truncated value + self.add_env_var(key, subbed_val) end # Return the value of a specific env var @@ -158,15 +154,13 @@ def get_env_var key, clean = false self.close # refresh the state key = key.to_s.upcase val = exec(Beaker::Command.new("set #{key}"), :accept_all_exit_codes => true).stdout.chomp - if val.empty? - return '' + return '' if val.empty? + + val = val.split("\n")[0] # only take the first result + if clean + val.gsub(/#{key}=/i, '') else - val = val.split("\n")[0] # only take the first result - if clean - val.gsub(/#{key}=/i, '') - else - val - end + val end end @@ -219,8 +213,7 @@ def environment_variable_string_pair_array env # Overrides the {Windows::Exec#ssh_permit_user_environment} method, # since no steps are needed in this setup to allow user ssh environments # to work. - def ssh_permit_user_environment - end + def ssh_permit_user_environment; end # Sets the user SSH environment. # diff --git a/lib/beaker/host/pswindows/file.rb b/lib/beaker/host/pswindows/file.rb index a5fd80063..aded8e0df 100644 --- a/lib/beaker/host/pswindows/file.rb +++ b/lib/beaker/host/pswindows/file.rb @@ -3,15 +3,13 @@ module PSWindows::File def tmpfile(_name = '') result = exec(powershell('[System.IO.Path]::GetTempFileName()')) - result.stdout.chomp() + result.stdout.chomp end def tmpdir(name = '') - tmp_path = exec(powershell('[System.IO.Path]::GetTempPath()')).stdout.chomp() + tmp_path = exec(powershell('[System.IO.Path]::GetTempPath()')).stdout.chomp - if name == '' - name = exec(powershell('[System.IO.Path]::GetRandomFileName()')).stdout.chomp() - end + name = exec(powershell('[System.IO.Path]::GetRandomFileName()')).stdout.chomp if name == '' exec(powershell("New-Item -Path '#{tmp_path}' -Force -Name '#{name}' -ItemType 'directory'")) File.join(tmp_path, name) end diff --git a/lib/beaker/host/unix.rb b/lib/beaker/host/unix.rb index 55fecb32f..d99e16115 100644 --- a/lib/beaker/host/unix.rb +++ b/lib/beaker/host/unix.rb @@ -54,8 +54,6 @@ def skip_set_env? case self['platform'].variant when /^(f5|netscaler)$/ "no puppet-agent package for network device platform '#{self['platform'].variant}'" - else - nil end end diff --git a/lib/beaker/host/unix/exec.rb b/lib/beaker/host/unix/exec.rb index 92ee43475..d3d8894e7 100644 --- a/lib/beaker/host/unix/exec.rb +++ b/lib/beaker/host/unix/exec.rb @@ -41,9 +41,7 @@ def reboot(wait_time = 10, max_connection_tries = 9, uptime_retries = 18) original_boot_time = Time.parse(original_boot_time_matches.first) - unless original_boot_time_matches.last - reboot_sleep = (61 - Time.now.strftime("%S").to_i) - end + reboot_sleep = (61 - Time.now.strftime("%S").to_i) unless original_boot_time_matches.last @logger.notify("Sleeping #{reboot_sleep} seconds before rebooting") @@ -92,9 +90,7 @@ def reboot(wait_time = 10, max_connection_tries = 9, uptime_retries = 18) @logger.debug("Current Boot Time: #{current_boot_time}") # If this is *exactly* the same then there is really no good way to detect a reboot - if current_boot_time == original_boot_time - raise Beaker::Host::RebootFailure, "Boot time did not reset. Reboot appears to have failed." - end + raise Beaker::Host::RebootFailure, "Boot time did not reset. Reboot appears to have failed." if current_boot_time == original_boot_time rescue ArgumentError => e raise Beaker::Host::RebootFailure, "Unable to parse time: #{e.message}" rescue Beaker::Host::RebootFailure => e @@ -180,9 +176,7 @@ def ping target, attempts = 5 try = 0 while try < attempts do result = exec(Beaker::Command.new("ping -c 1 #{target}"), :accept_all_exit_codes => true) - if result.exit_code == 0 - return true - end + return true if result.exit_code == 0 try += 1 end @@ -309,7 +303,7 @@ def ssh_service_restart def ssh_permit_user_environment case self['platform'] when /debian|ubuntu|cumulus|huaweios|archlinux|el-|centos|fedora|redhat|oracle|scientific|eos|opensuse|sles|solaris/ - directory = tmpdir() + directory = tmpdir exec(Beaker::Command.new("echo 'PermitUserEnvironment yes' | cat - /etc/ssh/sshd_config > #{directory}/sshd_config.permit")) exec(Beaker::Command.new("mv #{directory}/sshd_config.permit /etc/ssh/sshd_config")) exec(Beaker::Command.new("echo '' >/etc/environment")) if /ubuntu-2(0|2).04/.match?(self['platform']) @@ -319,7 +313,7 @@ def ssh_permit_user_environment raise ArgumentError, "Unsupported Platform: '#{self['platform']}'" end - ssh_service_restart() + ssh_service_restart end # Construct the environment string for this command @@ -447,11 +441,10 @@ def enable_remote_rsyslog(server = 'rsyslog.ops.puppetlabs.net', port = 514) def which(command) unless @which_command if execute('type -P true', :accept_all_exit_codes => true).empty? - if execute('which true', :accept_all_exit_codes => true).empty? - raise ArgumentError, "Could not find suitable 'which' command" - else - @which_command = 'which' - end + raise ArgumentError, "Could not find suitable 'which' command" if execute('which true', :accept_all_exit_codes => true).empty? + + @which_command = 'which' + else @which_command = 'type -P' end diff --git a/lib/beaker/host/unix/file.rb b/lib/beaker/host/unix/file.rb index 1f3c51514..b01d8d171 100644 --- a/lib/beaker/host/unix/file.rb +++ b/lib/beaker/host/unix/file.rb @@ -110,7 +110,7 @@ def package_config_dir # @return [String] Filename of the repo def repo_filename(package_name, build_version) variant, version, arch, codename = self['platform'].to_array - repo_filename = "pl-%s-%s-" % [package_name, build_version] + repo_filename = format("pl-%s-%s-", package_name, build_version) case variant when /fedora|el|redhat|centos|cisco_nexus|cisco_ios_xr|opensuse|sles/ @@ -130,15 +130,10 @@ def repo_filename(package_name, build_version) pattern = "%s-%s%s-%s.repo" - repo_filename << (pattern % [ - variant, - fedora_prefix, - version, - arch, - ]) + repo_filename << (format(pattern, variant, fedora_prefix, version, arch)) when /debian|ubuntu|cumulus|huaweios/ codename = variant if variant == 'cumulus' || variant == 'huaweios' - repo_filename << ("%s.list" % [codename]) + repo_filename << (format("%s.list", codename)) else msg = "#repo_filename: repo filename pattern not known for platform '#{self['platform']}'" raise ArgumentError, msg diff --git a/lib/beaker/host/unix/pkg.rb b/lib/beaker/host/unix/pkg.rb index a1c5e56de..819b89e85 100644 --- a/lib/beaker/host/unix/pkg.rb +++ b/lib/beaker/host/unix/pkg.rb @@ -48,9 +48,7 @@ def check_for_package(name, opts = {}) result = execute("pkg info #{name}", opts) { |result| result } when /solaris-10/ result = execute("pkginfo #{name}", opts) { |result| result } - if result.exit_code == 1 - result = execute("pkginfo CSW#{name}", opts) { |result| result } - end + result = execute("pkginfo CSW#{name}", opts) { |result| result } if result.exit_code == 1 when /openbsd/ result = execute("pkg_info #{name}", opts) { |result| result } when /archlinux/ @@ -64,25 +62,23 @@ def check_for_package(name, opts = {}) # If apt has not been updated since the last repo deployment it is # updated. Otherwise this is a noop def update_apt_if_needed - if /debian|ubuntu|cumulus|huaweios/.match?(self['platform']) - if @apt_needs_update - execute("apt-get update") - @apt_needs_update = false - end - end + return unless /debian|ubuntu|cumulus|huaweios/.match?(self['platform']) + return unless @apt_needs_update + + execute("apt-get update") + @apt_needs_update = false end # Arch Linux is a rolling release distribution. We need to ensure that it is up2date # Except for the kernel. An upgrade will purge the modules for the currently running kernel # Before upgrading packages, we need to ensure we've the latest keyring def update_pacman_if_needed - if self['platform'].include?('archlinux') - if @pacman_needs_update - execute("pacman --sync --noconfirm --noprogressbar --refresh archlinux-keyring") - execute("pacman --sync --noconfirm --noprogressbar --refresh --sysupgrade --ignore linux --ignore linux-docs --ignore linux-headers") - @pacman_needs_update = false - end - end + return unless self['platform'].include?('archlinux') + return unless @pacman_needs_update + + execute("pacman --sync --noconfirm --noprogressbar --refresh archlinux-keyring") + execute("pacman --sync --noconfirm --noprogressbar --refresh --sysupgrade --ignore linux --ignore linux-docs --ignore linux-headers") + @pacman_needs_update = false end def install_package(name, cmdline_args = '', version = nil, opts = {}) @@ -92,19 +88,13 @@ def install_package(name, cmdline_args = '', version = nil, opts = {}) when /el-4/ @logger.debug("Package installation not supported on rhel4") when /fedora-(2[2-9]|3[0-9])/ - if version - name = "#{name}-#{version}" - end + name = "#{name}-#{version}" if version execute("dnf -y #{cmdline_args} install #{name}", opts) when /cisco|fedora|centos|redhat|eos|el-/ - if version - name = "#{name}-#{version}" - end + name = "#{name}-#{version}" if version execute("yum -y #{cmdline_args} install #{name}", opts) when /ubuntu|debian|cumulus|huaweios/ - if version - name = "#{name}=#{version}" - end + name = "#{name}=#{version}" if version update_apt_if_needed execute("apt-get install --force-yes #{cmdline_args} -y #{name}", opts) when /solaris-11/ @@ -172,9 +162,7 @@ def install_package(name, cmdline_args = '', version = nil, opts = {}) # @api public def install_package_with_rpm(name, cmdline_args = '', opts = {}) proxy = '' - if name&.start_with?('http') and opts[:package_proxy] - proxy = extract_rpm_proxy_options(opts[:package_proxy]) - end + proxy = extract_rpm_proxy_options(opts[:package_proxy]) if name&.start_with?('http') and opts[:package_proxy] execute("rpm #{cmdline_args} -Uvh #{name} #{proxy}") end diff --git a/lib/beaker/host/windows.rb b/lib/beaker/host/windows.rb index dc200b8e0..76d0b07c9 100644 --- a/lib/beaker/host/windows.rb +++ b/lib/beaker/host/windows.rb @@ -44,9 +44,7 @@ def determine_ssh_server @ssh_server = :bitvise else status = execute('cmd.exe /c sc qc sshd', :accept_all_exit_codes => true) - if status&.include?('C:\\Windows\\System32\\OpenSSH\\sshd.exe') - @ssh_server = :win32_openssh - end + @ssh_server = :win32_openssh if status&.include?('C:\\Windows\\System32\\OpenSSH\\sshd.exe') end @ssh_server end diff --git a/lib/beaker/host/windows/exec.rb b/lib/beaker/host/windows/exec.rb index 1e54dfbdb..702d1d87a 100644 --- a/lib/beaker/host/windows/exec.rb +++ b/lib/beaker/host/windows/exec.rb @@ -54,9 +54,7 @@ def ping target, attempts = 5 try = 0 while try < attempts do result = exec(Beaker::Command.new("ping -n 1 #{target}"), :accept_all_exit_codes => true) - if result.exit_code == 0 - return true - end + return true if result.exit_code == 0 try += 1 end @@ -86,7 +84,7 @@ def ssh_service_restart # (from {#ssh_service_restart}). def ssh_permit_user_environment exec(Beaker::Command.new("echo '\nPermitUserEnvironment yes' >> /etc/sshd_config")) - ssh_service_restart() + ssh_service_restart end # Gets the specific prepend commands as needed for this host diff --git a/lib/beaker/host_prebuilt_steps.rb b/lib/beaker/host_prebuilt_steps.rb index a0d3d019b..6f358a6ec 100644 --- a/lib/beaker/host_prebuilt_steps.rb +++ b/lib/beaker/host_prebuilt_steps.rb @@ -69,11 +69,10 @@ def timesync host, opts end sleep SLEEPWAIT end - if success - logger.notify "NTP date succeeded on #{host} after #{try} tries" - else - raise "NTP date was not successful after #{try} tries" - end + raise "NTP date was not successful after #{try} tries" unless success + + logger.notify "NTP date succeeded on #{host} after #{try} tries" + end end nil @@ -212,9 +211,7 @@ def sync_root_keys host, opts # @param [Host, Array] hosts One or more hosts to act upon def apt_get_update hosts block_on hosts do |host| - if /ubuntu|debian|cumulus/.match?(host[:platform]) - host.exec(Command.new("apt-get update")) - end + host.exec(Command.new("apt-get update")) if /ubuntu|debian|cumulus/.match?(host[:platform]) end end @@ -256,9 +253,9 @@ def get_domain_name(host) return_value ||= domain return_value ||= search - if return_value - return_value.gsub(/\.$/, '') - end + return unless return_value + + return_value.gsub(/\.$/, '') end # Append the provided string to the /etc/hosts file of the provided host @@ -273,13 +270,13 @@ def set_etc_hosts(host, etc_hosts) host.exec(Command.new("echo '#{etc_hosts}' >> /etc/hosts")) end # AIX must be configured to prefer local DNS over external - if host['platform'].include?('aix') - aix_netsvc = '/etc/netsvc.conf' - aix_local_resolv = 'hosts = local, bind' - unless host.exec(Command.new("grep '#{aix_local_resolv}' #{aix_netsvc}"), :accept_all_exit_codes => true).exit_code == 0 - host.exec(Command.new("echo '#{aix_local_resolv}' >> #{aix_netsvc}")) - end - end + return unless host['platform'].include?('aix') + + aix_netsvc = '/etc/netsvc.conf' + aix_local_resolv = 'hosts = local, bind' + return if host.exec(Command.new("grep '#{aix_local_resolv}' #{aix_netsvc}"), :accept_all_exit_codes => true).exit_code == 0 + + host.exec(Command.new("echo '#{aix_local_resolv}' >> #{aix_netsvc}")) end # Make it possible to log in as root by copying the current users ssh keys to the root account @@ -314,9 +311,7 @@ def copy_ssh_to_root host, opts host.exec(Command.new('sudo su -c "cp -r .ssh /root/."'), { :pty => true }) end - if host.selinux_enabled? - host.exec(Command.new('sudo fixfiles restore /root')) - end + host.exec(Command.new('sudo fixfiles restore /root')) if host.selinux_enabled? end end @@ -478,9 +473,7 @@ def construct_env host, opts env.each_key do |key| separator = host['pathseparator'] - if key == 'PATH' && (not host.is_powershell?) - separator = ':' - end + separator = ':' if key == 'PATH' && (not host.is_powershell?) env[key] = env[key].join(separator) end env @@ -503,9 +496,7 @@ def set_env host, opts logger.debug("setting local environment on #{host.name}") - if host['platform'].include?('windows') && host.is_cygwin? - env['CYGWIN'] = 'nodosfilewarning' - end + env['CYGWIN'] = 'nodosfilewarning' if host['platform'].include?('windows') && host.is_cygwin? host.ssh_permit_user_environment host.ssh_set_user_environment(env) diff --git a/lib/beaker/hypervisor.rb b/lib/beaker/hypervisor.rb index 9b2866ad4..89ce607ed 100644 --- a/lib/beaker/hypervisor.rb +++ b/lib/beaker/hypervisor.rb @@ -74,9 +74,9 @@ def self.set_ssh_connection_preference(hosts_to_provision, hypervisor) # Proxy package managers on tests hosts created by this hypervisor, runs before validation and configuration. def proxy_package_manager - if @options[:package_proxy] - package_proxy(@hosts, @options) - end + return unless @options[:package_proxy] + + package_proxy(@hosts, @options) end # Default configuration steps to be run for a given hypervisor. Any additional configuration to be done @@ -87,23 +87,13 @@ def configure(opts = {}) run_in_parallel = run_in_parallel? opts, @options, 'configure' block_on @hosts, { :run_in_parallel => run_in_parallel } do |host| - if host[:timesync] - timesync(host, @options) - end - end - if @options[:root_keys] - sync_root_keys(@hosts, @options) - end - if @options[:set_env] - set_env(@hosts, @options) - end - if @options[:disable_updates] - disable_updates(@hosts, @options) + timesync(host, @options) if host[:timesync] end + sync_root_keys(@hosts, @options) if @options[:root_keys] + set_env(@hosts, @options) if @options[:set_env] + disable_updates(@hosts, @options) if @options[:disable_updates] rescue SignalException => e - if e.signo == 15 # SIGTERM - report_and_raise(@logger, e, "configure") - end + report_and_raise(@logger, e, "configure") if e.signo == 15 # SIGTERM raise end end @@ -111,18 +101,16 @@ def configure(opts = {}) # Default validation steps to be run for a given hypervisor. Ensures that SUTs meet requirements to be # beaker test nodes. def validate - if @options[:validate] - validate_host(@hosts, @options) - end + return unless @options[:validate] + + validate_host(@hosts, @options) end # Generate a random string composted of letter and numbers # prefixed with value of {Beaker::Hypervisor::create} option :host_name_prefix def generate_host_name n = CHARMAP[rand(25)] + (0...14).map { CHARMAP[rand(CHARMAP.length)] }.join - if @options[:host_name_prefix] - return @options[:host_name_prefix] + n - end + return @options[:host_name_prefix] + n if @options[:host_name_prefix] n end diff --git a/lib/beaker/logger.rb b/lib/beaker/logger.rb index 9b99d7f90..f3dec22b2 100644 --- a/lib/beaker/logger.rb +++ b/lib/beaker/logger.rb @@ -347,7 +347,9 @@ def optionally_color color_code, msg, add_newline = true @destinations.each do |to| to.print color_code if @color to.send print_statement, msg - to.print NORMAL if @color unless color_code == NONE + unless color_code == NONE + to.print NORMAL if @color + end to.flush end end @@ -365,9 +367,7 @@ def pretty_backtrace backtrace = caller(1) # Create a new StringIO log to track the current output def start_sublog - if @sublog - remove_destination(@sublog) - end + remove_destination(@sublog) if @sublog @sublog = StringIO.new add_destination(@sublog) end diff --git a/lib/beaker/logger_junit.rb b/lib/beaker/logger_junit.rb index 7fb48bc74..f77a03454 100644 --- a/lib/beaker/logger_junit.rb +++ b/lib/beaker/logger_junit.rb @@ -67,9 +67,9 @@ def self.get_xml_contents(xml_file, name, stylesheet) # # @return nil def self.copy_stylesheet_into_xml_dir(stylesheet, xml_file) - if not File.file?(File.join(File.dirname(xml_file), File.basename(stylesheet))) - FileUtils.copy(stylesheet, File.join(File.dirname(xml_file), File.basename(stylesheet))) - end + return if File.file?(File.join(File.dirname(xml_file), File.basename(stylesheet))) + + FileUtils.copy(stylesheet, File.join(File.dirname(xml_file), File.basename(stylesheet))) end # sets up doc & gives us the suites for the testsuite named @@ -85,9 +85,7 @@ def self.get_testsuites_from_doc(doc, name, already_existed) suites = REXML::XPath.first(doc, "testsuites") # remove old data suites.elements.each("testsuite") do |e| - if /#{name}/.match?(e.name) - suites.delete_element e - end + suites.delete_element e if /#{name}/.match?(e.name) end else suites = doc.add_element(REXML::Element.new('testsuites')) diff --git a/lib/beaker/network_manager.rb b/lib/beaker/network_manager.rb index 45507d915..da6ae51f1 100644 --- a/lib/beaker/network_manager.rb +++ b/lib/beaker/network_manager.rb @@ -47,9 +47,7 @@ def initialize(options, logger) # Provision all virtual machines. Provision machines according to their set hypervisor, if no hypervisor # is selected assume that the described hosts are already up and reachable and do no provisioning. def provision - if @hypervisors - cleanup - end + cleanup if @hypervisors @hypervisors = {} # sort hosts by their hypervisor, use hypervisor 'none' if no hypervisor is specified hostless_options = Beaker::Options::OptionsHash.new.merge(@options.select { |k, _v| !k.to_s.include?('HOSTS') }) @@ -82,30 +80,30 @@ def provision # attempt to add them. # @raise [Exception] Raise an exception if virtual machines fail to be validated def validate - if @hypervisors - @hypervisors.each_key do |type| - @hypervisors[type].validate - end + return unless @hypervisors + + @hypervisors.each_key do |type| + @hypervisors[type].validate end end # Configure all provisioned machines, adding any packages or settings required for SUTs # @raise [Exception] Raise an exception if virtual machines fail to be configured def configure - if @hypervisors - @hypervisors.each_key do |type| - @hypervisors[type].configure - end + return unless @hypervisors + + @hypervisors.each_key do |type| + @hypervisors[type].configure end end # configure proxy on all provioned machines # @raise [Exception] Raise an exception if virtual machines fail to be configured def proxy_package_manager - if @hypervisors - @hypervisors.each_key do |type| - @hypervisors[type].proxy_package_manager - end + return unless @hypervisors + + @hypervisors.each_key do |type| + @hypervisors[type].proxy_package_manager end end diff --git a/lib/beaker/options/command_line_parser.rb b/lib/beaker/options/command_line_parser.rb index 8f72fb142..a8120882c 100644 --- a/lib/beaker/options/command_line_parser.rb +++ b/lib/beaker/options/command_line_parser.rb @@ -152,9 +152,7 @@ def initialize 'Ensure SUT colored output is preserved', '(default: false)' do |bool| @cmd_options[:color_host_output] = bool - if bool - @cmd_options[:color_host_output] = true - end + @cmd_options[:color_host_output] = true if bool end opts.on '--log-level LEVEL', diff --git a/lib/beaker/options/hosts_file_parser.rb b/lib/beaker/options/hosts_file_parser.rb index d1ad2703a..f0452bc07 100644 --- a/lib/beaker/options/hosts_file_parser.rb +++ b/lib/beaker/options/hosts_file_parser.rb @@ -82,9 +82,7 @@ def self.fix_roles_array(host_options) host_options['HOSTS'].each_key do |host| host_options['HOSTS'][host]['roles'] ||= [] end - if host_options.has_key?('CONFIG') - host_options = host_options.merge(host_options.delete('CONFIG')) - end + host_options = host_options.merge(host_options.delete('CONFIG')) if host_options.has_key?('CONFIG') host_options end diff --git a/lib/beaker/options/options_file_parser.rb b/lib/beaker/options/options_file_parser.rb index 5f1be2b6b..571b06afb 100644 --- a/lib/beaker/options/options_file_parser.rb +++ b/lib/beaker/options/options_file_parser.rb @@ -27,9 +27,7 @@ def self.parse_options_file(options_file_path) result = Beaker::Options::OptionsHash.new if options_file_path options_file_path = File.expand_path(options_file_path) - unless File.exist?(options_file_path) - raise ArgumentError, "Specified options file '#{options_file_path}' does not exist!" - end + raise ArgumentError, "Specified options file '#{options_file_path}' does not exist!" unless File.exist?(options_file_path) # This eval will allow the specified options file to have access to our # scope. It is important that the variable 'options_file_path' is diff --git a/lib/beaker/options/options_hash.rb b/lib/beaker/options/options_hash.rb index 8269a1a4e..2ed774bb7 100644 --- a/lib/beaker/options/options_hash.rb +++ b/lib/beaker/options/options_hash.rb @@ -36,10 +36,8 @@ def get_type def dump_to_file(output_file) dirname = File.dirname(output_file) - unless File.directory?(dirname) - FileUtils.mkdir_p(dirname) - end - File.open(output_file, 'w+') { |f| f.write(dump) } + FileUtils.mkdir_p(dirname) unless File.directory?(dirname) + File.write(output_file, dump) end end end diff --git a/lib/beaker/options/parser.rb b/lib/beaker/options/parser.rb index 543df3904..a2d591d83 100644 --- a/lib/beaker/options/parser.rb +++ b/lib/beaker/options/parser.rb @@ -91,9 +91,9 @@ def file_list(paths) # @return nil # @api public def resolve_symlinks! - if @options[:hosts_file] && !@options[:hosts_file_generated] - @options[:hosts_file] = File.realpath(@options[:hosts_file]) - end + return unless @options[:hosts_file] && !@options[:hosts_file_generated] + + @options[:hosts_file] = File.realpath(@options[:hosts_file]) end # Converts array of paths into array of fully qualified git repo URLS with expanded keywords @@ -147,17 +147,17 @@ def set_default_host!(hosts) # default_set? will throw an error if length > 1 # and return false if no default is set. - if !@validator.default_set?(default) - # no default set, let's make one - if not master.empty? and master.length == 1 - default_host_name = master[0] - elsif hosts.length == 1 - default_host_name = hosts.keys[0] - end - if default_host_name - hosts[default_host_name][:roles] << 'default' - end + return if @validator.default_set?(default) + + # no default set, let's make one + if not master.empty? and master.length == 1 + default_host_name = master[0] + elsif hosts.length == 1 + default_host_name = hosts.keys[0] end + return unless default_host_name + + hosts[default_host_name][:roles] << 'default' end # Constructor for Parser @@ -224,7 +224,7 @@ def parse_args(args = ARGV) subcommand_options_file = Beaker::Subcommands::SubcommandUtil::SUBCOMMAND_OPTIONS { "project" => ".beaker.yml", - "homedir" => "#{ENV['HOME']}/#{subcommand_options_file}", + "homedir" => "#{ENV.fetch('HOME', nil)}/#{subcommand_options_file}", "subcommand" => subcommand_options_file, }.each_pair do |src, path| opts = if src == "project" @@ -330,21 +330,15 @@ def normalize_args end # use the keyfile if present - if @options.has_key?(:keyfile) - @options[:ssh][:keys] = [@options[:keyfile]] - end + @options[:ssh][:keys] = [@options[:keyfile]] if @options.has_key?(:keyfile) # split out arguments - these arguments can have the form of arg1,arg2 or [arg] or just arg # will end up being normalized into an array LONG_OPTS.each do |opt| if @options.has_key?(opt) update_option(opt, split_arg(@options[opt]), 'runtime') - if RB_FILE_OPTS.include?(opt) && (not @options[opt] == []) - update_option(opt, file_list(@options[opt]), 'runtime') - end - if opt == :install - update_option(:install, parse_git_repos(@options[:install]), 'runtime') - end + update_option(opt, file_list(@options[opt]), 'runtime') if RB_FILE_OPTS.include?(opt) && (not @options[opt] == []) + update_option(:install, parse_git_repos(@options[:install]), 'runtime') if opt == :install else update_option(opt, [], 'runtime') end @@ -373,14 +367,10 @@ def normalize_args # check that windows/el-4 boxes are only agents (solaris can be a master in foss cases) @options[:HOSTS].each_key do |name| host = @options[:HOSTS][name] - if /windows|el-4/.match?(host[:platform]) - test_host_roles(name, host) - end + test_host_roles(name, host) if /windows|el-4/.match?(host[:platform]) # check to see if a custom user account has been provided, if so use it - if host[:ssh] && host[:ssh][:user] - host[:user] = host[:ssh][:user] - end + host[:user] = host[:ssh][:user] if host[:ssh] && host[:ssh][:user] # merge host tags for this host with the global/preset host tags host[:host_tags] = @options[:host_tags].merge(host[:host_tags] || {}) @@ -426,13 +416,11 @@ def get_hypervisors(hosts) # @return [nil] no return # @raise [ArgumentError] Raises error if config file does not exist or is not valid YAML def check_hypervisor_config(visor) - if ['blimpy'].include?(visor) - @validator.check_yaml_file(@options[:ec2_yaml], "required by #{visor}") - end + @validator.check_yaml_file(@options[:ec2_yaml], "required by #{visor}") if ['blimpy'].include?(visor) - if %w(aix solaris vcloud).include?(visor) - @validator.check_yaml_file(@options[:dot_fog], "required by #{visor}") - end + return unless %w(aix solaris vcloud).include?(visor) + + @validator.check_yaml_file(@options[:dot_fog], "required by #{visor}") end # Normalize include and exclude tags. This modifies @options. @@ -457,9 +445,9 @@ def normalize_test_tags! def test_host_roles(host_name, host_hash) exclude_roles = %w(master database dashboard) host_roles = host_hash[:roles] - unless (host_roles & exclude_roles).empty? - @validator.parser_error "#{host_hash[:platform]} box '#{host_name}' may not have roles: #{exclude_roles.join(', ')}." - end + return if (host_roles & exclude_roles).empty? + + @validator.parser_error "#{host_hash[:platform]} box '#{host_name}' may not have roles: #{exclude_roles.join(', ')}." end end end diff --git a/lib/beaker/options/presets.rb b/lib/beaker/options/presets.rb index 97ae2dbc1..88089143e 100644 --- a/lib/beaker/options/presets.rb +++ b/lib/beaker/options/presets.rb @@ -44,9 +44,7 @@ class Presets def select_env_by_regex regex envs = Beaker::Options::OptionsHash.new ENV.each_pair do |k, v| - if /#{regex}/.match?(k.to_s) - envs[k] = v - end + envs[k] = v if /#{regex}/.match?(k.to_s) end envs end @@ -60,8 +58,8 @@ def collect_env_vars(env_var_spec) env_var_spec.each_with_object({}) do |key_value, memo| key, value = key_value[0], key_value[1] - set_env_var = Array(value).detect { |possible_variable| ENV[possible_variable] } - memo[key] = ENV[set_env_var] if set_env_var + set_env_var = Array(value).detect { |possible_variable| ENV.fetch(possible_variable, nil) } + memo[key] = ENV.fetch(set_env_var, nil) if set_env_var end end @@ -85,9 +83,7 @@ def format_found_env_vars(found_env_vars) found_env_vars[:type] = type end - if found_env_vars[:run_in_parallel] - found_env_vars[:run_in_parallel] = found_env_vars[:run_in_parallel].split(',') - end + found_env_vars[:run_in_parallel] = found_env_vars[:run_in_parallel].split(',') if found_env_vars[:run_in_parallel] found_env_vars[:pe_version_file_win] = found_env_vars[:pe_version_file] found_env_vars @@ -124,13 +120,13 @@ def presets :department => 'unknown', :created_by => ENV['USER'] || ENV['USERNAME'] || 'unknown', :host_tags => {}, - :openstack_api_key => ENV['OS_PASSWORD'], - :openstack_username => ENV['OS_USERNAME'], - :openstack_auth_url => "#{ENV['OS_AUTH_URL']}/tokens", - :openstack_tenant => ENV['OS_TENANT_NAME'], - :openstack_keyname => ENV['OS_KEYNAME'], - :openstack_network => ENV['OS_NETWORK'], - :openstack_region => ENV['OS_REGION'], + :openstack_api_key => ENV.fetch('OS_PASSWORD', nil), + :openstack_username => ENV.fetch('OS_USERNAME', nil), + :openstack_auth_url => "#{ENV.fetch('OS_AUTH_URL', nil)}/tokens", + :openstack_tenant => ENV.fetch('OS_TENANT_NAME', nil), + :openstack_keyname => ENV.fetch('OS_KEYNAME', nil), + :openstack_network => ENV.fetch('OS_NETWORK', nil), + :openstack_region => ENV.fetch('OS_REGION', nil), :openstack_volume_support => ENV['OS_VOLUME_SUPPORT'] || true, :jenkins_build_url => nil, :validate => true, @@ -177,7 +173,7 @@ def presets :host_name_prefix => nil, :ssh_env_file => '~/.ssh/environment', :profile_d_env_file => '/etc/profile.d/beaker_env.sh', - :dot_fog => File.join(ENV['HOME'], '.fog'), + :dot_fog => File.join(ENV.fetch('HOME', nil), '.fog'), :ec2_yaml => 'config/image_templates/ec2.yaml', :help => false, :collect_perf_data => 'none', @@ -195,8 +191,8 @@ def presets :auth_methods => ["publickey"], :port => 22, :forward_agent => true, - :keys => ["#{ENV['HOME']}/.ssh/id_rsa"], - :user_known_hosts_file => "#{ENV['HOME']}/.ssh/known_hosts", + :keys => ["#{ENV.fetch('HOME', nil)}/.ssh/id_rsa"], + :user_known_hosts_file => "#{ENV.fetch('HOME', nil)}/.ssh/known_hosts", :keepalive => true, }, }) diff --git a/lib/beaker/options/subcommand_options_file_parser.rb b/lib/beaker/options/subcommand_options_file_parser.rb index 4e96bbb22..7a1c2805e 100644 --- a/lib/beaker/options/subcommand_options_file_parser.rb +++ b/lib/beaker/options/subcommand_options_file_parser.rb @@ -4,9 +4,7 @@ module Options module SubcommandOptionsParser def self.parse_options_file(options_file_path) result = OptionsHash.new - if File.exist?(options_file_path) - result = YAML.load_file(options_file_path) - end + result = YAML.load_file(options_file_path) if File.exist?(options_file_path) result end diff --git a/lib/beaker/options/validator.rb b/lib/beaker/options/validator.rb index 5a63c515f..091bfdcbd 100644 --- a/lib/beaker/options/validator.rb +++ b/lib/beaker/options/validator.rb @@ -51,9 +51,9 @@ def default_set?(default) # @return [nil] Does not return anything def validate_fail_mode(fail_mode) # check for valid fail mode - unless fail_mode.is_a?(String) && VALID_FAIL_MODES.match?(fail_mode) - validator_error "--fail-mode must be one of fast or slow, not '#{fail_mode}'" - end + return if fail_mode.is_a?(String) && VALID_FAIL_MODES.match?(fail_mode) + + validator_error "--fail-mode must be one of fast or slow, not '#{fail_mode}'" end # Raises an error if hosts_setting is not a supported preserve hosts value. @@ -62,9 +62,9 @@ def validate_fail_mode(fail_mode) # @return [nil] Does not return anything def validate_preserve_hosts(hosts_setting) # check for valid preserve_hosts option - unless hosts_setting.is_a?(String) && VALID_PRESERVE_HOSTS.match?(hosts_setting) - validator_error("--preserve_hosts must be one of always, onfail, onpass or never, not '#{hosts_setting}'") - end + return if hosts_setting.is_a?(String) && VALID_PRESERVE_HOSTS.match?(hosts_setting) + + validator_error("--preserve_hosts must be one of always, onfail, onpass or never, not '#{hosts_setting}'") end # Raise an error if host does not have a platform defined. @@ -73,9 +73,9 @@ def validate_preserve_hosts(hosts_setting) # @param [String] name Host name # @return [nil] Does not return anything def validate_platform(host, name) - if !host['platform'] || host['platform'].empty? - validator_error "Host #{name} does not have a platform specified" - end + return unless !host['platform'] || host['platform'].empty? + + validator_error "Host #{name} does not have a platform specified" end # Raise an error if an item exists in both the include and exclude lists. @@ -86,16 +86,12 @@ def validate_platform(host, name) # @param [Array] tags_exclude excluded items # @return [nil] Does not return anything def validate_test_tags(tags_and, tags_or, tags_exclude) - if tags_and.length > 0 && tags_or.length > 0 - validator_error "cannot have values for both test tagging operands (AND and OR)" - end + validator_error "cannot have values for both test tagging operands (AND and OR)" if tags_and.length > 0 && tags_or.length > 0 tags_and.each do |included_tag| # select items from exclude set that match included_tag # no match is an empty list/array/[] - if tags_exclude.select { |ex| ex == included_tag } != [] - validator_error "tag '#{included_tag}' cannot be in both the included and excluded tag sets" - end + validator_error "tag '#{included_tag}' cannot be in both the included and excluded tag sets" if tags_exclude.select { |ex| ex == included_tag } != [] end end @@ -104,9 +100,9 @@ def validate_test_tags(tags_and, tags_or, tags_exclude) # @param [Array] role_array List of roles # @raise [ArgumentError] Raises if role_array contains conflicting roles def validate_frictionless_roles(role_array) - if role_array.include?(FRICTIONLESS_ROLE) and !(role_array & FRICTIONLESS_ADDITIONAL_ROLES).empty? - validator_error "Only agent nodes may have the role 'frictionless'." - end + return unless role_array.include?(FRICTIONLESS_ROLE) and !(role_array & FRICTIONLESS_ADDITIONAL_ROLES).empty? + + validator_error "Only agent nodes may have the role 'frictionless'." end # Raise an error if the master count is incorrect. @@ -115,9 +111,9 @@ def validate_frictionless_roles(role_array) # @return [nil] Nothing is returned # @raise [ArgumentError] Raises if master count is greater than 1 def validate_master_count(count) - if count > 1 - validator_error("Only one host/node may have the role 'master'.") - end + return unless count > 1 + + validator_error("Only one host/node may have the role 'master'.") end # Raise an error if file_list is empty @@ -126,9 +122,9 @@ def validate_master_count(count) # @param [String] path file path to report in error # @raise [ArgumentError] Raises if file_list is empty def validate_files(file_list, path) - if file_list.empty? - validator_error("No files found for path: '#{path}'") - end + return unless file_list.empty? + + validator_error("No files found for path: '#{path}'") end # Raise an error if path is not a valid file or directory @@ -136,9 +132,9 @@ def validate_files(file_list, path) # @param [String] path File path # @raise [ArgumentError] Raises if path is not a valid file or directory def validate_path(path) - if !File.file?(path) && !File.directory?(path) - validator_error("#{path} used as a file option but is not a file or directory!") - end + return unless !File.file?(path) && !File.directory?(path) + + validator_error("#{path} used as a file option but is not a file or directory!") end end end diff --git a/lib/beaker/perf.rb b/lib/beaker/perf.rb index d943eade1..345e98ab9 100644 --- a/lib/beaker/perf.rb +++ b/lib/beaker/perf.rb @@ -30,9 +30,7 @@ def setup_perf_on_host(host) # Install sysstat if required if PERF_SUPPORTED_PLATFORMS.match?(host['platform']) PERF_PACKAGES.each do |pkg| - if not host.check_for_package pkg - host.install_package pkg - end + host.install_package pkg if not host.check_for_package pkg end else @logger.perf_output("Perf (sysstat) not supported on host: " + host) @@ -53,9 +51,9 @@ def setup_perf_on_host(host) host.exec(Command.new('sed -i s/*\\\/10/*/ /etc/cron.d/sysstat')) end end - if PERF_START_PLATFORMS.match?(host['platform']) # SLES doesn't need this step - host.exec(Command.new('service sysstat start')) - end + return unless PERF_START_PLATFORMS.match?(host['platform']) # SLES doesn't need this step + + host.exec(Command.new('service sysstat start')) end # Iterate over all hosts, calling get_perf_data @@ -73,9 +71,7 @@ def print_perf_info def get_perf_data(host, perf_start, perf_end) @logger.perf_output("Getting perf data for host: " + host) if PERF_SUPPORTED_PLATFORMS.match?(host['platform']) # All flavours of Linux - if not @options[:collect_perf_data]&.include?('aggressive') - host.exec(Command.new("sar -A -s #{perf_start.strftime("%H:%M:%S")} -e #{perf_end.strftime("%H:%M:%S")}"), :acceptable_exit_codes => [0, 1, 2]) - end + host.exec(Command.new("sar -A -s #{perf_start.strftime("%H:%M:%S")} -e #{perf_end.strftime("%H:%M:%S")}"), :acceptable_exit_codes => [0, 1, 2]) if not @options[:collect_perf_data]&.include?('aggressive') if (defined? @options[:graphite_server] and not @options[:graphite_server].nil?) and (defined? @options[:graphite_perf_data] and not @options[:graphite_perf_data].nil?) export_perf_data_to_graphite(host) diff --git a/lib/beaker/platform.rb b/lib/beaker/platform.rb index acb21a2e8..8bac957de 100644 --- a/lib/beaker/platform.rb +++ b/lib/beaker/platform.rb @@ -78,9 +78,7 @@ class Platform < String # * netscaler # * archlinux def initialize(name) - if !PLATFORMS.match?(name) - raise ArgumentError, "Unsupported platform name #{name}" - end + raise ArgumentError, "Unsupported platform name #{name}" if !PLATFORMS.match?(name) super @@ -90,15 +88,15 @@ def initialize(name) @version = version @codename = nil - if codename_version_hash - if codename_version_hash[version] - @codename = version - @version = codename_version_hash[version] - else - version = version.delete('.') - version_codename_hash = codename_version_hash.invert - @codename = version_codename_hash[version] - end + return unless codename_version_hash + + if codename_version_hash[version] + @codename = version + @version = codename_version_hash[version] + else + version = version.delete('.') + version_codename_hash = codename_version_hash.invert + @codename = version_codename_hash[version] end end @@ -114,9 +112,7 @@ def to_array # @return [String] the platform string with the platform version represented as a codename def with_version_codename version_array = [@variant, @version, @arch] - if @codename - version_array = [@variant, @codename, @arch] - end + version_array = [@variant, @codename, @arch] if @codename return version_array.join('-') end diff --git a/lib/beaker/shared/fog_credentials.rb b/lib/beaker/shared/fog_credentials.rb index e322b7526..63976744d 100644 --- a/lib/beaker/shared/fog_credentials.rb +++ b/lib/beaker/shared/fog_credentials.rb @@ -41,20 +41,14 @@ def get_fog_credentials(fog_file_path = '~/.fog', credential_group = :default) rescue Psych::SyntaxError, Errno::ENOENT => e raise fog_credential_error fog_file_path, from_env, "(#{e.class}) #{e.message}" end - if fog == false # YAML.load => false for empty file - raise fog_credential_error fog_file_path, from_env, "is empty." - end + raise fog_credential_error fog_file_path, from_env, "is empty." if fog == false # YAML.load => false for empty file # transparently support symbols or strings for keys fog = StringifyHash.new.merge!(fog) # respect credential from env # @note ENV must be a string, e.g. "default" not ":default" - if ENV["FOG_CREDENTIAL"] - credential_group = ENV["FOG_CREDENTIAL"].to_sym - end - if not fog[credential_group] - raise fog_credential_error fog_file_path, from_env, "could not load the specified credential group '#{credential_group}'." - end + credential_group = ENV["FOG_CREDENTIAL"].to_sym if ENV["FOG_CREDENTIAL"] + raise fog_credential_error fog_file_path, from_env, "could not load the specified credential group '#{credential_group}'." if not fog[credential_group] fog[credential_group] end diff --git a/lib/beaker/shared/host_manager.rb b/lib/beaker/shared/host_manager.rb index e063e4083..4448515cc 100644 --- a/lib/beaker/shared/host_manager.rb +++ b/lib/beaker/shared/host_manager.rb @@ -91,17 +91,14 @@ def run_block_on hosts = [], filter = nil, opts = {}, &block result = nil block_hosts = hosts # the hosts to apply the block to after any filtering if filter - if not hosts.empty? - block_hosts = hosts_with_role(hosts, filter) # check by role - if block_hosts.empty? - block_hosts = hosts_with_name(hosts, filter) # check by name - end - if block_hosts.length == 1 # we only found one matching host, don't need it wrapped in an array - block_hosts = block_hosts.pop - end - else - raise ArgumentError, "Unable to sort for #{filter} type hosts when provided with [] as Hosts" + raise ArgumentError, "Unable to sort for #{filter} type hosts when provided with [] as Hosts" if hosts.empty? + + block_hosts = hosts_with_role(hosts, filter) # check by role + if block_hosts.empty? + block_hosts = hosts_with_name(hosts, filter) # check by name end + block_hosts = block_hosts.pop if block_hosts.length == 1 # we only found one matching host, don't need it wrapped in an array + end if block_hosts.is_a? Array if block_hosts.length > 0 @@ -118,13 +115,11 @@ def run_block_on hosts = [], filter = nil, opts = {}, &block run_block_on h, &block end end - else + elsif (cur_logger = (logger || @logger)) # there are no matching hosts to execute against # should warn here # check if logger is defined in this context - if (cur_logger = (logger || @logger)) - cur_logger.info "Attempting to execute against an empty array of hosts (#{hosts}, filtered to #{block_hosts}), no execution will occur" - end + cur_logger.info "Attempting to execute against an empty array of hosts (#{hosts}, filtered to #{block_hosts}), no execution will occur" end else result = yield block_hosts diff --git a/lib/beaker/shared/options_resolver.rb b/lib/beaker/shared/options_resolver.rb index e2e706bba..9e2ce902c 100644 --- a/lib/beaker/shared/options_resolver.rb +++ b/lib/beaker/shared/options_resolver.rb @@ -26,13 +26,9 @@ module OptionsResolver def run_in_parallel?(local_options = nil, global_options = nil, mode = nil) run_in_parallel = local_options[:run_in_parallel] unless local_options.nil? - if !run_in_parallel.nil? && run_in_parallel.is_a?(Array) - run_in_parallel = false - end + run_in_parallel = false if !run_in_parallel.nil? && run_in_parallel.is_a?(Array) - if run_in_parallel.nil? && global_options && global_options[:run_in_parallel].is_a?(Array) - run_in_parallel = global_options[:run_in_parallel].include?(mode) - end + run_in_parallel = global_options[:run_in_parallel].include?(mode) if run_in_parallel.nil? && global_options && global_options[:run_in_parallel].is_a?(Array) run_in_parallel end diff --git a/lib/beaker/shared/semvar.rb b/lib/beaker/shared/semvar.rb index 2de54dc25..48898a57e 100644 --- a/lib/beaker/shared/semvar.rb +++ b/lib/beaker/shared/semvar.rb @@ -15,13 +15,10 @@ def version_is_less a, b a_nums = a.split('-')[0].split('.') b_nums = b.split('-')[0].split('.') (0...a_nums.length).each do |i| - if i < b_nums.length - if a_nums[i].to_i < b_nums[i].to_i - return true - elsif a_nums[i].to_i > b_nums[i].to_i - return false - end - else + return false unless i < b_nums.length + if a_nums[i].to_i < b_nums[i].to_i + return true + elsif a_nums[i].to_i > b_nums[i].to_i return false end end @@ -39,29 +36,28 @@ def version_is_less a, b elsif !a_is_release && !b_is_release a_next = a_rest.shift b_next = b_rest.shift - if a_is_rc && b_is_rc - a_rc = a_next.gsub('rc', '').to_i - b_rc = b_next.gsub('rc', '').to_i - if a_rc < b_rc - return true - elsif a_rc > b_rc - return false - else - a_next = a_rest.shift - b_next = b_rest.shift - if a_next && b_next - return a_next.to_i < b_next.to_i - else - # If a has nothing after -rc#, it is a tagged RC and - # b must be a later build after this tag. - return a_next.nil? - end - end + return a_is_rc unless a_is_rc && b_is_rc + + a_rc = a_next.gsub('rc', '').to_i + b_rc = b_next.gsub('rc', '').to_i + if a_rc < b_rc + return true + elsif a_rc > b_rc + return false else - # If one of them is not an rc (and also not a release), - # that one is a post-release build. So if a is the RC, it is less. - return a_is_rc + a_next = a_rest.shift + b_next = b_rest.shift + return a_next.to_i < b_next.to_i if a_next && b_next + + # If a has nothing after -rc#, it is a tagged RC and + # b must be a later build after this tag. + return a_next.nil? + end + + # If one of them is not an rc (and also not a release), + # that one is a post-release build. So if a is the RC, it is less. + else return (b_is_release && a_is_rc) || (a_is_release && !b_is_rc) end diff --git a/lib/beaker/ssh_connection.rb b/lib/beaker/ssh_connection.rb index c86cc4ce7..1d8f4def1 100644 --- a/lib/beaker/ssh_connection.rb +++ b/lib/beaker/ssh_connection.rb @@ -289,9 +289,7 @@ def process_stdin_for channel, stdin def scp_to source, target, options = {} local_opts = options.dup - if local_opts[:recursive].nil? - local_opts[:recursive] = File.directory?(source) - end + local_opts[:recursive] = File.directory?(source) if local_opts[:recursive].nil? local_opts[:chunk_size] ||= 16384 result = Result.new(@hostname, [source, target]) @@ -303,7 +301,7 @@ def scp_to source, target, options = {} target = self.execute(%{echo "#{target}"}).output.strip.delete('"') if target.include?('%') @ssh.scp.upload! source, target, local_opts do |_ch, name, sent, total| - result.stdout << ("\tcopying %s: %10d/%d\n" % [name, sent, total]) + result.stdout << (format("\tcopying %s: %10d/%d\n", name, sent, total)) end rescue => e logger.warn "#{e.class} error in scp'ing. Forcing the connection to close, which should " << @@ -323,9 +321,7 @@ def scp_to source, target, options = {} def scp_from source, target, options = {} local_opts = options.dup - if local_opts[:recursive].nil? - local_opts[:recursive] = true - end + local_opts[:recursive] = true if local_opts[:recursive].nil? local_opts[:chunk_size] ||= 16384 result = Result.new(@hostname, [source, target]) @@ -337,7 +333,7 @@ def scp_from source, target, options = {} source = self.execute(%{echo "#{source}"}).output.strip.delete('"') if source.include?('%') @ssh.scp.download! source, target, local_opts do |_ch, name, sent, total| - result.stdout << ("\tcopying %s: %10d/%d\n" % [name, sent, total]) + result.stdout << (format("\tcopying %s: %10d/%d\n", name, sent, total)) end rescue => e logger.warn "#{e.class} error in scp'ing. Forcing the connection to close, which should " << diff --git a/lib/beaker/subcommand.rb b/lib/beaker/subcommand.rb index a6b75f1c9..88283e12d 100644 --- a/lib/beaker/subcommand.rb +++ b/lib/beaker/subcommand.rb @@ -86,9 +86,7 @@ def init options_to_write = SubcommandUtil.sanitize_options_for_save(@cli.configured_options) @cli.logger.notify 'Writing configured options to disk' - File.open(SubcommandUtil::SUBCOMMAND_OPTIONS, 'w') do |f| - f.write(options_to_write.to_yaml) - end + File.write(SubcommandUtil::SUBCOMMAND_OPTIONS, options_to_write.to_yaml) @cli.logger.notify "Options written to #{SubcommandUtil::SUBCOMMAND_OPTIONS}" state = YAML::Store.new(SubcommandUtil::SUBCOMMAND_STATE) @@ -111,9 +109,7 @@ def provision end state = YAML::Store.new(SubcommandUtil::SUBCOMMAND_STATE) - if state.transaction { state['provisioned'] } - SubcommandUtil.error_with('Provisioned SUTs detected. Please destroy and reprovision.') - end + SubcommandUtil.error_with('Provisioned SUTs detected. Please destroy and reprovision.') if state.transaction { state['provisioned'] } @cli.parse_options @cli.provision @@ -197,13 +193,13 @@ def exec(resource = nil) @cli.execute! - if options['preserve-state'] - @cli.logger.notify 'updating HOSTS key in subcommand_options' - hosts = SubcommandUtil.sanitize_options_for_save(@cli.combined_instance_and_options_hosts) - options_storage = YAML::Store.new(SubcommandUtil::SUBCOMMAND_OPTIONS) - options_storage.transaction do - options_storage['HOSTS'] = hosts - end + return unless options['preserve-state'] + + @cli.logger.notify 'updating HOSTS key in subcommand_options' + hosts = SubcommandUtil.sanitize_options_for_save(@cli.combined_instance_and_options_hosts) + options_storage = YAML::Store.new(SubcommandUtil::SUBCOMMAND_OPTIONS) + options_storage.transaction do + options_storage['HOSTS'] = hosts end end @@ -219,9 +215,7 @@ def destroy end state = YAML::Store.new(SubcommandUtil::SUBCOMMAND_STATE) - unless state.transaction { state['provisioned'] } - SubcommandUtil.error_with('Please provision an environment') - end + SubcommandUtil.error_with('Please provision an environment') unless state.transaction { state['provisioned'] } @cli.parse_options @cli.options[:provision] = false diff --git a/lib/beaker/tasks/quick_start.rb b/lib/beaker/tasks/quick_start.rb index f1ed44567..eb9dd3e4d 100644 --- a/lib/beaker/tasks/quick_start.rb +++ b/lib/beaker/tasks/quick_start.rb @@ -71,7 +71,7 @@ task :run_test, [:hypervisor] => ["beaker_quickstart:gen_hosts", 'beaker_quickstart:gen_pre_suite', 'beaker_quickstart:gen_smoke_test',] do |_t, args| hypervisor = args[:hypervisor] ||= 'vagrant' - system_args = Hash.new + system_args = {} system_args[:hosts] = "acceptance/config/default_#{hypervisor}_hosts.yaml" system_args[:pre_suite] = 'acceptance/setup/default_pre_suite.rb' system_args[:tests] = 'acceptance/tests/default_smoke_test.rb' diff --git a/lib/beaker/tasks/rake_task.rb b/lib/beaker/tasks/rake_task.rb index f10afd293..9859aebfd 100644 --- a/lib/beaker/tasks/rake_task.rb +++ b/lib/beaker/tasks/rake_task.rb @@ -33,9 +33,7 @@ def initialize(*args, &task_block) super @name = args.shift || 'beaker:test' - if args.empty? - args = [:hosts, :type] - end + args = [:hosts, :type] if args.empty? @acceptance_root = DEFAULT_ACCEPTANCE_ROOT @options_file = nil define(args, &task_block) @@ -53,10 +51,10 @@ def run_task(verbose) command = beaker_command puts command if verbose success = system(command) - if fail_mode == "fast" && !success - $stderr.puts "#{command} failed" - exit $?.exitstatus - end + return unless fail_mode == "fast" && !success + + $stderr.puts "#{command} failed" + exit $?.exitstatus end # @private @@ -77,19 +75,17 @@ def define(args, &task_block) # if no other options file is provided # def check_for_beaker_type_config - if !@options_file && File.exist?("#{@acceptance_root}/.beaker-#{@type}.cfg") - @options_file = File.join(@acceptance_root, ".beaker-#{@type}.cfg") - end + return unless !@options_file && File.exist?("#{@acceptance_root}/.beaker-#{@type}.cfg") + + @options_file = File.join(@acceptance_root, ".beaker-#{@type}.cfg") end # # Check for existence of ENV variables for test if !@tests is undef # def check_env_variables - if File.exist?(File.join(DEFAULT_ACCEPTANCE_ROOT, 'tests')) - @tests = File.join(DEFAULT_ACCEPTANCE_ROOT, 'tests') - end - @tests = ENV['TESTS'] || ENV['TEST'] if !@tests + @tests = File.join(DEFAULT_ACCEPTANCE_ROOT, 'tests') if File.exist?(File.join(DEFAULT_ACCEPTANCE_ROOT, 'tests')) + @tests = ENV['TESTS'] || ENV.fetch('TEST', nil) if !@tests end # diff --git a/lib/beaker/test_case.rb b/lib/beaker/test_case.rb index dc86107b2..27d9dde85 100644 --- a/lib/beaker/test_case.rb +++ b/lib/beaker/test_case.rb @@ -164,11 +164,11 @@ def log_and_fail_test(exception, status = :error) logger.error(line) end # If the status is already a test failure or error, don't overwrite with the teardown failure. - unless status == :teardown_error && (@test_status == :error || @test_status == :fail) - status = :error if status == :teardown_error - @test_status = status - @exception = exception - end + return if status == :teardown_error && (@test_status == :error || @test_status == :fail) + + status = :error if status == :teardown_error + @test_status = status + @exception = exception end end end diff --git a/lib/beaker/test_suite.rb b/lib/beaker/test_suite.rb index fdd22b2f1..e57d6e057 100644 --- a/lib/beaker/test_suite.rb +++ b/lib/beaker/test_suite.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - require 'fileutils' ['test_case', 'logger', 'test_suite_result'].each do |lib| require "beaker/#{lib}" diff --git a/lib/beaker/test_suite_result.rb b/lib/beaker/test_suite_result.rb index d255e0f1d..bb540e8f7 100644 --- a/lib/beaker/test_suite_result.rb +++ b/lib/beaker/test_suite_result.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- - require 'fileutils' ['test_case', 'logger', 'test_suite', 'logger_junit'].each do |lib| require "beaker/#{lib}" @@ -88,7 +86,7 @@ def summarize(summary_logger) average_test_time = elapsed_time / test_count - summary_logger.notify %[ + summary_logger.notify format(%[ - Test Case Summary for suite '#{@name}' - Total Suite Time: %.2f seconds @@ -102,7 +100,7 @@ def summarize(summary_logger) Total: #{@total_tests} - Specific Test Case Status - - ] % [elapsed_time, average_test_time] + ], elapsed_time, average_test_time) grouped_summary = @test_cases.group_by { |test_case| test_case.test_status } @@ -195,7 +193,7 @@ def write_junit_xml(xml_file, file_to_link = nil, time_sort = false) ['skipped', skipped_tests], ['pending', pending_tests], ['total', @total_tests], - ['time', "%f" % (stop_time - start_time)], + ['time', format("%f", (stop_time - start_time))], ], ) properties = suite.add_element(REXML::Element.new('properties')) diff --git a/spec/beaker/cli_spec.rb b/spec/beaker/cli_spec.rb index 638e22e55..53ee454b2 100644 --- a/spec/beaker/cli_spec.rb +++ b/spec/beaker/cli_spec.rb @@ -86,9 +86,7 @@ module Beaker it 'returns a list of options that were not presets' do attribution = cli.instance_variable_get(:@attribution) attribution.each do |attribute, setter| - if setter == 'preset' - expect(cli.configured_options[attribute]).to be_nil - end + expect(cli.configured_options[attribute]).to be_nil if setter == 'preset' end end end @@ -136,7 +134,7 @@ module Beaker context 'execute!' do before do - stub_const("Beaker::Logger", double().as_null_object) + stub_const("Beaker::Logger", double.as_null_object) File.open("sample.cfg", "w+") do |file| file.write("HOSTS:\n") file.write(" myhost:\n") diff --git a/spec/beaker/command_spec.rb b/spec/beaker/command_spec.rb index 53af6d38d..f7595ca2d 100644 --- a/spec/beaker/command_spec.rb +++ b/spec/beaker/command_spec.rb @@ -5,11 +5,11 @@ module Beaker subject(:cmd) { described_class.new(command, args, options) } let(:command) { @command || '/bin/ls' } - let(:args) { @args || Array.new } - let(:options) { @options || Hash.new } + let(:args) { @args || [] } + let(:options) { @options || {} } let(:host) do - h = Hash.new + h = {} allow(h).to receive(:environment_string).and_return('') h end @@ -94,10 +94,10 @@ module Beaker subject(:cmd) { described_class.new(command, args, options) } let(:command) { @command || '/bin/ls' } - let(:args) { @args || Array.new } - let(:options) { @options || Hash.new } + let(:args) { @args || [] } + let(:options) { @options || {} } - let(:host) { Hash.new } + let(:host) { {} } it 'returns a simple string passed in' do @command = "pants" @@ -119,7 +119,7 @@ module Beaker subject(:cmd) { described_class.new(platform, expression, filename, options) } let(:host) do - h = Hash.new + h = {} allow(h).to receive(:environment_string).and_return('') allow(h).to receive(:prepend_commands).and_return('') allow(h).to receive(:append_commands).and_return('') @@ -128,7 +128,7 @@ module Beaker let(:platform) { @platform || 'unix' } let(:expression) { @expression || 's/b/s/' } let(:filename) { @filename || '/fakefile' } - let(:options) { @options || Hash.new } + let(:options) { @options || {} } it 'forms a basic sed command correctly' do expect(cmd.cmd_line host).to be === "sed -i -e \"#{expression}\" #{filename}" diff --git a/spec/beaker/dsl/roles_spec.rb b/spec/beaker/dsl/roles_spec.rb index 4046fb41c..7578a265c 100644 --- a/spec/beaker/dsl/roles_spec.rb +++ b/spec/beaker/dsl/roles_spec.rb @@ -6,8 +6,8 @@ class ClassMixedWithDSLRoles end describe ClassMixedWithDSLRoles do - let(:hosts) { @hosts || Hash.new } - let(:options) { @options || Hash.new } + let(:hosts) { @hosts || {} } + let(:options) { @options || {} } let(:agent1) { make_host('agent1', { :roles => ['agent'] }) } let(:agent2) { make_host('agent2', { :roles => ['agent'] }) } let(:a_and_dash) { make_host('a_and_dash', { :roles => ['agent', 'dashboard'] }) } diff --git a/spec/beaker/dsl/structure_spec.rb b/spec/beaker/dsl/structure_spec.rb index b3465a2c8..14f03b7d8 100644 --- a/spec/beaker/dsl/structure_spec.rb +++ b/spec/beaker/dsl/structure_spec.rb @@ -189,7 +189,7 @@ class ClassMixedWithDSLStructure it 'append a block to the @teardown var' do teardown_array = double subject.instance_variable_set :@teardown_procs, teardown_array - block = lambda { 'blah' } + block = -> { 'blah' } expect(teardown_array).to receive(:<<).with(block) subject.teardown(&block) end @@ -201,24 +201,24 @@ class ClassMixedWithDSLStructure expect(logger).to receive(:notify) # We changed this lambda to use the simplest assert possible; using assert_equal # caused an error in minitest 5.9.0 trying to write to the file system. - block = lambda { assert(false, 'this assertion should be caught') } + block = -> { assert(false, 'this assertion should be caught') } expect { subject.expect_failure 'this is an expected failure', &block }.not_to raise_error end it 'passes when a Beaker assertion is raised' do expect(subject).to receive(:logger).and_return(logger) expect(logger).to receive(:notify) - block = lambda { refute_match('1', '1', '1 and 1 should not match') } + block = -> { refute_match('1', '1', '1 and 1 should not match') } expect { subject.expect_failure 'this is an expected failure', &block }.not_to raise_error end it 'fails when a non-Beaker, non-MiniTest assertion is raised' do - block = lambda { raise 'not a Beaker or MiniTest error' } + block = -> { raise 'not a Beaker or MiniTest error' } expect { subject.expect_failure 'this has a non-Beaker, non-MiniTest exception', &block }.to raise_error(RuntimeError, /not a Beaker or MiniTest error/) end it 'fails when no assertion is raised' do - block = lambda { expect('1').to(eq('1'), '1 should equal 1') } + block = -> { expect('1').to(eq('1'), '1 should equal 1') } expect { subject.expect_failure 'this has no failure', &block }.to raise_error(RuntimeError, /An assertion was expected to fail, but passed/) end end diff --git a/spec/beaker/dsl/test_tagging_spec.rb b/spec/beaker/dsl/test_tagging_spec.rb index 0c7d87c0d..195d4d012 100644 --- a/spec/beaker/dsl/test_tagging_spec.rb +++ b/spec/beaker/dsl/test_tagging_spec.rb @@ -149,13 +149,13 @@ class ClassMixedWithDSLStructure }] internal_hash = confiner.instance_variable_get(:@tag_confine_details_hash) - expect(internal_hash.keys()).to include('tag1') - expect(internal_hash.keys()).to include('tag2') - expect(internal_hash.keys().length()).to be === 2 + expect(internal_hash.keys).to include('tag1') + expect(internal_hash.keys).to include('tag2') + expect(internal_hash.keys.length).to be === 2 tag_reason_hash.each do |tag, reason| tag_array = internal_hash[tag] - expect(tag_array.length()).to be === 1 + expect(tag_array.length).to be === 1 tag_hash = tag_array[0] expect(tag_hash[:platform_regex]).to eql(platform_regex) expect(tag_hash[:log_message]).to match(/#{reason}/) @@ -181,13 +181,13 @@ class ClassMixedWithDSLStructure ] internal_hash = confiner.instance_variable_get(:@tag_confine_details_hash) - expect(internal_hash.keys()).to include('tag1') - expect(internal_hash.keys()).to include('tag2') - expect(internal_hash.keys()).to include('tag3') - expect(internal_hash.keys().length()).to be === 3 + expect(internal_hash.keys).to include('tag1') + expect(internal_hash.keys).to include('tag2') + expect(internal_hash.keys).to include('tag3') + expect(internal_hash.keys.length).to be === 3 shared_tag_array = internal_hash['tag1'] - expect(shared_tag_array.length()).to be === 2 + expect(shared_tag_array.length).to be === 2 platform_el_found = false platform_cisco_found = false @@ -237,7 +237,7 @@ class ClassMixedWithDSLStructure key_combos_to_test << ['tag0', 'tag2'] key_combos_to_test << ['tag1', 'tag4'] key_combos_to_test << ['tag2', 'tag3', 'tag4'] - key_combos_to_test << fake_confine_details_hash.keys() + key_combos_to_test << fake_confine_details_hash.keys before do confiner.instance_variable_set( diff --git a/spec/beaker/host_prebuilt_steps_spec.rb b/spec/beaker/host_prebuilt_steps_spec.rb index 491f8fcb2..ae52b12e1 100644 --- a/spec/beaker/host_prebuilt_steps_spec.rb +++ b/spec/beaker/host_prebuilt_steps_spec.rb @@ -1,7 +1,7 @@ require 'spec_helper' describe Beaker do - let(:options) { make_opts.merge({ 'logger' => double().as_null_object }) } + let(:options) { make_opts.merge({ 'logger' => double.as_null_object }) } let(:ntpserver_set) { "ntp_server_set" } let(:options_ntp) { make_opts.merge({ 'ntp_server' => ntpserver_set }) } let(:ntpserver) { Beaker::HostPrebuiltSteps::NTPSERVER } @@ -29,9 +29,7 @@ it "can enable root login on #{platform}" do hosts = make_hosts({ :platform => platform, :is_cygwin => non_cygwin }) - if commands.empty? - expect(Beaker::Command).to receive(:new).exactly(0).times - end + expect(Beaker::Command).to receive(:new).exactly(0).times if commands.empty? commands.each do |command| expect(Beaker::Command).to receive(:new).with(command).exactly(3).times diff --git a/spec/beaker/hypervisor/hypervisor_spec.rb b/spec/beaker/hypervisor/hypervisor_spec.rb index 027c21eba..3b9a09498 100644 --- a/spec/beaker/hypervisor/hypervisor_spec.rb +++ b/spec/beaker/hypervisor/hypervisor_spec.rb @@ -9,11 +9,11 @@ module Beaker it "includes custom hypervisor and call set_ssh_connection_preference" do allow(hypervisor).to receive(:set_ssh_connection_preference).with([], hypervisor) - expect { hypervisor.create('custom_hypervisor', [], make_opts()) }.to raise_error(LoadError, "cannot load such file -- beaker/hypervisor/custom_hypervisor") + expect { hypervisor.create('custom_hypervisor', [], make_opts) }.to raise_error(LoadError, "cannot load such file -- beaker/hypervisor/custom_hypervisor") end it "sets ssh connection preference if connection_preference method is not overwritten" do - hypervisor.create('none', hosts, make_opts()) + hypervisor.create('none', hosts, make_opts) expect(hosts[0][:ssh_connection_preference]).to eq([:ip, :vmhostname, :hostname]) end @@ -31,7 +31,7 @@ module Beaker it "gives highest precedence to preference specified in host file followed by hypervisor" do hosts[0].options[:ssh_preference] = [:set, :in, :hostfile] - hypervisor.create('none', hosts, make_opts()) + hypervisor.create('none', hosts, make_opts) allow(hypervisor).to receive(:connection_preference).and_return([:hypervisor, :pref]) hypervisor.set_ssh_connection_preference(hosts, hypervisor) expect(hosts[0][:ssh_connection_preference]).to eq([:set, :in, :hostfile, :hypervisor, :pref, :ip, :vmhostname, :hostname]) @@ -39,7 +39,7 @@ module Beaker end describe "#configure" do - let(:options) { make_opts.merge({ 'logger' => double().as_null_object }) } + let(:options) { make_opts.merge({ 'logger' => double.as_null_object }) } let(:hypervisor) { described_class.new(hosts, options) } context 'if :timesync option set true on host' do @@ -51,7 +51,7 @@ module Beaker end it 'catches signal exceptions and returns stack trace' do - logger = double() + logger = double hosts[0].options[:timesync] = true allow(logger).to receive(:error) allow(logger).to receive(:pretty_backtrace).and_return("multiline\nstring") @@ -131,8 +131,8 @@ module Beaker it "generates hostname with prefix" do prefix = "testing-prefix-to-test-" options[:host_name_prefix] = prefix - expect(hypervisor.generate_host_name().start_with?(prefix)).to be true - expect(hypervisor.generate_host_name().length - prefix.length >= 15).to be true + expect(hypervisor.generate_host_name.start_with?(prefix)).to be true + expect(hypervisor.generate_host_name.length - prefix.length >= 15).to be true end end end diff --git a/spec/beaker/logger_junit_spec.rb b/spec/beaker/logger_junit_spec.rb index fd32a430f..a4822de27 100644 --- a/spec/beaker/logger_junit_spec.rb +++ b/spec/beaker/logger_junit_spec.rb @@ -1,5 +1,3 @@ -# encoding: UTF-8 - require 'spec_helper' module Beaker diff --git a/spec/beaker/logger_spec.rb b/spec/beaker/logger_spec.rb index 068d310eb..7d9fe3440 100644 --- a/spec/beaker/logger_spec.rb +++ b/spec/beaker/logger_spec.rb @@ -1,5 +1,3 @@ -# encoding: UTF-8 - require 'spec_helper' module Beaker @@ -173,7 +171,7 @@ def prefix_log_line_test_compare_helper(in_test, out_answer) end context 'log_colors' do - original_build_number = ENV['BUILD_NUMBER'] + original_build_number = ENV.fetch('BUILD_NUMBER', nil) before do ENV['BUILD_NUMBER'] = nil diff --git a/spec/beaker/network_manager_spec.rb b/spec/beaker/network_manager_spec.rb index d4efcb872..3ff3a6262 100644 --- a/spec/beaker/network_manager_spec.rb +++ b/spec/beaker/network_manager_spec.rb @@ -1,5 +1,3 @@ -# encoding: UTF-8 - require 'spec_helper' module Beaker @@ -11,7 +9,7 @@ module Beaker end let(:options) do make_opts.merge({ - 'logger' => double().as_null_object, + 'logger' => double.as_null_object, :logger_sut => mock_provisioning_logger, :log_prefix => @log_prefix, :hosts_file => @hosts_file, diff --git a/spec/beaker/options/hosts_file_parser_spec.rb b/spec/beaker/options/hosts_file_parser_spec.rb index 31feb6ba6..9de06af89 100644 --- a/spec/beaker/options/hosts_file_parser_spec.rb +++ b/spec/beaker/options/hosts_file_parser_spec.rb @@ -23,7 +23,7 @@ module Options it "returns empty configuration when no file provided" do FakeFS.deactivate! - expect(parser.parse_hosts_file()).to be === { :HOSTS => {} } + expect(parser.parse_hosts_file).to be === { :HOSTS => {} } end it "raises an error on no file found" do diff --git a/spec/beaker/options/parser_spec.rb b/spec/beaker/options/parser_spec.rb index bce5826ee..46c7134a5 100644 --- a/spec/beaker/options/parser_spec.rb +++ b/spec/beaker/options/parser_spec.rb @@ -182,12 +182,12 @@ module Options end def mock_out_parsing - presets_obj = double() + presets_obj = double allow(presets_obj).to receive(:presets).and_return(presets) allow(presets_obj).to receive(:env_vars).and_return(env) parser.instance_variable_set(:@presets, presets_obj) - command_line_parser_obj = double() + command_line_parser_obj = double allow(command_line_parser_obj).to receive(:parse).and_return(argv) parser.instance_variable_set(:@command_line_parser, command_line_parser_obj) @@ -195,7 +195,7 @@ def mock_out_parsing allow(parser).to receive(:parse_hosts_options).and_return(host_file) allow(SubcommandOptionsParser).to receive(:parse_options_file).with(".beaker.yml").and_return(project_file) - allow(SubcommandOptionsParser).to receive(:parse_subcommand_options).with(anything, "#{ENV['HOME']}/.beaker/subcommand_options.yaml").and_return(homedir_file) + allow(SubcommandOptionsParser).to receive(:parse_subcommand_options).with(anything, "#{ENV.fetch('HOME', nil)}/.beaker/subcommand_options.yaml").and_return(homedir_file) allow(SubcommandOptionsParser).to receive(:parse_subcommand_options).with(anything, Pathname(".beaker/subcommand_options.yaml")).and_return(subcommand_file) end @@ -319,7 +319,7 @@ def mock_out_parsing type = 'git' log_level = 'debug' - old_build_url = ENV["BUILD_URL"] + old_build_url = ENV.fetch("BUILD_URL", nil) ENV["BUILD_URL"] = build_url args = ["-h", hosts_path, "--log-level", log_level, "--type", type, "--install", "PUPPET/1.0,HIERA/hello"] @@ -393,7 +393,7 @@ def mock_out_parsing end it 'calls beaker-hostgenerator to get hosts information with a default hypervisor' do - old_beaker_hypervisor = ENV['BEAKER_HYPERVISOR'] + old_beaker_hypervisor = ENV.fetch('BEAKER_HYPERVISOR', nil) begin ENV['BEAKER_HYPERVISOR'] = 'docker' diff --git a/spec/beaker/options/subcommand_options_parser_spec.rb b/spec/beaker/options/subcommand_options_parser_spec.rb index 2219e30bc..403f828c6 100644 --- a/spec/beaker/options/subcommand_options_parser_spec.rb +++ b/spec/beaker/options/subcommand_options_parser_spec.rb @@ -3,7 +3,7 @@ module Beaker module Options describe '#parse_subcommand_options' do - let(:home_options_file_path) { ENV['HOME'] + '/.beaker/subcommand_options.yaml' } + let(:home_options_file_path) { ENV.fetch('HOME', nil) + '/.beaker/subcommand_options.yaml' } let(:parser_mod) { Beaker::Options::SubcommandOptionsParser } let(:parser) { parser_mod.parse_subcommand_options(argv, options_file) } let(:file_parser) { parser_mod.parse_options_file({}) } diff --git a/spec/beaker/perf_spec.rb b/spec/beaker/perf_spec.rb index a3ad86124..3c6173ddd 100644 --- a/spec/beaker/perf_spec.rb +++ b/spec/beaker/perf_spec.rb @@ -15,8 +15,8 @@ module Beaker end it 'creates a new Perf object' do - hosts = Array.new - options = Hash.new + hosts = [] + options = {} options[:log_level] = :debug my_logger = Beaker::Logger.new(options) options[:logger] = my_logger diff --git a/spec/beaker/shared/host_manager_spec.rb b/spec/beaker/shared/host_manager_spec.rb index 5a4556756..177c36415 100644 --- a/spec/beaker/shared/host_manager_spec.rb +++ b/spec/beaker/shared/host_manager_spec.rb @@ -129,7 +129,7 @@ module Shared myhosts = host_handler.run_block_on(hosts, nil, { :run_in_parallel => true }) do |host| # kind of hacky workaround to remove logger which contains a singleton method injected by rspec - host.instance_eval("remove_instance_variable(:@logger)") + host.instance_eval("remove_instance_variable(:@logger)", __FILE__, __LINE__) host end diff --git a/spec/beaker/subcommand_spec.rb b/spec/beaker/subcommand_spec.rb index 2048812af..b7c7b92e1 100644 --- a/spec/beaker/subcommand_spec.rb +++ b/spec/beaker/subcommand_spec.rb @@ -134,9 +134,9 @@ module Beaker let(:cli) { subcommand.cli } let(:yaml_store_mock) { double('yaml_store_mock') } let(:host_hash) { { 'mynode.net' => { :name => 'mynode', :platform => Beaker::Platform.new('centos-6-x86_64') } } } - let(:cleaned_hosts) { double() } - let(:yielded_host_hash) { double() } - let(:yielded_host_name) { double() } + let(:cleaned_hosts) { double } + let(:yielded_host_hash) { double } + let(:yielded_host_name) { double } let(:network_manager) { double('network_manager') } let(:hosts) { double('hosts') } let(:hypervisors) { double('hypervisors') } @@ -169,7 +169,7 @@ module Beaker it 'does not allow hosts to be passed' do subcommand.options = { :hosts => "myhost" } - expect { subcommand.provision() }.to raise_error(NotImplementedError) + expect { subcommand.provision }.to raise_error(NotImplementedError) end end @@ -180,7 +180,7 @@ module Beaker allow(subcommand.cli).to receive(:execute!) end - let(:cleaned_hosts) { double() } + let(:cleaned_hosts) { double } let(:host_hash) { { 'mynode.net' => { :name => 'mynode', :platform => Beaker::Platform.new('centos-6-x86_64') } } } let(:yaml_store_mock) { double('yaml_store_mock') } diff --git a/spec/beaker/test_case_spec.rb b/spec/beaker/test_case_spec.rb index 5a7993802..cd05dcaab 100644 --- a/spec/beaker/test_case_spec.rb +++ b/spec/beaker/test_case_spec.rb @@ -9,9 +9,7 @@ module Beaker context 'run_test' do it 'defaults to test_status :pass on success' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "" - end + File.write(path, "") @path = path expect(testcase).not_to receive(:log_and_fail_test) testcase.run_test @@ -20,9 +18,7 @@ module Beaker it 'updates test_status to :skip on SkipTest' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise SkipTest" - end + File.write(path, "raise SkipTest") @path = path expect(testcase).not_to receive(:log_and_fail_test) testcase.run_test @@ -31,9 +27,7 @@ module Beaker it 'updates test_status to :pending on PendingTest' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise PendingTest" - end + File.write(path, "raise PendingTest") @path = path expect(testcase).not_to receive(:log_and_fail_test) testcase.run_test @@ -42,9 +36,7 @@ module Beaker it 'updates test_status to :fail on FailTest' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise FailTest" - end + File.write(path, "raise FailTest") @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Beaker::DSL::FailTest), :fail).and_call_original testcase.run_test @@ -53,9 +45,7 @@ module Beaker it 'correctly handles RuntimeError' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise RuntimeError" - end + File.write(path, "raise RuntimeError") @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(RuntimeError)) testcase.run_test @@ -63,9 +53,7 @@ module Beaker it 'correctly handles ScriptError' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise ScriptError" - end + File.write(path, "raise ScriptError") @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(ScriptError)) testcase.run_test @@ -73,9 +61,7 @@ module Beaker it 'correctly handles Timeout::Error' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise Timeout::Error" - end + File.write(path, "raise Timeout::Error") @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Timeout::Error)) testcase.run_test @@ -83,9 +69,7 @@ module Beaker it 'correctly handles CommandFailure' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "raise Host::CommandFailure" - end + File.write(path, "raise Host::CommandFailure") @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Host::CommandFailure)) testcase.run_test @@ -93,13 +77,11 @@ module Beaker it 'records a test failure if an assertion fails in a teardown block' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write <<-EOF + File.write(path, <<-EOF) teardown do assert_equal(1, 2, 'Oh noes!') end - EOF - end + EOF @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Minitest::Assertion), :teardown_error).and_call_original testcase.run_test @@ -108,14 +90,12 @@ module Beaker it 'does not overwrite a test failure if an assertion also happens in a teardown block' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write <<-EOF + File.write(path, <<-EOF) teardown do assert_equal(1, 2, 'Oh noes!') end assert_equal(true, false, 'failed test') - EOF - end + EOF @path = path expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Minitest::Assertion), :fail).and_call_original expect(testcase).to receive(:log_and_fail_test).once.with(kind_of(Minitest::Assertion), :teardown_error).and_call_original @@ -128,9 +108,7 @@ module Beaker it 'sets the filename correctly from the path' do answer = 'jacket' path = "#{answer}.rb" - File.open(path, 'w') do |f| - f.write "" - end + File.write(path, "") @path = path testcase.run_test metadata = testcase.instance_variable_get(:@metadata) @@ -139,9 +117,7 @@ module Beaker it 'resets the step name' do path = 'test.rb' - File.open(path, 'w') do |f| - f.write "" - end + File.write(path, "") @path = path # we have to create a TestCase by hand, so that we can set old tc = described_class.new({}, logger, {}, path) diff --git a/spec/beaker/test_suite_spec.rb b/spec/beaker/test_suite_spec.rb index c35e98672..4ab5651dc 100644 --- a/spec/beaker/test_suite_spec.rb +++ b/spec/beaker/test_suite_spec.rb @@ -12,7 +12,7 @@ module Beaker let(:sh_test) { File.expand_path(test_dir + '/my_shell_file.sh') } it 'fails without test files' do - expect { described_class.new('name', 'hosts', Hash.new, Time.now, :stop_on_error) }.to raise_error + expect { described_class.new('name', 'hosts', {}, Time.now, :stop_on_error) }.to raise_error end it 'includes specific files as test file when explicitly passed' do @@ -47,21 +47,21 @@ module Beaker end context 'run' do - let(:options) { make_opts.merge({ :logger => double().as_null_object, 'name' => create_files(@files), :log_dated_dir => '.', :xml_dated_dir => '.' }) } + let(:options) { make_opts.merge({ :logger => double.as_null_object, 'name' => create_files(@files), :log_dated_dir => '.', :xml_dated_dir => '.' }) } let(:broken_script) { "raise RuntimeError" } let(:fail_script) { "raise Beaker::DSL::Outcomes::FailTest" } let(:okay_script) { "true" } let(:rb_test) { 'my_ruby_file.rb' } let(:pl_test) { '/my_perl_file.pl' } let(:sh_test) { '/my_shell_file.sh' } - let(:hosts) { make_hosts() } + let(:hosts) { make_hosts } it 'fails fast if fail_mode != :slow and runtime error is raised' do allow(Logger).to receive('new') @files = [rb_test, pl_test, sh_test] - File.open(rb_test, 'w') { |file| file.write(broken_script) } - File.open(pl_test, 'w') { |file| file.write(okay_script) } - File.open(sh_test, 'w') { |file| file.write(okay_script) } + File.write(rb_test, broken_script) + File.write(pl_test, okay_script) + File.write(sh_test, okay_script) ts = described_class.new('name', hosts, options, Time.now, :stop) tsr = ts.instance_variable_get(:@test_suite_results) @@ -78,9 +78,9 @@ module Beaker it 'fails fast if fail_mode != :slow and fail test is raised' do allow(Logger).to receive('new') @files = [rb_test, pl_test, sh_test] - File.open(rb_test, 'w') { |file| file.write(fail_script) } - File.open(pl_test, 'w') { |file| file.write(okay_script) } - File.open(sh_test, 'w') { |file| file.write(okay_script) } + File.write(rb_test, fail_script) + File.write(pl_test, okay_script) + File.write(sh_test, okay_script) ts = described_class.new('name', hosts, options, Time.now, :stop) tsr = ts.instance_variable_get(:@test_suite_results) @@ -97,9 +97,9 @@ module Beaker it 'fails slow if fail_mode = :slow, even if a test fails and there is a runtime error' do allow(Logger).to receive('new') @files = [rb_test, pl_test, sh_test] - File.open(rb_test, 'w') { |file| file.write(broken_script) } - File.open(pl_test, 'w') { |file| file.write(fail_script) } - File.open(sh_test, 'w') { |file| file.write(okay_script) } + File.write(rb_test, broken_script) + File.write(pl_test, fail_script) + File.write(sh_test, okay_script) ts = described_class.new('name', hosts, options, Time.now, :slow) tsr = ts.instance_variable_get(:@test_suite_results) @@ -115,8 +115,8 @@ module Beaker end describe TestSuiteResult do - let(:options) { make_opts.merge({ :logger => double().as_null_object }) } - let(:hosts) { make_hosts() } + let(:options) { make_opts.merge({ :logger => double.as_null_object }) } + let(:hosts) { make_hosts } let(:testcase1) { Beaker::TestCase.new(hosts, options[:logger], options) } let(:testcase2) { Beaker::TestCase.new(hosts, options[:logger], options) } let(:testcase3) { Beaker::TestCase.new(hosts, options[:logger], options) } @@ -242,7 +242,7 @@ module Beaker describe '#write_junit_xml' do let(:options) do - make_opts.merge({ :logger => double().as_null_object, + make_opts.merge({ :logger => double.as_null_object, 'name' => create_files(@files), :log_dated_dir => '.', :xml_dated_dir => '.', }) @@ -330,8 +330,8 @@ module Beaker describe '#log_path' do let(:sh_test) { '/my_shell_file.sh' } let(:files) { @files ? @files : [sh_test] } - let(:options) { make_opts.merge({ :logger => double().as_null_object, 'name' => create_files(files) }) } - let(:hosts) { make_hosts() } + let(:options) { make_opts.merge({ :logger => double.as_null_object, 'name' => create_files(files) }) } + let(:hosts) { make_hosts } let(:testsuite) { described_class.new('name', hosts, options, Time.now, :stop) } it 'returns the simple joining of the log dir & file as required' do