Improve static code review issues

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Change Log
 
+## [Unreleased](https://github.com/DannyBen/runfile/tree/HEAD)
+
+[Full Changelog](https://github.com/DannyBen/runfile/compare/v0.10.0...HEAD)
+
+**Merged pull requests:**
+
+- Upgrade cucumber to version 3.0.2 [\#23](https://github.com/DannyBen/runfile/pull/23) ([depfu[bot]](https://github.com/apps/depfu))
+
 ## [v0.10.0](https://github.com/DannyBen/runfile/tree/v0.10.0) (2017-06-09)
 [Full Changelog](https://github.com/DannyBen/runfile/compare/v0.9.1...v0.10.0)
 

features/step_definitions/clicumber.rb

@@ -19,7 +19,7 @@
 
 Given(/^the (?:folder|dir|directory) "([^"]*)" (does not )?exists?$/) do |dir, negate|
   if negate
-    Dir.rm_rf(dir) if Dir.exist? dir
+    FileUtils.rm_rf(dir) if Dir.exist? dir
   else
     Dir.mkdir(dir) unless Dir.exist? dir
   end
@@ -37,7 +37,6 @@
   FileUtils.cp_r source, target
 end
 
-
 ## Given...file
 
 Given(/^the file "([^"]*)" (does not )?exists?$/) do |file, negate|
@@ -56,6 +55,22 @@
   FileUtils.cp source, target
 end
 
+## Given...environment
+
+Given(/^the variable "([^"]*)" is (not )?"([^"]*)"$/) do |name, negate, value|
+  if negate
+    ENV[name] = "not_#{value}"
+  else
+    ENV[name] = value
+  end
+end
+
+## Given...ran
+
+Given(/^I have already ran "([^"]*)"$/) do |command|
+  @stdout, @stderr, @status = Open3.capture3 command
+end
+
 ## When...run
 
 When(/^I run: (.+)$/) do |command|
@@ -103,9 +118,9 @@
 Then(/^the (error )?output should (not )?match "([^"]*)"$/) do |stderr, negate, content|
   stream = stderr ? @stderr : @stdout
   if negate
