Fixed specs and fixes based on failing specs

3 files changed, 222 insertions, 176 deletions

diff --git a/spec/lib/gitlab/slash_commands/command_definition_spec.rb b/spec/lib/gitlab/slash_commands/command_definition_spec.rb
new file mode 100644
index 00000000000..2a75fab24b0
--- /dev/null
+++ b/spec/lib/gitlab/slash_commands/command_definition_spec.rb
@@ -0,0 +1,143 @@
+require 'spec_helper'
+
+describe Gitlab::SlashCommands::CommandDefinition do
+  subject { described_class.new(:command) }
+
+  describe "#all_names" do
+    context "when the command has aliases" do
+      before do
+        subject.aliases = [:alias1, :alias2]
+      end
+
+      it "returns an array with the name and aliases" do
+        expect(subject.all_names).to eq([:command, :alias1, :alias2])
+      end
+    end
+
+    context "when the command doesn't have aliases" do
+      it "returns an array with the name" do
+        expect(subject.all_names).to eq([:command])
+      end
+    end
+  end
+
+  describe "#noop?" do
+    context "when the command has an action block" do
+      before do
+        subject.action_block = -> { }
+      end
+
+      it "returns false" do
+        expect(subject.noop?).to be false
+      end
+    end
+
+    context "when the command doesn't have an action block" do
+      it "returns true" do
+        expect(subject.noop?).to be true
+      end
+    end
+  end
+
+  describe "#available?" do
+    let(:opts) { { go: false } }
+
+    context "when the command has a condition block" do
+      before do
+        subject.condition_block = -> { go }
+      end
+
+      context "when the condition block returns true" do
+        before do
+          opts[:go] = true
+        end
+
+        it "returns true" do
+          expect(subject.available?(opts)).to be true
+        end
+      end
+
+      context "when the condition block returns false" do
+        it "returns false" do
+          expect(subject.available?(opts)).to be false
+        end
+      end
+    end
+
+    context "when the command doesn't have a condition block" do
+      it "returns true" do
+        expect(subject.available?(opts)).to be true
+      end
+    end
+  end
+
+  describe "#execute" do
+    let(:context) { OpenStruct.new(run: false) }
+
+    context "when the command is a noop" do
+      it "doesn't execute the command" do
+        expect(context).not_to receive(:instance_exec)
+
+        subject.execute(context, {})
+
+        expect(context.run).to be false
+      end
+    end
+
+    context "when the command is not a noop" do
+      before do
+        subject.action_block = -> { self.run = true }
+      end
+
+      context "when the command is not available" do
+        before do
+          subject.condition_block = -> { false }
+        end
+
+        it "doesn't execute the command" do
+          subject.execute(context, {})
+
+          expect(context.run).to be false
+        end
+      end
+
+      context "when the command is available" do
+        context "when the command has an exact number of arguments" do
+          before do
+            subject.action_block = ->(arg) { self.run = arg }
+          end
+
+          context "when the command is provided a wrong number of arguments" do
+            it "doesn't execute the command" do
+              subject.execute(context, {}, true, true)
+
+              expect(context.run).to be false
+            end
+          end
+
+          context "when the command is provided the right number of arguments" do
+            it "executes the command" do
+              subject.execute(context, {}, true)
+
+              expect(context.run).to be true
+            end
+          end
+        end
+
+        context "when the command has a variable number of arguments" do
+          before do
+            subject.action_block = ->(*args) { self.run = args.first }
+          end
+
+          context "when the command is provided any number of arguments" do
+            it "executes the command" do
+              subject.execute(context, {}, true, true)
+
+              expect(context.run).to be true
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/slash_commands/dsl_spec.rb b/spec/lib/gitlab/slash_commands/dsl_spec.rb
index 500ff3ca1fe..87be3455baf 100644
--- a/spec/lib/gitlab/slash_commands/dsl_spec.rb
+++ b/spec/lib/gitlab/slash_commands/dsl_spec.rb
@@ -10,9 +10,9 @@ describe Gitlab::SlashCommands::Dsl do
         "Hello World!"
       end
 
-      desc 'A command returning a value'
+      desc { "A command with #{something}" }
       command :returning do
-        return 42
+        42
       end
 
       params 'The first argument'
@@ -28,7 +28,7 @@ describe Gitlab::SlashCommands::Dsl do
         [arg1, arg2]
       end
 
-      command :cc, noop: true
+      command :cc
 
       condition do
         project == 'foo'
