From bc14ec5173e432ced601c108cae6eca56026d41e Mon Sep 17 00:00:00 2001
From: Sean Edge <asedge@gmail.com>
Date: Sun, 21 Dec 2014 13:11:50 -0500
Subject: [PATCH] Refactor of Gitlab::Shell to hopefully make it more readable
 & testable.  Wrote tests for some Gitlab::Shell & Gitlab::CLI::Helper
 methods.  Other minor improvements and refactors.

---
 lib/gitlab/cli.rb               |   6 +-
 lib/gitlab/cli_helpers.rb       |  48 +++----------
 lib/gitlab/help.rb              |  47 ++++++-------
 lib/gitlab/shell.rb             | 118 ++++++++++++++++++--------------
 spec/gitlab/cli_helpers_spec.rb |  46 +++++++++++++
 spec/gitlab/shell_spec.rb       |  52 ++++++++++++++
 6 files changed, 201 insertions(+), 116 deletions(-)
 create mode 100644 spec/gitlab/cli_helpers_spec.rb
 create mode 100644 spec/gitlab/shell_spec.rb

diff --git a/lib/gitlab/cli.rb b/lib/gitlab/cli.rb
index db6320e20..ef949debf 100644
--- a/lib/gitlab/cli.rb
+++ b/lib/gitlab/cli.rb
@@ -39,8 +39,10 @@ def self.run(cmd, args=[])
       end
 
       begin
-        yaml_load_and_symbolize_hash!(command_args)
-      rescue
+        yaml_load_arguments! command_args
+        command_args.map! {|arg| symbolize_keys arg }
+      rescue => e
+        puts e.message
         exit 1
       end
 
diff --git a/lib/gitlab/cli_helpers.rb b/lib/gitlab/cli_helpers.rb
index 839900b81..271ebafbe 100644
--- a/lib/gitlab/cli_helpers.rb
+++ b/lib/gitlab/cli_helpers.rb
@@ -92,44 +92,18 @@ def actions_table
     def output_table(cmd, args, data)
       case data
       when Gitlab::ObjectifiedHash
-        puts single_record_table(data, cmd, args)
+        puts record_table([data], cmd, args)
       when Array
-        puts multiple_record_table(data, cmd, args)
-      else
-        puts data.inspect
+        puts record_table(data, cmd, args)
+      else  # probably just an error msg
+        puts data
       end
     end
 
-    # Table for a single record.
+    # Table to display records.
     #
     # @return [String]
-    def single_record_table(data, cmd, args)
-      hash = data.to_h
-      keys = hash.keys.sort {|x, y| x.to_s <=> y.to_s }
-      keys = keys & required_fields(args) if required_fields(args).any?
-      keys = keys - excluded_fields(args)
-
-      table do |t|
-        t.title = "Gitlab.#{cmd} #{args.join(', ')}"
-
-        keys.each_with_index do |key, index|
-          case value = hash[key]
-          when Hash
-            value = 'Hash'
-          when nil
-            value = 'null'
-          end
-
-          t.add_row [key, value]
-          t.add_separator unless keys.size - 1 == index
-        end
-      end
-    end
-
-    # Table for multiple records.
-    #
-    # @return [String]
-    def multiple_record_table(data, cmd, args)
+    def record_table(data, cmd, args)
       return 'No data' if data.empty?
 
       arr = data.map(&:to_h)
@@ -181,7 +155,7 @@ def symbolize_keys(hash)
           begin
             newhash[key.to_sym] = symbolize_keys(value)
           rescue NoMethodError
-            puts "error: cannot convert hash key to symbol: #{arg}"
+            puts "error: cannot convert hash key to symbol: #{key}"
             raise
           end
         end
@@ -190,16 +164,12 @@ def symbolize_keys(hash)
       hash
     end
 
-    # Run YAML::load on each arg and symbolize hash keys if found.
+    # Run YAML::load on each arg.
     # @return [Array]
-    def yaml_load_and_symbolize_hash!(args)
+    def yaml_load_arguments!(args)
       args.map! do |arg|
         begin
           arg = YAML::load(arg)
-
-          if arg.is_a?(Hash)
-            arg = symbolize_keys(arg)
-          end
         rescue Psych::SyntaxError
           puts "error: Argument is not valid YAML syntax: #{arg}"
           raise
diff --git a/lib/gitlab/help.rb b/lib/gitlab/help.rb
index 365c40c68..d58ddc11b 100644
--- a/lib/gitlab/help.rb
+++ b/lib/gitlab/help.rb
@@ -4,41 +4,38 @@
 module Gitlab::Help
   extend Gitlab::CLI::Helpers
 
-  def self.get_help(methods,cmd=nil)
+  def self.get_help(methods,cmd)
     help = ''
 
-    if cmd.nil? || cmd == 'help'
-      help = actions_table
-    else
-      ri_cmd = `which ri`.chomp
+    ri_cmd = `which ri`.chomp
 
