From 6e3b17cc69102e6497d927fc48f5af4aa25224cc Mon Sep 17 00:00:00 2001
From: aaronskiba <askiba@ualberta.ca>
Date: Tue, 18 Mar 2025 10:44:30 -0600
Subject: [PATCH 1/4] Refactor/Consolidate Capybara config

- Moved all Capybara settings into `spec/support/capybara.rb` to centralize test configuration.
- Use `Capybara.default_driver = :rack_test` (prior to this, `spec/rails_helper.rb` was overriding the assignment)
- Use `Capybara.javascript_driver = :selenium_chrome_headless_custom` (previously defined in `spec/rails_helper.rb`)
- Use `Capybara.default_max_wait_time = 10`
---
 spec/rails_helper.rb     | 18 ------------------
 spec/support/capybara.rb | 19 ++++++++++++-------
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb
index 4be8e3b936..98c265b5cf 100644
--- a/spec/rails_helper.rb
+++ b/spec/rails_helper.rb
@@ -80,21 +80,3 @@
   config.include Devise::Test::ControllerHelpers, type: :view
   config.include Pundit::Matchers, type: :policy
 end
-
-Capybara.register_driver :headless_chrome do |app|
-  options = Selenium::WebDriver::Chrome::Options.new
-  options.add_argument('--headless')
-  options.add_argument('--no-sandbox')
-  options.add_argument('--disable-dev-shm-usage')
-
-  Capybara::Selenium::Driver.new(
-    app,
-    browser: :chrome,
-    options: options
-  )
-end
-
-Capybara.javascript_driver = :headless_chrome
-Capybara.default_driver = :headless_chrome
-# Configure Capybara to wait longer for elements to appear
-Capybara.default_max_wait_time = 20
diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb
index 50f5e42547..08b01eebd9 100644
--- a/spec/support/capybara.rb
+++ b/spec/support/capybara.rb
@@ -3,19 +3,24 @@
 # Use Puma as the webserver for feature tests
 Capybara.server = :puma, { Silent: true }
 
-# Use the fast rack_test driver for non-feature tests by default
-Capybara.default_driver = :rack_test
-
 # Create a custom driver based on Capybara's :selenium_chrome_headless driver
-# This resolves a ElementClickInterceptedError when executing `click_button 'Sign in'` with DMP Assistant
-Capybara.register_driver :selenium_chrome_headless_add_window_size do |app|
+Capybara.register_driver :selenium_chrome_headless_custom do |app|
   # Get a copy of the default options for Capybara's :selenium_chrome_headless driver
   options = Capybara.drivers[:selenium_chrome_headless].call.options[:options].dup
-  options.add_argument('--window-size=1920,1080') # default window-size is only (800x600)
+  # Increasing window size resolves ElementClickInterceptedError (default window-size is only (800x600))
+  options.add_argument('--window-size=1920,1080')
   # Create a new Selenium driver with the customised options
   Capybara::Selenium::Driver.new(app, browser: :chrome, options: options)
 end
 
+# Use the fast rack_test driver for non-feature tests by default
+Capybara.default_driver = :rack_test
+
+Capybara.javascript_driver = :selenium_chrome_headless_custom
+
+# Configure Capybara to wait longer for elements to appear
+Capybara.default_max_wait_time = 10
+
 RSpec.configure do |config|
   config.before(:each, type: :feature, js: false) do
     Capybara.use_default_driver
@@ -23,6 +28,6 @@
 
   # Use the Selenium headless Chrome driver for feature tests
   config.before(:each, type: :feature, js: true) do
-    Capybara.current_driver = :selenium_chrome_headless_add_window_size
+    Capybara.current_driver = :selenium_chrome_headless_custom
   end
 end

From 8dbbddf2273e5aae092a022e2fedc1cbb18f7207 Mon Sep 17 00:00:00 2001
From: aaronskiba <askiba@ualberta.ca>
Date: Tue, 18 Mar 2025 16:40:06 -0600
Subject: [PATCH 2/4] Fix Chrome 134 breaking changes/Add expect checks

