From edb276b9a95845e81c9b799cb66c72103115c003 Mon Sep 17 00:00:00 2001
From: David Taylor <david@taylorhq.com>
Date: Tue, 8 Aug 2023 10:16:09 +0100
Subject: [PATCH] DEV: Raise exception when capybara finder times out (#22686)

If a selenium finder takes the full wait duration to resolve, that means it has been written inefficiently. Most likely a matcher has been negated incorrectly.

This commit introduces a patch which will raise an error in this situation so that we can catch the issues while developing specs.

This commit also fixes chat's visit_thread helper. It was spinning on `has_css?(".chat-skeleton")` for the full selenium wait duration, and then returns false. That's because the thread is often already fully loaded before `has_css?` is even called. It's now updated to only look for the final expected state.
---
 .../spec/system/page_objects/chat/chat.rb     |  3 +--
 spec/rails_helper.rb                          | 27 +++++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/plugins/chat/spec/system/page_objects/chat/chat.rb b/plugins/chat/spec/system/page_objects/chat/chat.rb
index 694d0719507..6a77ed8402b 100644
--- a/plugins/chat/spec/system/page_objects/chat/chat.rb
+++ b/plugins/chat/spec/system/page_objects/chat/chat.rb
@@ -50,8 +50,7 @@ module PageObjects
 
       def visit_thread(thread)
         visit(thread.url)
-        has_css?(".chat-skeleton")
-        has_no_css?(".chat-skeleton")
+        has_css?(".chat-thread:not(.loading)[data-id=\"#{thread.id}\"]")
       end
 
       def visit_channel_settings(channel)
diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 2d0550dd8f2..c965f488436 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -283,6 +283,33 @@ RSpec.configure do |config|
 
     Capybara::Session.class_eval { prepend IgnoreUnicornCapturedErrors }
 
+    module CapybaraTimeoutExtension
+      class CapybaraTimedOut < StandardError
+        def initialize(wait_time)
+          super "Capybara waited for the full wait duration (#{wait_time}s). " +
+                  "This will slow down the test suite. " +
+                  "Beware of negating the result of selenium's RSpec matchers."
+        end
+      end
+
+      def synchronize(seconds = nil, errors: nil)
+        return super if session.synchronized # Nested synchronize. We only want our logic on the outermost call.
+        begin
+          super
+        rescue StandardError => e
+          seconds = session_options.default_max_wait_time if [nil, true].include? seconds
+          if catch_error?(e, errors) && seconds != 0
+            # This error will only have been raised if the timer expired. Raise our own error instead.
+            raise CapybaraTimedOut.new(seconds)
+          else
+            raise
+          end
+        end
+      end
+    end
+
+    Capybara::Node::Base.prepend(CapybaraTimeoutExtension)
+
     # possible values: OFF, SEVERE, WARNING, INFO, DEBUG, ALL
     browser_log_level = ENV["SELENIUM_BROWSER_LOG_LEVEL"] || "SEVERE"