More attempts

vinistock · vinistock · commit 572da64a7f89 · 2024-10-04T11:22:00.000-04:00
diff --git a/lib/ruby_lsp/ruby_lsp_rails/addon.rb b/lib/ruby_lsp/ruby_lsp_rails/addon.rb
@@ -46,13 +46,13 @@ def rails_runner_client
         @addon_mutex.synchronize { @rails_runner_client }
       end
 
-      sig { override.params(global_state: GlobalState, message_queue: Thread::Queue).void }
-      def activate(global_state, message_queue)
+      sig { override.params(global_state: GlobalState, outgoing_queue: Thread::Queue).void }
+      def activate(global_state, outgoing_queue)
         @global_state = global_state
-        @message_queue = message_queue
+        @message_queue = outgoing_queue
         @message_queue << Notification.window_log_message("Activating Ruby LSP Rails add-on v#{VERSION}")
 
-        register_additional_file_watchers(global_state: global_state, message_queue: message_queue)
+        register_additional_file_watchers(global_state: global_state, outgoing_queue: outgoing_queue)
         @global_state.index.register_enhancement(IndexingEnhancement.new)
 
         # Start booting the real client in a background thread. Until this completes, the client will be a NullClient
@@ -126,11 +126,11 @@ def workspace_did_change_watched_files(changes)
         end
       end
 
-      sig { params(global_state: GlobalState, message_queue: Thread::Queue).void }
-      def register_additional_file_watchers(global_state:, message_queue:)
+      sig { params(global_state: GlobalState, outgoing_queue: Thread::Queue).void }
+      def register_additional_file_watchers(global_state:, outgoing_queue:)
         return unless global_state.supports_watching_files
 
