From 8a6cfb507f9fdc88897a4b2ba31a1f0092fe3957 Mon Sep 17 00:00:00 2001 From: Luke Hill Date: Mon, 15 Jul 2019 11:53:59 +0200 Subject: [PATCH] Fix up a few of the LineLength cops --- .rubocop.yml | 10 ++++++ .rubocop_todo.yml | 7 ---- aruba.gemspec | 3 +- lib/aruba/api/core.rb | 8 +++-- lib/aruba/api/environment.rb | 12 ++++--- lib/aruba/api/filesystem.rb | 25 +++++++++---- lib/aruba/config_wrapper.rb | 4 ++- lib/aruba/configuration.rb | 36 ++++++++++++------- .../matchers/command/have_exit_status.rb | 14 ++++++-- .../windows_environment_variables.rb | 8 ++++- lib/aruba/processes/spawn_process.rb | 10 ++++-- spec/aruba/api/filesystem_spec.rb | 6 ++-- 12 files changed, 102 insertions(+), 41 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 2e45be51e..df84c8c36 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -18,4 +18,14 @@ Style/PercentLiteralDelimiters: Style/FrozenStringLiteralComment: Enabled: false +# TODO: This is placed in here so the auto-gen-config doesn't destroy these. +# Most of these need to be manually tackled, the LineLength value currently is good enough to stop most of +# these leaking through into the codebase +# Offense count: 375+ +# Cop supports --auto-correct. +# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. +# URISchemes: http, https +Metrics/LineLength: + Max: 150 + inherit_from: .rubocop_todo.yml diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 5589ffc02..ea696be87 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -256,10 +256,3 @@ Style/StderrPuts: # SupportedStyles: percent, brackets Style/SymbolArray: EnforcedStyle: brackets - -# Offense count: 411 -# Cop supports --auto-correct. -# Configuration parameters: AutoCorrect, AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns. -# URISchemes: http, https -Metrics/LineLength: - Max: 174 diff --git a/aruba.gemspec b/aruba.gemspec index 4250e20dd..8facf8123 100644 --- a/aruba.gemspec +++ b/aruba.gemspec @@ -6,7 +6,8 @@ Gem::Specification.new do |spec| spec.name = 'aruba' spec.version = Aruba::VERSION spec.author = 'Aslak Hellesøy, Matt Wynne and other Aruba Contributors' - spec.description = 'Extension for popular TDD and BDD frameworks like "Cucumber", "RSpec" and "Minitest" to make testing commandline applications meaningful, easy and fun.' + spec.description = 'Extension for popular TDD and BDD frameworks like "Cucumber", "RSpec" and "Minitest", +to make testing commandline applications meaningful, easy and fun.' spec.summary = "aruba-#{spec.version}" spec.license = 'MIT' spec.email = 'cukes@googlegroups.com' diff --git a/lib/aruba/api/core.rb b/lib/aruba/api/core.rb index 6bc1f73c2..39273ea7b 100644 --- a/lib/aruba/api/core.rb +++ b/lib/aruba/api/core.rb @@ -58,7 +58,9 @@ def in_current_directory(&block) def cd(dir, &block) if block_given? begin - fail ArgumentError, "#{expand_path(dir)} is not a directory or does not exist." unless Aruba.platform.directory? expand_path(dir) + unless Aruba.platform.directory? expand_path(dir) + fail ArgumentError, "#{expand_path(dir)} is not a directory or does not exist." + end old_directory = expand_path('.') aruba.current_directory << dir @@ -83,7 +85,9 @@ def cd(dir, &block) return result end - fail ArgumentError, "#{expand_path(dir)} is not a directory or does not exist." unless Aruba.platform.directory? expand_path(dir) + unless Aruba.platform.directory? expand_path(dir) + fail ArgumentError, "#{expand_path(dir)} is not a directory or does not exist." + end old_directory = expand_path('.') aruba.current_directory << dir diff --git a/lib/aruba/api/environment.rb b/lib/aruba/api/environment.rb index 1cff1cf43..2db0e6a2b 100644 --- a/lib/aruba/api/environment.rb +++ b/lib/aruba/api/environment.rb @@ -23,7 +23,8 @@ def set_environment_variable(name, value) aruba.environment[name] = value new_environment = aruba.environment.to_h - aruba.event_bus.notify Events::AddedEnvironmentVariable.new(old: old_environment, new: new_environment, changed: { name: name, value: value }) + environment_change = { old: old_environment, new: new_environment, changed: { name: name, value: value } } + aruba.event_bus.notify Events::AddedEnvironmentVariable.new(environment_change) self end @@ -45,7 +46,8 @@ def append_environment_variable(name, value) aruba.environment.append name, value new_environment = aruba.environment.to_h - aruba.event_bus.notify Events::ChangedEnvironmentVariable.new(old: old_environment, new: new_environment, changed: { name: name, value: value }) + environment_change = { old: old_environment, new: new_environment, changed: { name: name, value: value } } + aruba.event_bus.notify Events::ChangedEnvironmentVariable.new(environment_change) self end @@ -67,7 +69,8 @@ def prepend_environment_variable(name, value) aruba.environment.prepend name, value new_environment = aruba.environment.to_h - aruba.event_bus.notify Events::ChangedEnvironmentVariable.new(old: old_environment, new: new_environment, changed: { name: name, value: value }) + environment_change = { old: old_environment, new: new_environment, changed: { name: name, value: value } } + aruba.event_bus.notify Events::ChangedEnvironmentVariable.new(environment_change) self end @@ -85,7 +88,8 @@ def delete_environment_variable(name) aruba.environment.delete name new_environment = aruba.environment.to_h - aruba.event_bus.notify Events::DeletedEnvironmentVariable.new(old: old_environment, new: new_environment, changed: { name: name, value: '' }) + environment_change = { old: old_environment, new: new_environment, changed: { name: name, value: '' } } + aruba.event_bus.notify Events::ChangedEnvironmentVariable.new(environment_change) self end diff --git a/lib/aruba/api/filesystem.rb b/lib/aruba/api/filesystem.rb index 9c61ce497..b25e55307 100644 --- a/lib/aruba/api/filesystem.rb +++ b/lib/aruba/api/filesystem.rb @@ -104,7 +104,9 @@ def directory(path) # The content of directory def list(name) fail ArgumentError, %(Path "#{name}" does not exist.) unless exist? name - fail ArgumentError, %(Only directories are supported. Path "#{name}" is not a directory.) unless directory? name + unless directory? name + fail ArgumentError, %(Only directories are supported. Path "#{name}" is not a directory.) + end existing_files = Dir.glob(expand_path(File.join(name, '**', '*'))) current_working_directory = ArubaPath.new(expand_path('.')) @@ -181,8 +183,13 @@ def copy(*args) raise ArgumentError, %(The following source "#{s}" does not exist.) unless exist? s end - raise ArgumentError, "Using a fixture as destination (#{destination}) is not supported" if destination.start_with? aruba.config.fixtures_path_prefix - raise ArgumentError, 'Multiples sources can only be copied to a directory' if source.count > 1 && exist?(destination) && !directory?(destination) + if destination.start_with? aruba.config.fixtures_path_prefix + raise ArgumentError, "Using a fixture as destination (#{destination}) is not supported" + end + + if source.count > 1 && exist?(destination) && !directory?(destination) + raise ArgumentError, 'Multiples sources can only be copied to a directory' + end source_paths = source.map { |f| expand_path(f) } destination_path = expand_path(destination) @@ -219,16 +226,22 @@ def move(*args) source = args source.each do |s| - raise ArgumentError, "Using a fixture as source (#{source}) is not supported" if s.start_with? aruba.config.fixtures_path_prefix + if s.start_with? aruba.config.fixtures_path_prefix + raise ArgumentError, "Using a fixture as source (#{source}) is not supported" + end end - raise ArgumentError, "Using a fixture as destination (#{destination}) is not supported" if destination.start_with? aruba.config.fixtures_path_prefix + if destination.start_with? aruba.config.fixtures_path_prefix + raise ArgumentError, "Using a fixture as destination (#{destination}) is not supported" + end source.each do |s| raise ArgumentError, %(The following source "#{s}" does not exist.) unless exist? s end - raise ArgumentError, 'Multiple sources can only be copied to a directory' if source.count > 1 && exist?(destination) && !directory?(destination) + if source.count > 1 && exist?(destination) && !directory?(destination) + raise ArgumentError, 'Multiple sources can only be copied to a directory' + end source_paths = source.map { |f| expand_path(f) } destination_path = expand_path(destination) diff --git a/lib/aruba/config_wrapper.rb b/lib/aruba/config_wrapper.rb index ccaed4cc6..1e09f2c58 100644 --- a/lib/aruba/config_wrapper.rb +++ b/lib/aruba/config_wrapper.rb @@ -30,7 +30,9 @@ def initialize(config, event_bus) # If one method ends with "=", e.g. ":option1=", then notify the event # queue, that the user changes the value of "option1" def method_missing(name, *args, &block) - event_bus.notify Events::ChangedConfiguration.new(changed: { name: name.to_s.gsub(/=$/, ''), value: args.first }) if name.to_s.end_with? '=' + if name.to_s.end_with? '=' + event_bus.notify Events::ChangedConfiguration.new(changed: { name: name.to_s.gsub(/=$/, ''), value: args.first }) + end config.send(name, *args, &block) end diff --git a/lib/aruba/configuration.rb b/lib/aruba/configuration.rb index ce3953fda..609554d6d 100644 --- a/lib/aruba/configuration.rb +++ b/lib/aruba/configuration.rb @@ -19,7 +19,9 @@ module Aruba class Configuration < BasicConfiguration option_reader :root_directory, contract: { None => String }, default: Dir.getwd - option_accessor :working_directory, contract: { Aruba::Contracts::RelativePath => Aruba::Contracts::RelativePath }, default: 'tmp/aruba' + option_accessor :working_directory, + contract: { Aruba::Contracts::RelativePath => Aruba::Contracts::RelativePath }, + default: 'tmp/aruba' option_reader :fixtures_path_prefix, contract: { None => String }, default: '%' @@ -27,26 +29,36 @@ class Configuration < BasicConfiguration option_accessor :stop_signal, contract: { Maybe[String] => Maybe[String] }, default: nil option_accessor :io_wait_timeout, contract: { Num => Num }, default: 0.1 option_accessor :startup_wait_time, contract: { Num => Num }, default: 0 - option_accessor :fixtures_directories, contract: { Array => ArrayOf[String] }, default: %w(features/fixtures spec/fixtures test/fixtures fixtures) + option_accessor :fixtures_directories, + contract: { Array => ArrayOf[String] }, + default: %w(features/fixtures spec/fixtures test/fixtures fixtures) + option_accessor :command_runtime_environment, contract: { Hash => Hash }, default: {} - # rubocop:disable Metrics/LineLength - option_accessor(:command_search_paths, contract: { ArrayOf[String] => ArrayOf[String] }) { |config| [File.join(config.root_directory.value, 'bin'), File.join(config.root_directory.value, 'exe')] } - # rubocop:enable Metrics/LineLength + option_accessor :command_search_paths, + contract: { ArrayOf[String] => ArrayOf[String] } do |config| + [File.join(config.root_directory.value, 'bin'), File.join(config.root_directory.value, 'exe')] + end option_accessor :remove_ansi_escape_sequences, contract: { Bool => Bool }, default: true - # rubocop:disable Metrics/LineLength - option_accessor :command_launcher, contract: { Aruba::Contracts::Enum[:in_process, :spawn, :debug] => Aruba::Contracts::Enum[:in_process, :spawn, :debug] }, default: :spawn + option_accessor :command_launcher, + contract: { Aruba::Contracts::Enum[:in_process, :spawn, :debug] => Aruba::Contracts::Enum[:in_process, :spawn, :debug] }, + default: :spawn option_accessor :main_class, contract: { Class => Maybe[Class] }, default: nil - option_accessor :home_directory, contract: { Or[Aruba::Contracts::AbsolutePath, Aruba::Contracts::RelativePath] => Or[Aruba::Contracts::AbsolutePath, Aruba::Contracts::RelativePath] } do |config| - File.join(config.root_directory.value, config.working_directory.value) - end + option_accessor :home_directory, + contract: { Or[Aruba::Contracts::AbsolutePath, Aruba::Contracts::RelativePath] => Or[Aruba::Contracts::AbsolutePath, Aruba::Contracts::RelativePath] } do |config| + File.join(config.root_directory.value, config.working_directory.value) + end - option_accessor :log_level, contract: { Aruba::Contracts::Enum[:fatal, :warn, :debug, :info, :error, :unknown, :silent] => Aruba::Contracts::Enum[:fatal, :warn, :debug, :info, :error, :unknown, :silent] }, default: :info + option_accessor :log_level, + contract: { Aruba::Contracts::Enum[:fatal, :warn, :debug, :info, :error, :unknown, :silent] => Aruba::Contracts::Enum[:fatal, :warn, :debug, :info, :error, :unknown, :silent] }, + default: :info # TODO: deprecate this value and replace with "filesystem allocation unit" # equal to 4096 by default. "filesystem allocation unit" would represent # the actual MINIMUM space taken in bytes by a 1-byte file - option_accessor :physical_block_size, contract: { Aruba::Contracts::IsPowerOfTwo => Aruba::Contracts::IsPowerOfTwo }, default: 512 + option_accessor :physical_block_size, + contract: { Aruba::Contracts::IsPowerOfTwo => Aruba::Contracts::IsPowerOfTwo }, + default: 512 option_accessor :console_history_file, contract: { String => String }, default: '~/.aruba_history' option_accessor :activate_announcer_on_command_failure, contract: { ArrayOf[Symbol] => ArrayOf[Symbol] }, default: [] diff --git a/lib/aruba/matchers/command/have_exit_status.rb b/lib/aruba/matchers/command/have_exit_status.rb index 8ed613855..4b667fe8d 100644 --- a/lib/aruba/matchers/command/have_exit_status.rb +++ b/lib/aruba/matchers/command/have_exit_status.rb @@ -29,10 +29,20 @@ end failure_message do |actual| - format(%(expected that command "%s" has exit status of "%s", but has "%s".), @old_actual.commandline, expected.to_s, actual.to_s) + format( + %(expected that command "%s" has exit status of "%s", but has "%s".), + @old_actual.commandline, + expected.to_s, + actual.to_s + ) end failure_message_when_negated do |actual| - format(%(expected that command "%s" does not have exit status of "%s", but has "%s".), @old_actual.commandline, expected.to_s, actual.to_s) + format( + %(expected that command "%s" does not have exit status of "%s", but has "%s".), + @old_actual.commandline, + expected.to_s, + actual.to_s + ) end end diff --git a/lib/aruba/platforms/windows_environment_variables.rb b/lib/aruba/platforms/windows_environment_variables.rb index 96b11dd1d..cc1655181 100644 --- a/lib/aruba/platforms/windows_environment_variables.rb +++ b/lib/aruba/platforms/windows_environment_variables.rb @@ -22,7 +22,13 @@ module Platforms # # @example But if you copy the ENV to a hash, Ruby treats the keys as case sensitive: # env_copy = ENV.to_hash - # # => {"ALLUSERSPROFILE"=>"C:\\ProgramData", "ANSICON"=>"119x1000 (119x58)", "ANSICON_DEF"=>"7", APPDATA"=>"C:\\Users\\blorf\\AppData\\Roaming", ....} + # # => { + # "ALLUSERSPROFILE"=> + # "C:\\ProgramData", + # "ANSICON"=>"119x1000 (119x58)", + # "ANSICON_DEF"=>"7", + # APPDATA" => "C:\\Users\\blorf\\AppData\\Roaming", .... + # } # env["Path"] # # => ".;.\bin;c:\rubys\ruby-2.1.6-p336\bin;" # env["PATH"] diff --git a/lib/aruba/processes/spawn_process.rb b/lib/aruba/processes/spawn_process.rb index 1ff04dc89..67208bdc5 100644 --- a/lib/aruba/processes/spawn_process.rb +++ b/lib/aruba/processes/spawn_process.rb @@ -66,7 +66,10 @@ def initialize(cmd, exit_timeout, io_wait_timeout, working_directory, # rubocop:disable Metrics/MethodLength def start # rubocop:disable Metrics/LineLength - fail CommandAlreadyStartedError, %(Command "#{commandline}" has already been started. Please `#stop` the command first and `#start` it again. Alternatively use `#restart`.\n#{caller.join("\n")}) if started? + if started? + error_message = %(Command "#{commandline}" has already been started. Please `#stop` the command first and `#start` it again. Alternatively use `#restart`.\n#{caller.join("\n")}) + fail CommandAlreadyStartedError, error_message + end # rubocop:enable Metrics/LineLength @@ -230,11 +233,12 @@ def pid # @param [String] signal # The signal, i.e. 'TERM' def send_signal(signal) - fail CommandAlreadyStoppedError, %(Command "#{commandline}" with PID "#{pid}" has already stopped.) if @process.exited? + error_message = %(Command "#{commandline}" with PID "#{pid}" has already stopped.) + fail CommandAlreadyStoppedError, error_message if @process.exited? Process.kill signal, pid rescue Errno::ESRCH - raise CommandAlreadyStoppedError, %(Command "#{commandline}" with PID "#{pid}" has already stopped.) + raise CommandAlreadyStoppedError, error_message end # Return file system stats for the given command diff --git a/spec/aruba/api/filesystem_spec.rb b/spec/aruba/api/filesystem_spec.rb index 39825b679..bbe2f5bd5 100644 --- a/spec/aruba/api/filesystem_spec.rb +++ b/spec/aruba/api/filesystem_spec.rb @@ -394,10 +394,11 @@ context 'when destination is not a directory' do let(:source) { %w(file1.txt file2.txt file3.txt) } let(:destination) { 'file.txt' } + let(:error_message) { 'Multiples sources can only be copied to a directory' } before(:each) { create_test_files(destination) } - it { expect { @aruba.copy source, destination }.to raise_error ArgumentError, 'Multiples sources can only be copied to a directory' } + it { expect { @aruba.copy source, destination }.to raise_error ArgumentError, error_message } end context 'when a source is the same like destination' do @@ -414,8 +415,9 @@ context 'when a fixture is destination' do let(:source) { '%/copy/file.txt' } let(:destination) { '%/copy/file.txt' } + let(:error_message) { "Using a fixture as destination (#{destination}) is not supported" } - it { expect { @aruba.copy source, destination }.to raise_error ArgumentError, "Using a fixture as destination (#{destination}) is not supported" } + it { expect { @aruba.copy source, destination }.to raise_error ArgumentError, error_message } end end end