Polish the exit! command and its tests (#867)

* Remove IRB.irb_exit! method

It's not necessary to introduce a new method just for the exit! command
at this moment.

* Rename ExitForcedAction to ForceExit

* Move force exit tests to a dedicated file

* Fix nested history saving with exit! command

Because we switched to use `Kernel#exit` instead of `exit!`, the outer
session's ensure block in `Irb#run` will be run, which will save the
history. This means the separate check to save history when force exiting
is no longer necessary.

* execute_lines helper should also capture IRB setup's output

This prevents setup warnings from being printed to test output
while allowing those output to be tested.

* Update readme

README.md

@@ -112,6 +112,7 @@ The following commands are available on IRB. You can get the same output from th
 ```txt
 IRB
   exit           Exit the current irb session.
+  exit!          Exit the current process.
   irb_load       Load a Ruby file.
   irb_require    Require a Ruby file.
   source         Loads a given file in the current session.

lib/irb.rb

@@ -889,10 +889,6 @@ def IRB.irb_exit(*)
     throw :IRB_EXIT, false
   end
 
-  def IRB.irb_exit!(*)
-    throw :IRB_EXIT, true
-  end
-
   # Aborts then interrupts irb.
   #
   # Will raise an Abort exception, or the given +exception+.
@@ -972,8 +968,7 @@ def run(conf = IRB.conf)
       conf[:IRB_RC].call(context) if conf[:IRB_RC]
       conf[:MAIN_CONTEXT] = context
 
-      supports_history_saving = conf[:SAVE_HISTORY] && context.io.support_history_saving?
-      save_history = !in_nested_session && supports_history_saving
+      save_history = !in_nested_session && conf[:SAVE_HISTORY] && context.io.support_history_saving?
 
       if save_history
         context.io.load_history
@@ -993,12 +988,8 @@ def run(conf = IRB.conf)
         trap("SIGINT", prev_trap)
         conf[:AT_EXIT].each{|hook| hook.call}
 
-        if forced_exit
-          context.io.save_history if supports_history_saving
-          Kernel.exit(0)
-        else
-          context.io.save_history if save_history
-        end
+        context.io.save_history if save_history
+        Kernel.exit(0) if forced_exit
       end
     end
 

lib/irb/cmd/exit_forced_action.rb → lib/irb/cmd/force_exit.rb

@@ -6,12 +6,12 @@ module IRB
   # :stopdoc:
 
   module ExtendCommand
-    class ExitForcedAction < Nop
+    class ForceExit < Nop
       category "IRB"
       description "Exit the current process."
 
       def execute(*)
-        IRB.irb_exit!
+        throw :IRB_EXIT, true
       rescue UncaughtThrowError
         Kernel.exit(0)
       end

lib/irb/extend-command.rb

@@ -37,7 +37,7 @@ def irb_context
         [:irb_quit, OVERRIDE_PRIVATE_ONLY],
       ],
       [
-        :irb_exit!, :ExitForcedAction, "cmd/exit_forced_action",
+        :irb_exit!, :ForceExit, "cmd/force_exit",
         [:exit!, OVERRIDE_PRIVATE_ONLY],
       ],
 

test/irb/cmd/test_force_exit.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: false
+require 'irb'
+
+require_relative "../helper"
+
+module TestIRB
+  class ForceExitTest < IntegrationTestCase
+    def test_forced_exit_finishes_process_immediately
+      write_ruby <<~'ruby'
+        puts "First line"
+        puts "Second line"
+        binding.irb
+        puts "Third line"
+        binding.irb
+        puts "Fourth line"
+      ruby
+
+      output = run_ruby_file do
+        type "123"
+        type "456"
+        type "exit!"
+      end
+
+      assert_match(/First line\r\n/, output)
+      assert_match(/Second line\r\n/, output)
+      assert_match(/irb\(main\):001> 123/, output)
+      assert_match(/irb\(main\):002> 456/, output)
+      refute_match(/Third line\r\n/, output)
+      refute_match(/Fourth line\r\n/, output)
+    end
+
+    def test_forced_exit_in_nested_sessions
+      write_ruby <<~'ruby'
+        def foo
+          binding.irb
+        end
+
+        binding.irb
+        binding.irb
+      ruby
+
+      output = run_ruby_file do
+        type "123"
+        type "foo"
+        type "exit!"
+      end
+
+      assert_match(/irb\(main\):001> 123/, output)
+    end
+  end
+end

test/irb/test_cmd.rb

@@ -34,17 +34,17 @@ def teardown
     end
 
     def execute_lines(*lines, conf: {}, main: self, irb_path: nil)
-      IRB.init_config(nil)
-      IRB.conf[:VERBOSE] = false
-      IRB.conf[:PROMPT_MODE] = :SIMPLE
-      IRB.conf[:USE_PAGER] = false
-      IRB.conf.merge!(conf)
-      input = TestInputMethod.new(lines)
-      irb = IRB::Irb.new(IRB::WorkSpace.new(main), input)
-      irb.context.return_format = "=> %s\n"
-      irb.context.irb_path = irb_path if irb_path
-      IRB.conf[:MAIN_CONTEXT] = irb.context
       capture_output do
+        IRB.init_config(nil)
+        IRB.conf[:VERBOSE] = false
+        IRB.conf[:PROMPT_MODE] = :SIMPLE
+        IRB.conf[:USE_PAGER] = false
+        IRB.conf.merge!(conf)
+        input = TestInputMethod.new(lines)
+        irb = IRB::Irb.new(IRB::WorkSpace.new(main), input)
+        irb.context.return_format = "=> %s\n"
+        irb.context.irb_path = irb_path if irb_path
+        IRB.conf[:MAIN_CONTEXT] = irb.context
         irb.eval_input
       end
     end
@@ -58,7 +58,9 @@ def test_calling_command_on_a_frozen_main
         "irb_info",
         main: main
       )
-      assert_empty err
+      # Because the main object is frozen, IRB would wrap a delegator around it
+      # Which's exit! method can't be overridden and would raise a warning
+      assert_match(/delegator does not forward private method #exit\!/, err)
       assert_match(/RUBY_PLATFORM/, out)
     end
   end

test/irb/test_debug_cmd.rb

@@ -255,47 +255,6 @@ def test_exit
       assert_match(/irb\(main\):001> next/, output)
     end
 
-    def test_forced_exit_finishes_process_when_nested_sessions
-      write_ruby <<~'ruby'
-        puts "First line"
-        puts "Second line"
-        binding.irb
-        puts "Third line"
-        binding.irb
-        puts "Fourth line"
-      ruby
-
-      output = run_ruby_file do
-        type "123"
-        type "456"
-        type "exit!"
-      end
-
-      assert_match(/First line\r\n/, output)
-      assert_match(/Second line\r\n/, output)
-      assert_match(/irb\(main\):001> 123/, output)
-      assert_match(/irb\(main\):002> 456/, output)
-      refute_match(/Third line\r\n/, output)
-      refute_match(/Fourth line\r\n/, output)
-    end
-
-    def test_forced_exit
-      write_ruby <<~'ruby'
-        puts "Hello"
-        binding.irb
-      ruby
-
-      output = run_ruby_file do
-        type "123"
-        type "456"
-        type "exit!"
-      end
-
-      assert_match(/Hello\r\n/, output)
-      assert_match(/irb\(main\):001> 123/, output)
-      assert_match(/irb\(main\):002> 456/, output)
-    end
-
     def test_quit
       write_ruby <<~'RUBY'
         binding.irb

test/irb/test_history.rb

@@ -379,20 +379,58 @@ def foo
       HISTORY
     end
 
-    def test_history_saving_with_exit!
+    def test_nested_history_saving_from_inner_session_with_exit!
       write_history ""
 
       write_ruby <<~'RUBY'
+        def foo
+          binding.irb
+        end
+
         binding.irb
       RUBY
 
       run_ruby_file do
-        type "'starting session'"
+        type "'outer session'"
+        type "foo"
+        type "'inner session'"
         type "exit!"
       end
 
       assert_equal <<~HISTORY, @history_file.open.read
-        'starting session'
+        'outer session'
+        foo
+        'inner session'
+        exit!
+      HISTORY
+    end
+
+    def test_nested_history_saving_from_outer_session_with_exit!
+      write_history ""
+
+      write_ruby <<~'RUBY'
+        def foo
+          binding.irb
+        end
+
+        binding.irb
+      RUBY
+
+      run_ruby_file do
+        type "'outer session'"
+        type "foo"
+        type "'inner session'"
+        type "exit"
+        type "'outer session again'"
+        type "exit!"
+      end
+
+      assert_equal <<~HISTORY, @history_file.open.read
+        'outer session'
+        foo
+        'inner session'
+        exit
+        'outer session again'
         exit!
       HISTORY
     end