-      if $? == 0
-        namespace = methods.select {|m| m[:name] === cmd }.map {|m| m[:owner]+'.'+m[:name] }.shift
+    if $? == 0
+      namespace = methods.select {|m| m[:name] === cmd }.
+                          map {|m| m[:owner]+'.'+m[:name] }.shift
 
-        if namespace
-          begin
-            ri_output = `#{ri_cmd} -T #{namespace} 2>&1`.chomp
+      if namespace
+        begin
+          ri_output = `#{ri_cmd} -T #{namespace} 2>&1`.chomp
 
-            if $? == 0
-              ri_output.gsub!(/#{cmd}\((.*?)\)/m, cmd+' \1')
-              ri_output.gsub!(/Gitlab\./, 'gitlab> ')
-              ri_output.gsub!(/Gitlab\..+$/, '')
-              ri_output.gsub!(/\,[\s]*/, ' ')
-              help = ri_output
-            else
-              help = "Ri docs not found for #{namespace}, please install the docs to use 'help'"
-            end
-          rescue => e
-            puts e.message
+          if $? == 0
+            ri_output.gsub!(/#{cmd}\((.*?)\)/m, cmd+' \1')
+            ri_output.gsub!(/Gitlab\./, 'gitlab> ')
+            ri_output.gsub!(/Gitlab\..+$/, '')
+            ri_output.gsub!(/\,[\s]*/, ' ')
+            help = ri_output
+          else
+            help = "Ri docs not found for #{namespace}, please install the docs to use 'help'."
           end
-        else
-          help = "Unknown command: #{cmd}"
+        rescue => e
+          puts e.message
         end
       else
-        help = "'ri' tool not found in your PATH, please install it to use the help."
+        help = "Unknown command: #{cmd}."
       end
+    else
+      help = "'ri' tool not found in your PATH, please install it to use the help."
     end
 
-    puts help
+    help
   end
 end
diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb
index 741a15fe3..86ae02a89 100644
--- a/lib/gitlab/shell.rb
+++ b/lib/gitlab/shell.rb
@@ -8,39 +8,69 @@
 class Gitlab::Shell
   extend Gitlab::CLI::Helpers
 
-  # Start gitlab shell and run infinite loop waiting for user input
-  def self.start
-    history.load
-    actions = Gitlab.actions
-
-    comp = proc { |s| actions.map(&:to_s).grep(/^#{Regexp.escape(s)}/) }
-
-    Readline.completion_proc = comp
-    Readline.completion_append_character = ' '
+  class << self
+    attr_reader :client, :actions, :arguments, :command
+
+    def start
+      setup
+
+      while buffer = Readline.readline('gitlab> ')
+        trap('INT') { quit_shell } # capture ctrl-c
+        
+        parse_input buffer
+
+        yaml_load_arguments! @arguments
+        @arguments.map! { |arg| symbolize_keys arg }
+
+        case buffer
+        when nil, '' 
+          next
+        when 'exit'
+          quit_shell
+        when /^\bhelp\b+/
+          puts help arguments[0]
+        else
+          begin
+            history << buffer
+
+            data = execute command, arguments
+            output_table command, arguments, data
+          rescue ArgumentError => e
+            puts e.message
+            next
+          rescue
+            next
+          end
+        end
+      end
+    end
 
-    client = Gitlab::Client.new(endpoint: '')
+    def parse_input buffer
+      buf = Shellwords.shellwords(buffer)
 
-    while buf = Readline.readline('gitlab> ')
-      trap('INT') { quit_shell } # capture ctrl-c
+      @command = buf.shift
+      @arguments = buf.count > 0 ? buf : []
+    end
 
-      next if buf.nil? || buf.empty?
-      quit_shell if buf == 'exit'
+    def setup
+      history.load
 
-      history << buf
+      Readline.completion_proc = completion
+      Readline.completion_append_character = ' '
 
-      begin
-        buf = Shellwords.shellwords(buf)
-      rescue ArgumentError => e
-        puts e.message
-        next
-      end
+      @client = Gitlab::Client.new(endpoint: '')
+      @actions = Gitlab.actions
+    end
 
-      cmd = buf.shift
-      args = buf.count > 0 ? buf : []
+    def completion
+      proc { |str| actions.map(&:to_s).grep(/^#{Regexp.escape(str)}/) }
+    end
 
-      if cmd == 'help'
+    def help cmd
+      if cmd.nil?
+        actions_table
+      else
         methods = []
-
         actions.each do |action|
           methods << {
             name: action.to_s,
@@ -48,40 +78,28 @@ def self.start
           }
         end
 
-        args[0].nil? ? Gitlab::Help.get_help(methods) :
-                       Gitlab::Help.get_help(methods, args[0])
-        next
-      end
-
-      syntax_errors = false
-
-      begin
-        yaml_load_and_symbolize_hash!(args)
-      rescue
-        syntax_errors = true
+        Gitlab::Help.get_help(methods, arguments[0])
       end
+    end
 
-      # errors have been displayed, return to the prompt
-      next if syntax_errors 
-
-      data = if actions.include?(cmd.to_sym)
+    def execute cmd = command, args = arguments
+      if actions.include?(cmd.to_sym)
         confirm_command(cmd)
         gitlab_helper(cmd, args)
       else
-        "'#{cmd}' is not a valid command. " +
+        "Unknown command: #{cmd}. " +
         "See the 'help' for a list of valid commands."
       end
+    end
 
-      output_table(cmd, args, data)
+    def quit_shell
+      history.save
+      exit
     end
-  end
 
-  def self.quit_shell
-    history.save
-    exit
-  end
+    def history
+      @history ||= History.new
+    end
 
-  def self.history
-    @history ||= History.new
-  end
+  end  # class << self
 end
diff --git a/spec/gitlab/cli_helpers_spec.rb b/spec/gitlab/cli_helpers_spec.rb
new file mode 100644
index 000000000..f7ede8cd3
--- /dev/null
+++ b/spec/gitlab/cli_helpers_spec.rb
@@ -0,0 +1,46 @@
+require 'spec_helper'
+
+describe Gitlab::CLI::Helpers do
+  describe ".valid_command?" do
+    it "should return true when command is valid" do
+      expect(Gitlab::CLI::Helpers.valid_command? 'merge_requests').to be_truthy
+    end
+    it "should return false when command is NOT valid" do
+      expect(Gitlab::CLI::Helpers.valid_command? 'mmmmmerge_requests').to be_falsy
+    end
+  end
+  describe ".symbolize_keys" do
+    context "when input is a Hash" do
+      it "should return a Hash with symbols for keys" do
+        hash = {'key1' => 'val1', 'key2' => 'val2'}
+        symbolized_hash = Gitlab::CLI::Helpers.symbolize_keys(hash)
+        expect(symbolized_hash).to eq({key1: 'val1', key2: 'val2'})
+      end
+    end
+    context "when input is NOT a Hash" do
+      it "should return input untouched" do
+        array = [1, 2, 3]
+        new_array = Gitlab::CLI::Helpers.symbolize_keys(array) 
+        expect(new_array).to eq([1, 2, 3])
+      end
+    end
+  end
+
+  describe ".yaml_load_arguments!" do
+    context "when arguments are YAML" do
+      it "should return Ruby objects" do
+        arguments = ["{foo: bar, sna: fu}"] 
+        Gitlab::CLI::Helpers.yaml_load_arguments! arguments
+        expect(arguments).to eq([{'foo' => 'bar', 'sna' => 'fu'}])
+      end
+    end
+
+    context "when input is NOT valid YAML" do
+      it "should raise" do
+        ruby_array = [1, 2, 3, 4]
+        expect { Gitlab::CLI::Helpers.yaml_load_arguments! ruby_array}.to raise_exception
+      end
+    end
+  end
+
+end
diff --git a/spec/gitlab/shell_spec.rb b/spec/gitlab/shell_spec.rb
new file mode 100644
index 000000000..99fa1f8af
--- /dev/null
+++ b/spec/gitlab/shell_spec.rb
@@ -0,0 +1,52 @@
+require 'spec_helper'
+
+describe Gitlab::Shell do
+  describe ".history" do
+    it "should return a Gitlab::Shell::History instance" do
+      history = Gitlab::Shell.history
+      expect(history).to be_a Gitlab::Shell::History
+    end
+  end
+
+  describe ".setup" do
+    before(:all) do
+      Gitlab::Shell.setup
+    end
+    it "should create an instance of Gitlab::Client in class variable 'client'" do
+      expect(Gitlab::Shell.client).to be_a Gitlab::Client
+    end
+    it "should set array of @actions" do
+      expect(Gitlab::Shell.actions).to be_a Array
+      expect(Gitlab::Shell.actions.sort).to eq(Gitlab.actions.sort)
+    end
+    it "should set the Readline completion_proc" do
+      completion = Readline.completion_proc
+      expect(completion).to be_truthy
+    end
+  end
+
+  describe ".completion" do
+    it "should return a Proc object" do
+      comp = Gitlab::Shell.completion
+      expect(comp).to be_a Proc
+    end
+  end
+
+  describe ".parse_input" do
+    context "with arguments" do
+      it "should set command & arguements" do
+        Gitlab::Shell.parse_input('create_branch 1 "api" "master"') 
+        expect(Gitlab::Shell.command).to eq('create_branch')
+        expect(Gitlab::Shell.arguments).to eq(['1', 'api', 'master'])
+      end
+    end
+      
+    context "without arguments" do
+      it 'should set command & empty arguments' do
+        Gitlab::Shell.parse_input('exit') 
+        expect(Gitlab::Shell.command).to eq('exit')
+        expect(Gitlab::Shell.arguments).to be_empty
+      end
+    end
+  end
+end