From 60f0eab599be7bae6ddacd139642e6f5ba6f3ce4 Mon Sep 17 00:00:00 2001 From: Tobias Kraze Date: Tue, 5 Dec 2023 17:17:46 +0100 Subject: [PATCH] fix "nested element" case for FieldWithErrorFinder; speed it up; use XPath library to construct xpaths --- lib/spreewald_support/capybara_wrapper.rb | 19 ++++++++ lib/spreewald_support/field_errors.rb | 43 +++++++++++++----- .../tolerance_for_selenium_sync_issues.rb | 22 ++-------- lib/spreewald_support/without_waiting.rb | 13 ++++++ spec/spec_helper.rb | 2 + ...tolerance_for_selenium_sync_issues_spec.rb | 8 ---- .../spreewald_support/without_waiting_spec.rb | 41 +++++++++++++++++ .../views/forms/invalid_rails_form.html.haml | 4 ++ .../shared/features/shared/web_steps.feature | 44 ++++++++++--------- 9 files changed, 138 insertions(+), 58 deletions(-) create mode 100644 lib/spreewald_support/capybara_wrapper.rb create mode 100644 lib/spreewald_support/without_waiting.rb create mode 100644 spec/spreewald_support/without_waiting_spec.rb diff --git a/lib/spreewald_support/capybara_wrapper.rb b/lib/spreewald_support/capybara_wrapper.rb new file mode 100644 index 00000000..93ce6fdc --- /dev/null +++ b/lib/spreewald_support/capybara_wrapper.rb @@ -0,0 +1,19 @@ +module Spreewald + class CapybaraWrapper + def self.default_max_wait_time + if Capybara.respond_to?(:default_max_wait_time) + Capybara.default_max_wait_time + else + Capybara.default_wait_time + end + end + + def self.default_max_wait_time=(value) + if Capybara.respond_to?(:default_max_wait_time=) + Capybara.default_max_wait_time = value + else + Capybara.default_wait_time = value + end + end + end +end diff --git a/lib/spreewald_support/field_errors.rb b/lib/spreewald_support/field_errors.rb index 637352e6..25ae6879 100644 --- a/lib/spreewald_support/field_errors.rb +++ b/lib/spreewald_support/field_errors.rb @@ -1,4 +1,5 @@ require 'spreewald_support/driver_info' +require 'spreewald_support/without_waiting' module Spreewald def self.field_error_class @@ -21,6 +22,7 @@ def self.error_message_xpath_selector=(message_selector) class FieldErrorFinder include Spreewald::DriverInfo + include WithoutWaiting def initialize(page, element) @page = page @@ -28,29 +30,50 @@ def initialize(page, element) end def error_present? - custom_error? || bootstrap3_error? || bootstrap45_error? || rails_error? + without_waiting do + custom_error? || bootstrap3_error? || bootstrap45_error? || rails_error? + end end def custom_error? - Spreewald.field_error_class && @element.has_xpath?("ancestor-or-self::div[contains(@class, \"#{Spreewald.field_error_class}\")]") + return false unless Spreewald.field_error_class + + has_xpath? do |x| + x.ancestor_or_self(:div)[x.attr(:class).contains_word(Spreewald.field_error_class)] + end end def bootstrap3_error? - @element.has_xpath?('ancestor::div[@class="form-group has-error"]') + has_xpath? do |x| + x.ancestor(:div)[x.attr(:class).contains_word('form-group')][x.attr(:class).contains_word('has-error')] + end end def bootstrap45_error? - element_classes = @element[:class] &.split(' ') || [] - invalid_elements = if javascript_capable? - @page.all(':invalid') # Collect all invalid elements as Bootstrap 4 and 5 support client validation - end + without_waiting do + element_classes = @element[:class]&.split(' ') || [] + invalid_elements = if javascript_capable? + @page.all(':invalid') # Collect all invalid elements as Bootstrap 4 and 5 support client validation + end - element_classes.include?('is-invalid') || (invalid_elements && invalid_elements.include?(@element)) + element_classes.include?('is-invalid') || (invalid_elements && invalid_elements.include?(@element)) + end end def rails_error? - parent_element_classes = @element.find(:xpath, '..')[:class] &.split(' ') || [] - parent_element_classes.include?('field_with_errors') + has_xpath? do |x| + x.ancestor(:div)[x.attr(:class).contains_word('field_with_errors')] + end + end + + private + + def has_xpath?(&block) + xpath = XPath.generate(&block) + without_waiting do + @element.has_xpath?(xpath) + end end + end end diff --git a/lib/spreewald_support/tolerance_for_selenium_sync_issues.rb b/lib/spreewald_support/tolerance_for_selenium_sync_issues.rb index 23746946..0f18a43e 100644 --- a/lib/spreewald_support/tolerance_for_selenium_sync_issues.rb +++ b/lib/spreewald_support/tolerance_for_selenium_sync_issues.rb @@ -1,3 +1,5 @@ +require 'spreewald_support/capybara_wrapper' + module ToleranceForSeleniumSyncIssues RETRY_ERRORS = %w[ ActionController::UrlGenerationError @@ -17,24 +19,6 @@ module ToleranceForSeleniumSyncIssues Selenium::WebDriver::Error::NoSuchAlertError ] - class CapybaraWrapper - def self.default_max_wait_time - if Capybara.respond_to?(:default_max_wait_time) - Capybara.default_max_wait_time - else - Capybara.default_wait_time - end - end - - def self.default_max_wait_time=(value) - if Capybara.respond_to?(:default_max_wait_time=) - Capybara.default_max_wait_time = value - else - Capybara.default_wait_time = value - end - end - end - class Patiently WAIT_PERIOD = 0.05 @@ -68,7 +52,7 @@ def retryable_error?(e) end - def patiently(seconds = CapybaraWrapper.default_max_wait_time, &block) + def patiently(seconds = Spreewald::CapybaraWrapper.default_max_wait_time, &block) if page.driver.wait? Patiently.new.patiently(seconds, &block) else diff --git a/lib/spreewald_support/without_waiting.rb b/lib/spreewald_support/without_waiting.rb new file mode 100644 index 00000000..27987081 --- /dev/null +++ b/lib/spreewald_support/without_waiting.rb @@ -0,0 +1,13 @@ +require 'spreewald_support/capybara_wrapper' + +module Spreewald + module WithoutWaiting + def without_waiting + prior_max_wait_time = CapybaraWrapper.default_max_wait_time + CapybaraWrapper.default_max_wait_time = 0 + yield + ensure + CapybaraWrapper.default_max_wait_time = prior_max_wait_time + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cf726788..d5001647 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -3,6 +3,8 @@ require 'pathname' +require 'capybara' + Dir[Pathname.new(__FILE__).join('..', 'support', '**', '*.rb')].each { |f| require f } diff --git a/spec/spreewald_support/tolerance_for_selenium_sync_issues_spec.rb b/spec/spreewald_support/tolerance_for_selenium_sync_issues_spec.rb index 7a2f3c91..0a6f83f1 100644 --- a/spec/spreewald_support/tolerance_for_selenium_sync_issues_spec.rb +++ b/spec/spreewald_support/tolerance_for_selenium_sync_issues_spec.rb @@ -1,13 +1,5 @@ require 'spreewald_support/tolerance_for_selenium_sync_issues' -module Capybara - class ElementNotFound < StandardError; end - - class << self - attr_accessor :default_max_wait_time - end -end - describe ToleranceForSeleniumSyncIssues do subject { World.new } let(:wait_time) { 0.2 } diff --git a/spec/spreewald_support/without_waiting_spec.rb b/spec/spreewald_support/without_waiting_spec.rb new file mode 100644 index 00000000..8c9d9f99 --- /dev/null +++ b/spec/spreewald_support/without_waiting_spec.rb @@ -0,0 +1,41 @@ +require 'spreewald_support/without_waiting' + +describe Spreewald::WithoutWaiting do + + subject do + Class.new do + include Spreewald::WithoutWaiting + end.new + end + + describe '#without_waiting' do + it 'calls the block while setting the Capybara wait time to 0' do + wait_time_in_block = nil + subject.without_waiting do + wait_time_in_block = Capybara.default_max_wait_time + end + expect(wait_time_in_block).to eq(0) + end + + it 'resets the prior wait time' do + prior = Capybara.default_max_wait_time + Capybara.default_max_wait_time = 4 + subject.without_waiting {} + expect(Capybara.default_max_wait_time).to eq(4) + Capybara.default_max_wait_time = prior + end + + it 'resets the prior wait time on exceptions' do + prior = Capybara.default_max_wait_time + Capybara.default_max_wait_time = 4 + expect do + subject.without_waiting do + raise 'error' + end + end.to raise_error('error') + expect(Capybara.default_max_wait_time).to eq(4) + Capybara.default_max_wait_time = prior + end + end + +end diff --git a/tests/shared/app/views/forms/invalid_rails_form.html.haml b/tests/shared/app/views/forms/invalid_rails_form.html.haml index 515b18c7..42b78840 100644 --- a/tests/shared/app/views/forms/invalid_rails_form.html.haml +++ b/tests/shared/app/views/forms/invalid_rails_form.html.haml @@ -11,3 +11,7 @@ .form-group = label_tag 'textarea_control_2', 'C' = text_area_tag 'textarea_control_2', "Textarea control line 1\nTextarea control line 2\n" + .field_with_errors + .form-group + = label_tag 'textarea_control_3', 'Nested field is invalid' + = text_area_tag 'textarea_control_3', "Textarea control line 1\nTextarea control line 2\n" diff --git a/tests/shared/features/shared/web_steps.feature b/tests/shared/features/shared/web_steps.feature index 5438d11d..c957b0c1 100644 --- a/tests/shared/features/shared/web_steps.feature +++ b/tests/shared/features/shared/web_steps.feature @@ -35,53 +35,55 @@ Feature: Web steps Scenario: /^the "([^\"]*)" field should( not)? have an error$/ When I go to "/forms/invalid_rails_form" Then the "A" field should have an error - Then the "B" field should have an error - Then the "Disabled" field should have an error - Then the "C" field should not have an error + And the "B" field should have an error + And the "Disabled" field should have an error + And the "C" field should not have an error + And the "Nested field" field should have an error When I go to "/forms/invalid_bootstrap3_form" Then the "A" field should have an error - Then the "B" field should have an error - Then the "Disabled" field should have an error - Then the "C" field should not have an error + And the "B" field should have an error + And the "Disabled" field should have an error + And the "C" field should not have an error When I go to "/forms/invalid_bootstrap4_form" Then the "A" field should have an error - Then the "B" field should have an error - Then the "Disabled" field should have an error - Then the "C" field should not have an error + And the "B" field should have an error + And the "Disabled" field should have an error + And the "C" field should not have an error When I set the custom field error class to "my-error" And I go to "/forms/invalid_custom_form" Then the "A" field should have an error - Then the "B" field should have an error - Then the "Disabled" field should have an error - Then the "C" field should not have an error + And the "B" field should have an error + And the "Disabled" field should have an error + And the "C" field should not have an error @javascript @field_errors Scenario: /^the "([^"]*)" field should have the error "([^"]*)"$/ When I go to "/forms/invalid_rails_form" Then the "A" field should have the error "is invalid" - Then the "B" field should have the error "is invalid" - Then the "Disabled" field should have the error "is invalid" + And the "B" field should have the error "is invalid" + And the "Disabled" field should have the error "is invalid" + And the "Nested field" field should have an error When I go to "/forms/invalid_bootstrap3_form" Then the "A" field should have the error "is invalid" - Then the "B" field should have the error "is invalid" - Then the "Disabled" field should have the error "is invalid" + And the "B" field should have the error "is invalid" + And the "Disabled" field should have the error "is invalid" When I go to "/forms/invalid_bootstrap4_form" Then the "A" field should have the error "is invalid" - Then the "B" field should have the error "is invalid" - Then the "Disabled" field should have the error "is invalid" + And the "B" field should have the error "is invalid" + And the "Disabled" field should have the error "is invalid" When I set the custom field error class to "my-error" And I set the custom error message xpath to "parent::*/child::*[contains(@class, "my-error-description")]" And I go to "/forms/invalid_custom_form" - Then the "A" field should have the error "is invalid" - Then the "B" field should have the error "is invalid" - Then the "Disabled" field should have the error "is invalid" + And the "A" field should have the error "is invalid" + And the "B" field should have the error "is invalid" + And the "Disabled" field should have the error "is invalid" Scenario: /^I should see a form with the following values:$/ When I go to "/forms/form1"