From 467205c247abcd897f6b248024c3c09ccbb086cd Mon Sep 17 00:00:00 2001
From: Matheus Richard <matheusrichardt@gmail.com>
Date: Thu, 8 Aug 2019 16:10:44 -0300
Subject: [PATCH] Refactor web command executor

---
 lib/rubygems/executor.rb                      | 36 +++++++++----------
 .../rubygems/test_gem_commands_web_command.rb | 28 +++++++--------
 2 files changed, 28 insertions(+), 36 deletions(-)

diff --git a/lib/rubygems/executor.rb b/lib/rubygems/executor.rb
index e87e45be23d5..162c308f2c1b 100644
--- a/lib/rubygems/executor.rb
+++ b/lib/rubygems/executor.rb
@@ -7,49 +7,45 @@ module Gem
   module Web
     class Executor
 
-      def supported_options_from_metadata
-        ["source_code_uri", "documentation_uri"]
-      end
-
       def open_page(gem, options)
-        spec = Gem::Specification.find_by_name(gem)
+        begin
+          spec = Gem::Specification.find_by_name(gem)
+        rescue Gem::MissingSpecError => e
+          puts e.message
+          exit 1
+        end
 
         if options[:sourcecode]
           get_info_from_metadata(spec, "source_code_uri")
         elsif options[:doc]
           get_info_from_metadata(spec, "documentation_uri")
-        elsif options[:webpage]
-          open_default_browser(spec.homepage)
         elsif options[:rubygems]
           open_rubygems(gem)
         else # The default option is homepage
-          open_default_browser(spec.homepage)
+          open_browser(spec.homepage)
         end
-      rescue Gem::MissingSpecError => e
-        puts e.message
       end
 
       def get_info_from_metadata(spec, info)
-        return unless supported_options_from_metadata.include?(info)
-
         uri = spec.metadata[info]
-        if !uri.nil? && !uri.empty?
-          open_default_browser(uri)
+
+        if uri.nil? || uri.empty?
+          puts "Gem '#{spec.name}' does not specify #{info}."
         else
-          puts("This gem does not have this information.")
+          open_browser(uri)
         end
       end
 
       def open_rubygems(gem)
-        open_default_browser("https://rubygems.org/gems/#{gem}")
+        open_browser("https://rubygems.org/gems/#{gem}")
       end
 
-      def open_default_browser(uri)
-        open_browser_cmd = ENV['BROWSER']
-        if open_browser_cmd.nil? || open_browser_cmd.empty?
+      def open_browser(uri)
+        browser = ENV['BROWSER']
+        if browser.nil? || browser.empty?
           puts uri
         else
-          system(open_browser_cmd, uri)
+          system(browser, uri)
         end
       end
 
diff --git a/test/rubygems/test_gem_commands_web_command.rb b/test/rubygems/test_gem_commands_web_command.rb
index c2505f224c2b..8fa7978bb4c7 100644
--- a/test/rubygems/test_gem_commands_web_command.rb
+++ b/test/rubygems/test_gem_commands_web_command.rb
@@ -67,10 +67,14 @@ def test_open_the_source_code
   end
 
   def test_open_when_info_is_missing
-    ["-c", "-d"].each do |option|
+    [["-c", "source_code_uri"], ["-d", "documentation_uri"]].each do |test_case|
+      option = test_case[0]
+      info = test_case[1]
       @mock.expect(:metadata, {})
+      @mock.expect(:name, @gem)
+
       Gem::Specification.stub :find_by_name, @mock do
-        assert_output("This gem does not have this information.\n") do
+        assert_output("Gem '#{@gem}' does not specify #{info}.\n") do
           @cmd.handle_options [option, @gem]
           @cmd.execute
         end
@@ -91,22 +95,14 @@ def test_open_rubygems
 
   def test_search_unexisting_gem
     gem = "this-is-an-unexisting-gem"
-    assert_output(/Could not find '#{gem}'/) do
-      @cmd.handle_options [gem]
-      @cmd.execute
+    assert_raises(SystemExit) do
+      assert_output(/Could not find '#{gem}'/) do
+        @cmd.handle_options [gem]
+        @cmd.execute
+      end
     end
   end
 
-  # def test_open_rubygems_if_it_could_not_find_page
-  #   Gem::Specification.stub :find_by_name, @mock do
-  #     out, _ = capture_io do
-  #       @cmd.executor.launch_browser("rails", "")
-  #     end
-  #     assert_match(/Did not find page for rails, opening RubyGems page instead./, out)
-  #     assert_match(/https:\/\/rubygems.org\/gems\/rails/, out)
-  #   end
-  # end
-
   def test_open_browser_if_env_variable_is_set
     open_browser_cmd = "open"
     uri = "http://github.com/rails"
@@ -119,7 +115,7 @@ def test_open_browser_if_env_variable_is_set
 
     ENV.stub :[], env_mock do
       @cmd.executor.stub :system, browser_mock do
-        @cmd.executor.open_default_browser(uri)
+        @cmd.executor.open_browser(uri)
       end
     end