@@ -49,182 +49,74 @@ describe Gitlab::SlashCommands::Dsl do
         {
           name: :no_args, aliases: [:none],
           description: 'A command with no args', params: [],
-          condition_block: nil, action_block: a_kind_of(Proc),
-          opts: {}
+          condition_block: nil, action_block: a_kind_of(Proc)
         },
         {
           name: :returning, aliases: [],
           description: 'A command returning a value', params: [],
-          condition_block: nil, action_block: a_kind_of(Proc),
-          opts: {}
+          condition_block: nil, action_block: a_kind_of(Proc)
         },
         {
           name: :one_arg, aliases: [:once, :first],
           description: '', params: ['The first argument'],
-          condition_block: nil, action_block: a_kind_of(Proc),
-          opts: {}
+          condition_block: nil, action_block: a_kind_of(Proc)
         },
         {
           name: :two_args, aliases: [],
           description: '', params: ['The first argument', 'The second argument'],
-          condition_block: nil, action_block: a_kind_of(Proc),
-          opts: {}
+          condition_block: nil, action_block: a_kind_of(Proc)
         },
         {
           name: :cc, aliases: [],
           description: '', params: [],
-          condition_block: nil, action_block: nil,
-          opts: { noop: true }
+          condition_block: nil, action_block: nil
         },
         {
           name: :wildcard, aliases: [],
           description: '', params: [],
-          condition_block: nil, action_block: a_kind_of(Proc),
-          opts: {}
+          condition_block: nil, action_block: a_kind_of(Proc)
         }
       ]
     end
 
     it 'returns an array with commands definitions' do
-      expect(DummyClass.command_definitions).to match_array base_expected
-    end
-
-    context 'with options passed' do
-      context 'when condition is met' do
-        let(:expected) do
-          base_expected << {
-            name: :cond_action, aliases: [],
-            description: '', params: [],
-            condition_block: a_kind_of(Proc), action_block: a_kind_of(Proc),
-            opts: {}
-          }
-        end
-
-        it 'returns an array with commands definitions' do
-          expect(DummyClass.command_definitions(project: 'foo')).to match_array expected
-        end
-      end
-
-      context 'when condition is not met' do
-        it 'returns an array with commands definitions without actions that did not met conditions' do
-          expect(DummyClass.command_definitions(project: 'bar')).to match_array base_expected
-        end
-      end
-
-      context 'when description can be generated dynamically' do
-        it 'returns an array with commands definitions with dynamic descriptions' do
-          base_expected[3][:description] = 'A dynamic description for MERGE REQUEST'
-
-          expect(DummyClass.command_definitions(noteable: 'merge request')).to match_array base_expected
-        end
-      end
-    end
-  end
-
-  describe '.command_names' do
-    let(:base_expected) do
-      [
-        :no_args, :none, :returning, :one_arg,
-        :once, :first, :two_args, :wildcard
-      ]
-    end
-
-    it 'returns an array with commands definitions' do
-      expect(DummyClass.command_names).to eq base_expected
-    end
-
-    context 'with options passed' do
-      context 'when condition is met' do
-        let(:expected) { base_expected << :cond_action }
-
-        it 'returns an array with commands definitions' do
-          expect(DummyClass.command_names(project: 'foo')).to match_array expected
-        end
-      end
-
-      context 'when condition is not met' do
-        it 'returns an array with commands definitions without action that did not met conditions' do
-          expect(DummyClass.command_names(project: 'bar')).to match_array base_expected
-        end
-      end
-    end
-  end
-
-  let(:dummy) { DummyClass.new(nil) }
-
-  describe '#execute_command' do
-    describe 'command with no args' do
-      context 'called with no args' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:no_args)).to eq 'Hello World!'
-        end
-      end
-    end
-
-    describe 'command with an explicit return' do
-      context 'called with no args' do
-        it 'succeeds' do
-          expect { dummy.execute_command(:returning) }.to raise_error(LocalJumpError)
-        end
-      end
-    end
-
-    describe 'command with one arg' do
-      context 'called with one arg' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:one_arg, 42)).to eq 42
-        end
-      end
-    end
-
-    describe 'command with two args' do
-      context 'called with two args' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:two_args, 42, 'foo')).to eq [42, 'foo']
-        end
-      end
-    end
-
-    describe 'noop command' do
-      it 'returns nil' do
-        expect(dummy.execute_command(:cc)).to be_nil
-      end
-    end
-
-    describe 'command with condition' do
-      context 'when condition is not met' do
-        it 'returns nil' do
-          expect(dummy.execute_command(:cond_action)).to be_nil
-        end
-      end
-
-      context 'when condition is met' do
-        let(:dummy) { DummyClass.new('foo') }
-
-        it 'succeeds' do
-          expect(dummy.execute_command(:cond_action, 42)).to eq 42
-        end
-      end
-    end
-
-    describe 'command with wildcard' do
-      context 'called with no args' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:wildcard)).to eq []
-        end
-      end
-
-      context 'called with one arg' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:wildcard, 42)).to eq [42]
-        end
-      end
-
-      context 'called with two args' do
-        it 'succeeds' do
-          expect(dummy.execute_command(:wildcard, 42, 'foo')).to eq [42, 'foo']
-        end
-      end
+      no_args_def, returning_def, one_arg_def, two_args_def, cc_def, cond_action_def, wildcard_def = DummyClass.command_definitions
+
+      expect(no_args_def.name).to eq(:no_args)
+      expect(no_args_def.aliases).to eq([:none])
+      expect(no_args_def.description).to eq('A command with no args')
+      expect(no_args_def.params).to eq([])
+      expect(no_args_def.condition_block).to be_nil
+      expect(no_args_def.action_block).to be_a_kind_of(Proc)
+
+      expect(returning_def.name).to eq(:returning)
+      expect(returning_def.aliases).to eq([])
+      expect(returning_def.description).to be_a_kind_of(Proc)
+      expect(returning_def.to_h(something: "a block description")[:description]).to eq('A command with a block description')
+      expect(returning_def.params).to eq([])
+      expect(returning_def.condition_block).to be_nil
+      expect(returning_def.action_block).to be_a_kind_of(Proc)
+
+      expect(one_arg_def.name).to eq(:one_arg)
+      expect(one_arg_def.aliases).to eq([:once, :first])
+      expect(one_arg_def.description).to eq('')
+      expect(one_arg_def.params).to eq(['The first argument'])
+      expect(one_arg_def.condition_block).to be_nil
+      expect(one_arg_def.action_block).to be_a_kind_of(Proc)
+
+      expect(cc_def.name).to eq(:cc)
+      expect(cc_def.aliases).to eq([])
+      expect(cc_def.description).to eq('')
+      expect(cc_def.params).to eq([])
+      expect(cc_def.condition_block).to be_nil
+      expect(cc_def.action_block).to be_nil
+
+      expect(wildcard_def.name).to eq(:wildcard)
+      expect(wildcard_def.aliases).to eq([])
+      expect(wildcard_def.description).to eq('')
+      expect(wildcard_def.params).to eq([])
+      expect(wildcard_def.condition_block).to be_nil
+      expect(wildcard_def.action_block).to be_a_kind_of(Proc)
     end
   end
 end