-        message_queue << Request.new(
+        outgoing_queue << Request.new(
           id: "ruby-lsp-rails-file-watcher",
           method: "client/registerCapability",
           params: Interface::RegistrationParams.new(
diff --git a/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb b/lib/ruby_lsp/ruby_lsp_rails/runner_client.rb
@@ -10,28 +10,34 @@ class RunnerClient
       class << self
         extend T::Sig
 
-        sig { params(message_queue: Thread::Queue).returns(RunnerClient) }
-        def create_client(message_queue)
+        sig { params(outgoing_queue: Thread::Queue).returns(RunnerClient) }
+        def create_client(outgoing_queue)
           if File.exist?("bin/rails")
-            new(message_queue)
+            new(outgoing_queue)
           else
-            message_queue << RubyLsp::Notification.window_log_message(
+            unless outgoing_queue.closed?
+              outgoing_queue << RubyLsp::Notification.window_log_message(
+                <<~MESSAGE.chomp,
+                  Ruby LSP Rails failed to locate bin/rails in the current directory: #{Dir.pwd}
+                  Server dependent features will not be available
+                MESSAGE
+                type: RubyLsp::Constant::MessageType::WARNING,
+              )
+            end
+
+            NullClient.new
+          end
+        rescue Errno::ENOENT, StandardError => e # rubocop:disable Lint/ShadowedException
+          unless outgoing_queue.closed?
+            outgoing_queue << RubyLsp::Notification.window_log_message(
               <<~MESSAGE.chomp,
-                Ruby LSP Rails failed to locate bin/rails in the current directory: #{Dir.pwd}
+                Ruby LSP Rails failed to initialize server: #{e.full_message}
                 Server dependent features will not be available
               MESSAGE
-              type: RubyLsp::Constant::MessageType::WARNING,
+              type: Constant::MessageType::ERROR,
             )
-            NullClient.new
           end
-        rescue Errno::ENOENT, StandardError => e # rubocop:disable Lint/ShadowedException
-          message_queue << RubyLsp::Notification.window_log_message(
-            <<~MESSAGE.chomp,
-              Ruby LSP Rails failed to initialize server: #{e.full_message}
-              Server dependent features will not be available
-            MESSAGE
-            type: Constant::MessageType::ERROR,
-          )
+
           NullClient.new
         end
       end
@@ -47,9 +53,9 @@ class EmptyMessageError < StandardError; end
       sig { returns(String) }
       attr_reader :rails_root
 
-      sig { params(message_queue: Thread::Queue).void }
-      def initialize(message_queue)
-        @message_queue = T.let(message_queue, Thread::Queue)
+      sig { params(outgoing_queue: Thread::Queue).void }
+      def initialize(outgoing_queue)
+        @outgoing_queue = T.let(outgoing_queue, Thread::Queue)
         @mutex = T.let(Mutex.new, Mutex)
         # Spring needs a Process session ID. It uses this ID to "attach" itself to the parent process, so that when the
         # parent ends, the spring process ends as well. If this is not set, Spring will throw an error while trying to
@@ -78,19 +84,19 @@ def initialize(message_queue)
         @stdout.binmode
         @stderr.binmode
 
-        @message_queue << RubyLsp::Notification.window_log_message("Ruby LSP Rails booting server")
+        log_message("Ruby LSP Rails booting server")
         count = 0
 
         begin
           count += 1
           initialize_response = T.must(read_response)
           @rails_root = T.let(initialize_response[:root], String)
         rescue EmptyMessageError
-          @message_queue << RubyLsp::Notification.window_log_message("Ruby LSP Rails is retrying initialize (#{count})")
+          log_message("Ruby LSP Rails is retrying initialize (#{count})")
           retry if count < MAX_RETRIES
         end
 
-        @message_queue << RubyLsp::Notification.window_log_message("Finished booting Ruby LSP Rails server")
+        log_message("Finished booting Ruby LSP Rails server")
 
         unless ENV["RAILS_ENV"] == "test"
           at_exit do
@@ -108,7 +114,7 @@ def initialize(message_queue)
       def register_server_addon(server_addon_path)
         send_notification("server_addon/register", server_addon_path: server_addon_path)
       rescue IncompleteMessageError
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed to register server addon #{server_addon_path}",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -119,7 +125,7 @@ def register_server_addon(server_addon_path)
       def model(name)
         make_request("model", name: name)
       rescue IncompleteMessageError
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed to get model information: #{@stderr.read}",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -139,7 +145,7 @@ def association_target_location(model_name:, association_name:)
           association_name: association_name,
         )
       rescue => e
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed with #{e.message}: #{@stderr.read}",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -150,7 +156,7 @@ def association_target_location(model_name:, association_name:)
       def route_location(name)
         make_request("route_location", name: name)
       rescue IncompleteMessageError
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed to get route location: #{@stderr.read}",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -161,7 +167,7 @@ def route_location(name)
       def route(controller:, action:)
         make_request("route_info", controller: controller, action: action)
       rescue IncompleteMessageError
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed to get route information: #{@stderr.read}",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -170,10 +176,10 @@ def route(controller:, action:)
 
       sig { void }
       def trigger_reload
-        @message_queue << RubyLsp::Notification.window_log_message("Reloading Rails application")
+        log_message("Reloading Rails application")
         send_notification("reload")
       rescue IncompleteMessageError
-        @message_queue << RubyLsp::Notification.window_log_message(
+        log_message(
           "Ruby LSP Rails failed to trigger reload",
           type: RubyLsp::Constant::MessageType::ERROR,
         )
@@ -182,7 +188,7 @@ def trigger_reload
 
       sig { void }
       def shutdown
-        @message_queue << RubyLsp::Notification.window_log_message("Ruby LSP Rails shutting down server")
+        log_message("Ruby LSP Rails shutting down server")
         send_message("shutdown")
         sleep(0.5) # give the server a bit of time to shutdown
         [@stdin, @stdout, @stderr].each(&:close)
@@ -241,7 +247,7 @@ def read_response
         response = JSON.parse(T.must(raw_response), symbolize_names: true)
 
         if response[:error]
-          @message_queue << RubyLsp::Notification.window_log_message(
+          log_message(
             "Ruby LSP Rails error: #{response[:error]}",
             type: RubyLsp::Constant::MessageType::ERROR,
           )
@@ -259,6 +265,13 @@ def force_kill
         # Windows does not support the `TERM` signal, so we're forced to use `KILL` here
         Process.kill(T.must(Signal.list["KILL"]), @wait_thread.pid)
       end
+
+      sig { params(message: ::String, type: ::Integer).void }
+      def log_message(message, type: RubyLsp::Constant::MessageType::LOG)
+        return if @outgoing_queue.closed?
+
+        @outgoing_queue << RubyLsp::Notification.window_log_message(message, type: type)
+      end
     end
 
     class NullClient < RunnerClient
diff --git a/test/ruby_lsp_rails/addon_test.rb b/test/ruby_lsp_rails/addon_test.rb
@@ -68,6 +68,7 @@ def with_addon(&block)
         addon.activate(RubyLsp::GlobalState.new, message_queue)
         thread.join
       ensure
+        T.must(addon).deactivate
         T.must(message_queue).close
       end
     end
diff --git a/test/ruby_lsp_rails/runner_client_test.rb b/test/ruby_lsp_rails/runner_client_test.rb
@@ -7,23 +7,9 @@
 module RubyLsp
   module Rails
     class RunnerClientTest < ActiveSupport::TestCase
-      setup do
-        @message_queue = Thread::Queue.new
-        @client = T.let(RunnerClient.new(@message_queue), RunnerClient)
-      end
-
-      teardown do
-        @client.shutdown
-
-        # On Windows, the server process sometimes takes a lot longer to shutdown and may end up getting force killed,
-        # which makes this assertion flaky
-        assert_predicate(@client, :stopped?) unless Gem.win_platform?
-        @message_queue.close
-      end
-
-      # These are integration tests which start the server. For the more fine-grained tests, see `server_test.rb`.
-
       test "#model returns information for the requested model" do
+        message_queue = Thread::Queue.new
+        client = T.let(RunnerClient.new(message_queue), RunnerClient)
         # These columns are from the schema in the dummy app: test/dummy/db/schema.rb
         columns = [
           ["id", "integer"],
@@ -34,13 +20,23 @@ class RunnerClientTest < ActiveSupport::TestCase
           ["updated_at", "datetime"],
           ["country_id", "integer"],
         ]
-        response = T.must(@client.model("User"))
+        response = T.must(client.model("User"))
         assert_equal(columns, response.fetch(:columns))
         assert_match(%r{db/schema\.rb$}, response.fetch(:schema_file))
+      ensure
+        T.must(client).shutdown
+        assert_predicate(client, :stopped?) unless Gem.win_platform?
+        T.must(message_queue).close
       end
 
       test "returns nil if the request returns a nil response" do
-        assert_nil @client.model("ApplicationRecord") # ApplicationRecord is abstract
+        message_queue = Thread::Queue.new
+        client = T.let(RunnerClient.new(message_queue), RunnerClient)
+        assert_nil(client.model("ApplicationRecord")) # ApplicationRecord is abstract
+      ensure
+        T.must(client).shutdown
+        assert_predicate(client, :stopped?) unless Gem.win_platform?
+        T.must(message_queue).close
       end
 
       test "falls back to null client when bin/rails is not found" do
@@ -58,6 +54,8 @@ class RunnerClientTest < ActiveSupport::TestCase
         assert_instance_of(RubyLsp::Notification, log)
         assert_match("Ruby LSP Rails failed to locate bin/rails in the current directory", log.params.message)
       ensure
+        T.must(client).shutdown
+        assert_predicate(client, :stopped?) unless Gem.win_platform?
         T.must(message_queue).close
         FileUtils.mv("bin/rails_backup", "bin/rails")
       end
@@ -76,6 +74,8 @@ class RunnerClientTest < ActiveSupport::TestCase
         assert_instance_of(RubyLsp::Notification, log)
         assert_match("Ruby LSP Rails failed to initialize server", log.params.message)
       ensure
+        T.must(client).shutdown
+        assert_predicate(client, :stopped?) unless Gem.win_platform?
         T.must(message_queue).close
         FileUtils.mv("test/dummy/config/application.rb.bak", "test/dummy/config/application.rb")
       end
@@ -87,13 +87,13 @@ class RunnerClientTest < ActiveSupport::TestCase
         junk = %{\nputs "1\r\n\r\nhello"}
         File.write("test/dummy/config/application.rb", content + junk)
 
-        capture_subprocess_io do
-          client = RunnerClient.create_client(message_queue)
+        client = RunnerClient.create_client(message_queue)
 
-          response = T.must(client.model("User"))
-          assert(response.key?(:columns))
-        end
+        response = T.must(client.model("User"))
+        assert(response.key?(:columns))
       ensure
+        T.must(client).shutdown
+        assert_predicate(client, :stopped?) unless Gem.win_platform?
         T.must(message_queue).close
         FileUtils.mv("test/dummy/config/application.rb.bak", "test/dummy/config/application.rb")
       end