- These changes address the Chrome 134 breaking changes (https://github.com/DMPRoadmap/roadmap/issues/3485).
- Added `expect` statements to ensure that page loads are completing successfully.
  - The `expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})` statements help verify the page load.
  - It would be better to implement a notification such as "Customised template created successfully." and use that to verify the page load.
- These changes should be revisited (and possibly reverted) when a fix is available for these breaking changes via Chrome, Capybara, or Selenium.
---
 .../annotations/annotations_editing_spec.rb   | 22 +++++++++++++++++--
 spec/features/feedback_requests_spec.rb       |  1 +
 spec/features/plans_spec.rb                   |  1 +
 spec/features/registrations_spec.rb           |  1 +
 spec/features/sessions_spec.rb                |  1 +
 .../templates/templates_copying_spec.rb       |  1 +
 .../templates/templates_editings_spec.rb      |  4 ++++
 .../templates_upgrade_customisations_spec.rb  |  9 +++++++-
 spec/support/helpers/sessions_helper.rb       |  1 +
 9 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/spec/features/annotations/annotations_editing_spec.rb b/spec/features/annotations/annotations_editing_spec.rb
index 0edfc4a25e..3101b262d4 100644
--- a/spec/features/annotations/annotations_editing_spec.rb
+++ b/spec/features/annotations/annotations_editing_spec.rb
@@ -37,6 +37,9 @@
     end
     expect do
       click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     end.to change { Template.count }.by(1)
 
     # New Template created
@@ -53,7 +56,13 @@
     # NOTE: This is question 2, since Annotation was copied upon clicking "Customise"
     within("#edit_question_#{template.question_ids.last}") do
       # Expect it to destroy the newly cleared Annotation
-      expect { click_button 'Save' }.not_to change { Annotation.count }
+      expect do
+        click_button 'Save'
+        current_path = org_admin_template_phase_path(template,
+                                                     template.phases.first) +
+                       "?section=#{template.phases.first.sections.first.id}"
+        expect(page).to have_current_path(current_path)
+      end.not_to change { Annotation.count }
     end
     expect(annotation.text).to eql('Foo bar')
     expect(Annotation.order('created_at').last.text).to eql('<p>Noo bar</p>')
@@ -67,6 +76,9 @@
     end
     expect do
       click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     end.to change { Template.count }.by(1)
     template = Template.last
     click_link 'Customise phase'
@@ -79,7 +91,13 @@
     # NOTE: This is question 2, since Annotation was copied upon clicking "Customise"
     within("#edit_question_#{template.question_ids.last}") do
       # Expect it to destroy the newly cleared Annotation
-      expect { click_button 'Save' }.to change { Annotation.count }.by(-1)
+      expect do
+        click_button 'Save'
+        current_path = org_admin_template_phase_path(template,
+                                                     template.phases.first) +
+                       "?section=#{template.phases.first.sections.first.id}"
+        expect(page).to have_current_path(current_path)
+      end.to change { Annotation.count }.by(-1)
     end
     expect(page).not_to have_errors
   end
diff --git a/spec/features/feedback_requests_spec.rb b/spec/features/feedback_requests_spec.rb
index 4d6eaccfdf..5c06ccb5eb 100644
--- a/spec/features/feedback_requests_spec.rb
+++ b/spec/features/feedback_requests_spec.rb
@@ -40,6 +40,7 @@
     end
 
     # Expectations
+    expect(page).to have_content('Feedback has been requested.')
     expect(plan.reload).to be_feedback_requested
     expect(ActionMailer::Base.deliveries).to have_exactly(1).item
   end
diff --git a/spec/features/plans_spec.rb b/spec/features/plans_spec.rb
index eb7ba84a43..9593fc2491 100644
--- a/spec/features/plans_spec.rb
+++ b/spec/features/plans_spec.rb
@@ -45,6 +45,7 @@
     click_button 'Create plan'
 
     # Expectations
+    expect(page).to have_content('Successfully created the plan.')
     expect(@user.plans).to be_one
     @plan = Plan.last
     expect(current_path).to eql(plan_path(@plan))
diff --git a/spec/features/registrations_spec.rb b/spec/features/registrations_spec.rb
index 50ef4d60ab..c9430a6d90 100644
--- a/spec/features/registrations_spec.rb
+++ b/spec/features/registrations_spec.rb
@@ -26,6 +26,7 @@
     click_button 'Create account'
 
     # Expectations
+    expect(page).to have_text('Welcome! You have signed up successfully.')
     expect(current_path).to eql(plans_path)
     expect(page).to have_text(user_attributes[:firstname])
     expect(page).to have_text(user_attributes[:surname])
diff --git a/spec/features/sessions_spec.rb b/spec/features/sessions_spec.rb
index 9fb3d7682e..4313605f94 100644
--- a/spec/features/sessions_spec.rb
+++ b/spec/features/sessions_spec.rb
@@ -15,6 +15,7 @@
     click_button 'Sign in'
 
     # Expectation
+    expect(page).to have_content('Signed in successfully.')
     expect(current_path).to eql(plans_path)
     expect(page).to have_text(user.firstname)
     expect(page).to have_text(user.surname)
diff --git a/spec/features/templates/templates_copying_spec.rb b/spec/features/templates/templates_copying_spec.rb
index 6910c47c5d..42f668aae3 100644
--- a/spec/features/templates/templates_copying_spec.rb
+++ b/spec/features/templates/templates_copying_spec.rb
@@ -34,6 +34,7 @@
     end
 
     # Expectations
+    expect(page).to have_content('Template was successfully copied.')
     expect(Template.count).to eql(2)
     new_template = Template.last
     expect(new_template.title).to include(parent_template.title)
diff --git a/spec/features/templates/templates_editings_spec.rb b/spec/features/templates/templates_editings_spec.rb
index a56237f880..e3bb56f276 100644
--- a/spec/features/templates/templates_editings_spec.rb
+++ b/spec/features/templates/templates_editings_spec.rb
@@ -26,10 +26,14 @@
 
   scenario "Admin edits a Template's existing question", :js do
     click_link 'Customisable Templates'
+    expect(page).to have_current_path(customisable_org_admin_templates_path)
     within("#template_#{template.id}") do
       click_button 'Actions'
     end
     click_link 'Customise'
+    # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+    # However, the test is currently evaluating Template.count prior to the new Template being created
+    expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
     # New template created
     template = Template.last
     within("#phase_#{template.phase_ids.first}") do
diff --git a/spec/features/templates/templates_upgrade_customisations_spec.rb b/spec/features/templates/templates_upgrade_customisations_spec.rb
index 715691b69c..252a56d947 100644
--- a/spec/features/templates/templates_upgrade_customisations_spec.rb
+++ b/spec/features/templates/templates_upgrade_customisations_spec.rb
@@ -36,7 +36,12 @@
     click_link('Customisable Templates')
 
     click_button 'Actions'
-    expect { click_link 'Customise' }.to change { Template.count }.by(1)
+    expect do
+      click_link 'Customise'
+      # `org_admin_template_path(Template.last)` would be preferred over %r{#{org_admin_templates_path}/\d+}
+      # However, the test is currently evaluating Template.count prior to the new Template being created
+      expect(page).to have_current_path(%r{#{org_admin_templates_path}/\d+})
+    end.to change { Template.count }.by(1)
 
     customized_template = Template.last
 
@@ -48,6 +53,7 @@
     # Publish our customisation
     click_button 'Actions'
     click_link 'Publish'
+    expect(page).to have_text('Your customisation has been published')
     expect(customized_template.reload.published?).to eql(true)
 
     # Move to the other funder Org's Templates
@@ -88,6 +94,7 @@
 
     click_button 'Actions'
     click_link 'Publish changes'
+    expect(page).to have_text('Your template has been published')
     expect(new_funder_template.reload.published?).to eql(true)
 
     # Go back to the original Org...
diff --git a/spec/support/helpers/sessions_helper.rb b/spec/support/helpers/sessions_helper.rb
index 3fa530d4e9..a906723092 100644
--- a/spec/support/helpers/sessions_helper.rb
+++ b/spec/support/helpers/sessions_helper.rb
@@ -20,5 +20,6 @@ def sign_in_as_user(user)
       fill_in 'Password', with: user.password.presence || 'password'
       click_button 'Sign in'
     end
+    expect(page).to have_content('Signed in successfully.')
   end
 end

From ff0417519d43d6af9b293b06fc2b9b70ac4d44cc Mon Sep 17 00:00:00 2001
From: aaronskiba <askiba@ualberta.ca>
Date: Wed, 19 Mar 2025 11:54:12 -0600
Subject: [PATCH 3/4] Let Capybara retry Selenium WebDriver UnknownError

- This code change allows Capybara to retry an action when the following error is encountered:
```
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,"message":"Node with given id does not belong to the document"}
```
- An example of this error can be seen here (https://github.com/DMPRoadmap/roadmap/actions/runs/13951080715/job/39050483020)
- This handling has been added to address the breaking changes encountered between Chrome 134 and our features tests (https://github.com/DMPRoadmap/roadmap/issues/3485)
---
 spec/support/capybara.rb | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/spec/support/capybara.rb b/spec/support/capybara.rb
index 08b01eebd9..b85b0db32b 100644
--- a/spec/support/capybara.rb
+++ b/spec/support/capybara.rb
@@ -29,5 +29,19 @@
   # Use the Selenium headless Chrome driver for feature tests
   config.before(:each, type: :feature, js: true) do
     Capybara.current_driver = :selenium_chrome_headless_custom
+    add_invalid_element_error
+  end
+end
+
+# Mitigate the following error by having Capybara retry an action when it occurs:
+# - Selenium::WebDriver::Error::UnknownError:
+# - unknown error: unhandled inspector error:
+# - {"code":-32000,"message":"Node with given id does not belong to the document"}
+# Source: https://github.com/teamcapybara/capybara/issues/2800#issuecomment-2728801284
+def add_invalid_element_error
+  return unless page.driver.respond_to?(:invalid_element_errors)
+
+  page.driver.invalid_element_errors.tap do |errors|
+    errors << Selenium::WebDriver::Error::UnknownError unless errors.include?(Selenium::WebDriver::Error::UnknownError)
   end
 end

From 9e47493556fa030143dcd3e35a0f11a14d56841a Mon Sep 17 00:00:00 2001
From: aaronskiba <askiba@ualberta.ca>
Date: Wed, 19 Mar 2025 15:37:01 -0600
Subject: [PATCH 4/4] Fix Chrome 134 breaking changes/Add expect check

This change adds to commit 8dbbddf2273e5aae092a022e2fedc1cbb18f7207
---
 spec/features/templates/templates_editings_spec.rb | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/spec/features/templates/templates_editings_spec.rb b/spec/features/templates/templates_editings_spec.rb
index e3bb56f276..f8f5e3713f 100644
--- a/spec/features/templates/templates_editings_spec.rb
+++ b/spec/features/templates/templates_editings_spec.rb
@@ -46,6 +46,10 @@
       tinymce_fill_in(:"question_annotations_attributes_annotation_#{$1}_text", with: 'Foo bar')
       # rubocop:enable Lint/UselessAssignment, Style/PerlBackrefs
       click_button 'Save'
+      current_path = org_admin_template_phase_path(template,
+                                                   template.phases.first) +
+                     "?section=#{template.phases.first.sections.first.id}"
+      expect(page).to have_current_path(current_path)
     end
     # Make sure annotation has been updated
     expect(Question.find(template.question_ids.first).annotations.first.text).to eql('<p>Foo bar</p>')