diff --git a/spec/lib/gitlab/slash_commands/extractor_spec.rb b/spec/lib/gitlab/slash_commands/extractor_spec.rb
index 8a6801205fa..09f909dcdd2 100644
--- a/spec/lib/gitlab/slash_commands/extractor_spec.rb
+++ b/spec/lib/gitlab/slash_commands/extractor_spec.rb
@@ -1,32 +1,43 @@
 require 'spec_helper'
 
 describe Gitlab::SlashCommands::Extractor do
-  let(:extractor) { described_class.new([:open, :assign, :labels, :power]) }
+  let(:definitions) do
+    Class.new do
+      include Gitlab::SlashCommands::Dsl
+
+      command(:reopen, :open) { }
+      command(:assign) { }
+      command(:labels) { }
+      command(:power) { }
+    end.command_definitions
+  end
+
+  let(:extractor) { described_class.new(definitions) }
 
   shared_examples 'command with no argument' do
     it 'extracts command' do
-      commands = extractor.extract_commands(original_msg)
+      msg, commands = extractor.extract_commands(original_msg)
 
       expect(commands).to eq [['open']]
-      expect(original_msg).to eq final_msg
+      expect(msg).to eq final_msg
     end
   end
 
   shared_examples 'command with a single argument' do
     it 'extracts command' do
-      commands = extractor.extract_commands(original_msg)
+      msg, commands = extractor.extract_commands(original_msg)
 
       expect(commands).to eq [['assign', '@joe']]
-      expect(original_msg).to eq final_msg
+      expect(msg).to eq final_msg
     end
   end
 
   shared_examples 'command with multiple arguments' do
     it 'extracts command' do
-      commands = extractor.extract_commands(original_msg)
+      msg, commands = extractor.extract_commands(original_msg)
 
       expect(commands).to eq [['labels', '~foo ~"bar baz" label']]
-      expect(original_msg).to eq final_msg
+      expect(msg).to eq final_msg
     end
   end
 
@@ -49,7 +60,7 @@ describe Gitlab::SlashCommands::Extractor do
       context 'in the middle of a line' do
         it 'does not extract command' do
           msg = "hello\nworld /open"
-          commands = extractor.extract_commands(msg)
+          msg, commands = extractor.extract_commands(msg)
 
           expect(commands).to be_empty
           expect(msg).to eq "hello\nworld /open"