-    expect(stream).to_not match /#{content}/im
+    expect(stream).to_not match(/#{content}/im)
   else
-    expect(stream).to match /#{content}/im 
+    expect(stream).to match(/#{content}/im)
   end
 end
 
@@ -139,9 +154,9 @@
 
 Then(/^the file "([^"]*)" should (not )?match "([^"]*)"$/) do |file, negate, content|
   if negate
-    expect(File.read file).to_not match /#{content}/im
+    expect(File.read file).to_not match(/#{content}/im)
   else
-    expect(File.read file).to match /#{content}/im
+    expect(File.read file).to match(/#{content}/im)
   end
 end
 
@@ -161,6 +176,14 @@
   end
 end
 
+Then(/^the file "([^"]*)" should (not )?be like "([^"]*)"$/) do |file1, negate, file2|
+  if negate
+    expect(File.read(file1).strip).to_not eq File.read(file2).strip
+  else
+    expect(File.read(file1).strip).to eq File.read(file2).strip
+  end
+end
+
 ## Then...dir
 
 Then(/^the (?:folder|dir|directory) "([^"]*)" should (not )?exist$/) do |dir, negate|
@@ -179,12 +202,26 @@
   end
 end
 
+## Then...debug
+
+Then(/^stop for debug$/) do
+  byebug
+end
+
 ## Then...exit code
 
-Then(/^the (?:status|exit) code should (not )?be "([^"]*)"$/) do |negate, code|
+Then(/^the (?:status|exit) code should (not )?be "([^"]+)"$/) do |negate, code|
   if negate
-    expect(@status.to_i).to_not eq code.to_i
+    expect(@status.exitstatus).to_not eq code.to_i
+  else
+    expect(@status.exitstatus).to eq code.to_i
+  end
+end
+
+Then(/^the (?:status|exit) code should mean (success|failure)$/) do |type|
+  if type == "failure"
+    expect(@status.success?).to_not be true
   else
-    expect(@status.to_i).to eq code.to_i
+    expect(@status.success?).to be true
   end
 end

lib/runfile/docopt_helper.rb

@@ -9,20 +9,20 @@ module Runfile
   class DocoptHelper
     include Colsole
 
-    # The constructor expects to get all the textual details
-    # needed to generate a docopt document (name, version, 
+    # The constructor expects to an object that responds to all the 
+    # textual details needed to generate a docopt document (name, version, 
     # summary, options) and an array of Action objects.
     # The superspace argument will be the name of runfile, in case we
     # are running a named.runfile. It is only needed to generate the 
     # proper `run superspace (-h|--help|--version)` line
-    def initialize(superspace, name, version, summary, actions, options, examples)
-      @superspace = superspace
-      @name       = name
-      @version    = version
-      @summary    = summary
-      @actions    = actions
-      @options    = options
-      @examples   = examples
+    def initialize(options)
+      @superspace = options.superspace
+      @name       = options.name
+      @version    = options.version
+      @summary    = options.summary
+      @actions    = options.actions
+      @options    = options.options
+      @examples   = options.examples
     end
 
     # Generate a document based on all the actions, help messages
@@ -42,7 +42,7 @@ def docopt
     # Return all docopt lines for the 'Usage' section
     def docopt_usage 
       doc = ["\nUsage:"];
-      @actions.each do |name, action|
+      @actions.each do |_name, action|
         doc << "  run #{action.usage}" unless action.usage == false
       end
       basic_flags = @version ? "(-h|--help|--version)" : "(-h|--help)"
@@ -101,11 +101,10 @@ def docopt_examples(width)
       doc
     end
 
-
     # Call the docopt handler, which will either return a parsed
     # arguments list, or halt execution and show usage.
     def args(argv)
-      Docopt::docopt(docopt, version: @version, argv:argv)
+      Docopt.docopt(docopt, version: @version, argv:argv)
     end
   end
 end

lib/runfile/dsl.rb

@@ -74,27 +74,27 @@ def execute(command_string)
     # Run a command, wait until it is done and continue
     #   run 'rails server'
     def run(*args)
-      ExecHandler.instance.run *args
+      ExecHandler.instance.run(*args)
     end
 
     # Run a command, wait until it is done, then exit
     #   run! 'rails server'
     def run!(*args)
-      ExecHandler.instance.run! *args
+      ExecHandler.instance.run!(*args)
     end
 
     # Run a command in the background, optionally log to a log file and save
     # the process ID in a pid file
     #   run_bg 'rails server', pid: 'rails', log: 'tmp/log.log'
     def run_bg(*args)
-      ExecHandler.instance.run_bg *args
+      ExecHandler.instance.run_bg(*args)
     end
 
     # Stop a command started with 'run_bg'. Provide the name of he pid file you 
     # used in 'run_bg'
     #   stop_bg 'rails'
     def stop_bg(*args)
-      ExecHandler.instance.stop_bg *args
+      ExecHandler.instance.stop_bg(*args)
     end
 
     # Set a block to be called before each run. The block should return
@@ -105,15 +105,15 @@ def stop_bg(*args)
     #     command
     #   end
     def before_run(&block)
-      ExecHandler.instance.before_run &block
+      ExecHandler.instance.before_run(&block)
     end
 
     # Set a block to be called after each run
     #   before_run do |command|
     #     puts "AFTER #{command}"
     #   end
     def after_run(&block)
-      ExecHandler.instance.after_run &block
+      ExecHandler.instance.after_run(&block)
     end
 
     # Also allow to use 'endcommand' instead of 'command' to end

lib/runfile/runfile_helper.rb

@@ -110,7 +110,7 @@ def runfile_folders_with_path
     def say_runfile_list(runfiles)
       runfile_paths = runfiles.map { |f| File.dirname f }
       max = runfile_paths.max_by(&:length).size
-      width, height = detect_terminal_size
+      width = detect_terminal_size[0]
       runfiles.each do |f|
         f[/([^\/]+).runfile$/]
         command  = "run #{$1}"

lib/runfile/runner.rb

@@ -13,7 +13,7 @@ class Runner
     include SettingsMixin
 
     attr_accessor :last_usage, :last_help, :name, :version, 
-      :summary, :namespace, :superspace
+      :summary, :namespace, :superspace, :actions, :examples, :options
 
     # Initialize all variables to sensible defaults.
     def initialize
@@ -41,7 +41,7 @@ def execute(argv, filename='Runfile')
       rescue => ex
         abort "Runfile error:\n#{ex.message}\n#{ex.backtrace[0]}"
       end
-      run *argv
+      run(*argv)
     end
 
     # Add an action to the @actions array, and use the last known
@@ -92,7 +92,7 @@ def run(*argv)
     # function. Expects to get a single string that looks as if
     # it was typed in the command prompt.
     def cross_call(command_string) 
-      argv = command_string.split /\s(?=(?:[^"]|"[^"]*")*$)/
+      argv = command_string.split(/\s(?=(?:[^"]|"[^"]*")*$)/)
       begin
         docopt_exec argv
       rescue Docopt::Exit => ex
@@ -108,7 +108,7 @@ def cross_call(command_string)
     # This should always be called in a begin...rescue block and
     # you should handle the Docopt::Exit exception.
     def docopt_exec(argv)
-      helper = DocoptHelper.new(@superspace, @name, @version, @summary, @actions, @options, @examples)
+      helper = DocoptHelper.new(self)
       args   = helper.args argv
       action = find_action argv
       action or abort "Runfile error: Action not found"