@@ -59,7 +70,7 @@ describe Gitlab::SlashCommands::Extractor do
       context 'at the end of content' do
         it_behaves_like 'command with no argument' do
           let(:original_msg) { "hello\n/open" }
-          let(:final_msg) { "hello\n" }
+          let(:final_msg) { "hello" }
         end
       end
     end
@@ -82,7 +93,7 @@ describe Gitlab::SlashCommands::Extractor do
       context 'in the middle of a line' do
         it 'does not extract command' do
           msg = "hello\nworld /assign @joe"
-          commands = extractor.extract_commands(msg)
+          msg, commands = extractor.extract_commands(msg)
 
           expect(commands).to be_empty
           expect(msg).to eq "hello\nworld /assign @joe"
@@ -92,14 +103,14 @@ describe Gitlab::SlashCommands::Extractor do
       context 'at the end of content' do
         it_behaves_like 'command with a single argument' do
           let(:original_msg) { "hello\n/assign @joe" }
-          let(:final_msg) { "hello\n" }
+          let(:final_msg) { "hello" }
         end
       end
 
       context 'when argument is not separated with a space' do
         it 'does not extract command' do
           msg = "hello\n/assign@joe\nworld"
-          commands = extractor.extract_commands(msg)
+          msg, commands = extractor.extract_commands(msg)
 
           expect(commands).to be_empty
           expect(msg).to eq "hello\n/assign@joe\nworld"
@@ -125,7 +136,7 @@ describe Gitlab::SlashCommands::Extractor do
       context 'in the middle of a line' do
         it 'does not extract command' do
           msg = %(hello\nworld /labels ~foo ~"bar baz" label)
-          commands = extractor.extract_commands(msg)
+          msg, commands = extractor.extract_commands(msg)
 
           expect(commands).to be_empty
           expect(msg).to eq %(hello\nworld /labels ~foo ~"bar baz" label)
@@ -135,14 +146,14 @@ describe Gitlab::SlashCommands::Extractor do
       context 'at the end of content' do
         it_behaves_like 'command with multiple arguments' do
           let(:original_msg) { %(hello\n/labels ~foo ~"bar baz" label) }
-          let(:final_msg) { "hello\n" }
+          let(:final_msg) { "hello" }
         end
       end
 
       context 'when argument is not separated with a space' do
         it 'does not extract command' do
           msg = %(hello\n/labels~foo ~"bar baz" label\nworld)
-          commands = extractor.extract_commands(msg)
+          msg, commands = extractor.extract_commands(msg)
 
           expect(commands).to be_empty
           expect(msg).to eq %(hello\n/labels~foo ~"bar baz" label\nworld)
@@ -152,7 +163,7 @@ describe Gitlab::SlashCommands::Extractor do
 
     it 'extracts command with multiple arguments and various prefixes' do
       msg = %(hello\n/power @user.name %9.10 ~"bar baz.2"\nworld)
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2"']]
       expect(msg).to eq "hello\nworld"
@@ -160,15 +171,15 @@ describe Gitlab::SlashCommands::Extractor do
 
     it 'extracts multiple commands' do
       msg = %(hello\n/power @user.name %9.10 ~"bar baz.2" label\nworld\n/open)
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to eq [['power', '@user.name %9.10 ~"bar baz.2" label'], ['open']]
-      expect(msg).to eq "hello\nworld\n"
+      expect(msg).to eq "hello\nworld"
     end
 
     it 'does not alter original content if no command is found' do
       msg = 'Fixes #123'
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to be_empty
       expect(msg).to eq 'Fixes #123'
@@ -177,7 +188,7 @@ describe Gitlab::SlashCommands::Extractor do
     it 'does not extract commands inside a blockcode' do
       msg = "Hello\r\n```\r\nThis is some text\r\n/close\r\n/assign @user\r\n```\r\n\r\nWorld"
       expected = msg.delete("\r")
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to be_empty
       expect(msg).to eq expected
@@ -186,7 +197,7 @@ describe Gitlab::SlashCommands::Extractor do
     it 'does not extract commands inside a blockquote' do
       msg = "Hello\r\n>>>\r\nThis is some text\r\n/close\r\n/assign @user\r\n>>>\r\n\r\nWorld"
       expected = msg.delete("\r")
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to be_empty
       expect(msg).to eq expected
@@ -195,7 +206,7 @@ describe Gitlab::SlashCommands::Extractor do
     it 'does not extract commands inside a HTML tag' do
       msg = "Hello\r\n<div>\r\nThis is some text\r\n/close\r\n/assign @user\r\n</div>\r\n\r\nWorld"
       expected = msg.delete("\r")
-      commands = extractor.extract_commands(msg)
+      msg, commands = extractor.extract_commands(msg)
 
       expect(commands).to be_empty
       expect(msg).to eq expected