Commit 8802b619 authored by Mark Lapierre's avatar Mark Lapierre Committed by Sanad Liaquat

Refactor `wait` and `retry_*` methods

This is the first step in refactoring the Waiter and Retrier modules

* Moves wait into the Retrier module as wait_until, and renames
  the max parameter to max_duration, to convey that its purpose
  is similar to retry_until.
* Combines the loop logic into a single private method that the
  different public wait/retry methods use.
* Allows retry_until to retry up to a certain maximum time limit,
  like wait_until, as well as the original funtionality of waiting
  until a specified condition is met.
* Adds a retry_on_exception parameter to so that it's not necessary
  to nest wait/retry blocks if you want to do both.
* Adds a raise_on_failure parameter, which is the same as
  retry_until's exit_on_failure parameter, but now it can be used
  with wait_until.

Code review change to improve error message
parent 814709c4
......@@ -19,4 +19,5 @@ group :test do
gem 'pry-byebug', '~> 3.5.1', platform: :mri
gem "ruby-debug-ide", "~> 0.7.0"
gem "debase", "~> 0.2.4.1"
gem 'timecop', '~> 0.9.1'
end
......@@ -99,6 +99,7 @@ GEM
childprocess (>= 0.5, < 4.0)
rubyzip (>= 1.2.2)
thread_safe (0.3.6)
timecop (0.9.1)
tzinfo (1.2.5)
thread_safe (~> 0.1)
unf (0.1.4)
......@@ -128,6 +129,7 @@ DEPENDENCIES
rspec_junit_formatter (~> 0.4.1)
ruby-debug-ide (~> 0.7.0)
selenium-webdriver (~> 3.12)
timecop (~> 0.9.1)
BUNDLED WITH
1.17.3
......@@ -488,8 +488,9 @@ module QA
end
autoload :Api, 'qa/support/api'
autoload :Dates, 'qa/support/dates'
autoload :Waiter, 'qa/support/waiter'
autoload :Repeater, 'qa/support/repeater'
autoload :Retrier, 'qa/support/retrier'
autoload :Waiter, 'qa/support/waiter'
autoload :WaitForRequests, 'qa/support/wait_for_requests'
end
end
......
......@@ -31,28 +31,28 @@ module QA
end
def enforce_sso
Support::Retrier.retry_until(sleep_interval: 1.0, exit_on_failure: true) do
Support::Retrier.retry_until(sleep_interval: 1.0, raise_on_failure: true) do
click_element :enforced_sso_toggle_button unless find_element(:enforced_sso_toggle_button)[:class].include?('is-checked')
find_element(:enforced_sso_toggle_button)[:class].include?('is-checked')
end
end
def disable_enforce_sso
Support::Retrier.retry_until(sleep_interval: 1.0, exit_on_failure: true) do
Support::Retrier.retry_until(sleep_interval: 1.0, raise_on_failure: true) do
click_element :enforced_sso_toggle_button if find_element(:enforced_sso_toggle_button)[:class].include?('is-checked')
!find_element(:enforced_sso_toggle_button)[:class].include?('is-checked')
end
end
def enable_group_managed_accounts
Support::Retrier.retry_until(sleep_interval: 1.0, exit_on_failure: true) do
Support::Retrier.retry_until(sleep_interval: 1.0, raise_on_failure: true) do
click_element :group_managed_accounts_toggle_button unless find_element(:group_managed_accounts_toggle_button)[:class].include?('is-checked')
find_element(:group_managed_accounts_toggle_button)[:class].include?('is-checked')
end
end
def disable_group_managed_accounts
Support::Retrier.retry_until(sleep_interval: 1.0, exit_on_failure: true) do
Support::Retrier.retry_until(sleep_interval: 1.0, raise_on_failure: true) do
click_element :group_managed_accounts_toggle_button if find_element(:group_managed_accounts_toggle_button)[:class].include?('is-checked')
!find_element(:group_managed_accounts_toggle_button)[:class].include?('is-checked')
end
......
......@@ -26,20 +26,20 @@ module QA
wait_for_requests
end
def wait(max: 60, interval: 0.1, reload: true)
QA::Support::Waiter.wait(max: max, interval: interval) do
def wait(max: 60, interval: 0.1, reload: true, raise_on_failure: false)
Support::Waiter.wait_until(max_duration: max, sleep_interval: interval, raise_on_failure: raise_on_failure) do
yield || (reload && refresh && false)
end
end
def retry_until(max_attempts: 3, reload: false, sleep_interval: 0)
QA::Support::Retrier.retry_until(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do
def retry_until(max_attempts: 3, reload: false, sleep_interval: 0, raise_on_failure: false)
Support::Retrier.retry_until(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval, raise_on_failure: raise_on_failure) do
yield
end
end
def retry_on_exception(max_attempts: 3, reload: false, sleep_interval: 0.5)
QA::Support::Retrier.retry_on_exception(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do
Support::Retrier.retry_on_exception(max_attempts: max_attempts, reload_page: (reload && self), sleep_interval: sleep_interval) do
yield
end
end
......
......@@ -4,6 +4,7 @@ module QA
module Resource
module Events
MAX_WAIT = 10
RAISE_ON_FAILURE = true
EventNotFoundError = Class.new(RuntimeError)
......@@ -21,7 +22,7 @@ module QA
end
def wait_for_event
event_found = QA::Support::Waiter.wait(max: max_wait) do
event_found = Support::Waiter.wait_until(max_duration: max_wait, raise_on_failure: raise_on_failure) do
yield
end
......@@ -31,6 +32,10 @@ module QA
def max_wait
MAX_WAIT
end
def raise_on_failure
RAISE_ON_FAILURE
end
end
end
end
......
......@@ -352,7 +352,7 @@ module QA
page.visit Runtime::Scenario.gitlab_address
Support::Retrier.retry_until(exit_on_failure: true) do
Support::Retrier.retry_until(raise_on_failure: true) do
Page::Main::Menu.perform(&:sign_out_if_signed_in)
!Page::Main::Menu.perform(&:signed_in?)
end
......
......@@ -53,7 +53,7 @@ module QA
Page::Main::Login.perform do |login_page|
user = Struct.new(:ldap_username, :ldap_password).new('adminuser1', 'password')
QA::Support::Retrier.retry_until(exit_on_failure: true, sleep_interval: 3, max_attempts: 5) do
QA::Support::Retrier.retry_until(raise_on_failure: true, sleep_interval: 3, max_attempts: 5) do
login_page.sign_in_using_ldap_credentials(user: user)
end
end
......
# frozen_string_literal: true
require 'active_support/inflector'
module QA
module Support
module Repeater
DEFAULT_MAX_WAIT_TIME = 60
RetriesExceededError = Class.new(RuntimeError)
WaitExceededError = Class.new(RuntimeError)
def repeat_until(max_attempts: nil, max_duration: nil, reload_page: nil, sleep_interval: 0, raise_on_failure: true, retry_on_exception: false)
attempts = 0
start = Time.now
begin
while remaining_attempts?(attempts, max_attempts) && remaining_time?(start, max_duration)
QA::Runtime::Logger.debug("Attempt number #{attempts + 1}") if max_attempts
result = yield
return result if result
sleep_and_reload_if_needed(sleep_interval, reload_page)
attempts += 1
end
rescue StandardError, RSpec::Expectations::ExpectationNotMetError
raise unless retry_on_exception
attempts += 1
if remaining_attempts?(attempts, max_attempts) && remaining_time?(start, max_duration)
sleep_and_reload_if_needed(sleep_interval, reload_page)
retry
else
raise
end
end
if raise_on_failure
raise RetriesExceededError, "Retry condition not met after #{max_attempts} #{'attempt'.pluralize(max_attempts)}" unless remaining_attempts?(attempts, max_attempts)
raise WaitExceededError, "Wait condition not met after #{max_duration} #{'second'.pluralize(max_duration)}"
end
false
end
private
def sleep_and_reload_if_needed(sleep_interval, reload_page)
sleep(sleep_interval)
reload_page.refresh if reload_page
end
def remaining_attempts?(attempts, max_attempts)
max_attempts ? attempts < max_attempts : true
end
def remaining_time?(start, max_duration)
max_duration ? Time.now - start < max_duration : true
end
end
end
end
......@@ -3,49 +3,61 @@
module QA
module Support
module Retrier
extend Repeater
module_function
def retry_on_exception(max_attempts: 3, reload_page: nil, sleep_interval: 0.5)
QA::Runtime::Logger.debug("with retry_on_exception: max_attempts #{max_attempts}; sleep_interval #{sleep_interval}")
attempts = 0
QA::Runtime::Logger.debug(
<<~MSG.tr("\n", ' ')
with retry_on_exception: max_attempts: #{max_attempts};
reload_page: #{reload_page};
sleep_interval: #{sleep_interval}
MSG
)
begin
QA::Runtime::Logger.debug("Attempt number #{attempts + 1}")
yield
rescue StandardError, RSpec::Expectations::ExpectationNotMetError
sleep sleep_interval
reload_page.refresh if reload_page
attempts += 1
result = nil
repeat_until(
max_attempts: max_attempts,
reload_page: reload_page,
sleep_interval: sleep_interval,
retry_on_exception: true
) do
result = yield
retry if attempts < max_attempts
QA::Runtime::Logger.debug("Raising exception after #{max_attempts} attempts")
raise
# This method doesn't care what the return value of the block is.
# We set it to `true` so that it doesn't repeat if there's no exception
true
end
end
def retry_until(max_attempts: 3, reload_page: nil, sleep_interval: 0, exit_on_failure: false)
QA::Runtime::Logger.debug("with retry_until: max_attempts #{max_attempts}; sleep_interval #{sleep_interval}; reload_page:#{reload_page}")
attempts = 0
QA::Runtime::Logger.debug("ended retry_on_exception")
while attempts < max_attempts
QA::Runtime::Logger.debug("Attempt number #{attempts + 1}")
result = yield
return result if result
result
end
sleep sleep_interval
def retry_until(max_attempts: nil, max_duration: nil, reload_page: nil, sleep_interval: 0, raise_on_failure: false, retry_on_exception: false)
# For backwards-compatibility
max_attempts = 3 if max_attempts.nil? && max_duration.nil?
reload_page.refresh if reload_page
start_msg ||= ["with retry_until:"]
start_msg << "max_attempts: #{max_attempts};" if max_attempts
start_msg << "max_duration: #{max_duration};" if max_duration
start_msg << "reload_page: #{reload_page}; sleep_interval: #{sleep_interval}; raise_on_failure: #{raise_on_failure}; retry_on_exception: #{retry_on_exception}"
QA::Runtime::Logger.debug(start_msg.join(' '))
attempts += 1
end
if exit_on_failure
QA::Runtime::Logger.debug("Raising exception after #{max_attempts} attempts")
raise
result = nil
repeat_until(
max_attempts: max_attempts,
max_duration: max_duration,
reload_page: reload_page,
sleep_interval: sleep_interval,
raise_on_failure: raise_on_failure,
retry_on_exception: retry_on_exception
) do
result = yield
end
QA::Runtime::Logger.debug("ended retry_until")
false
result
end
end
end
......
......@@ -3,30 +3,39 @@
module QA
module Support
module Waiter
DEFAULT_MAX_WAIT_TIME = 60
extend Repeater
module_function
def wait(max: DEFAULT_MAX_WAIT_TIME, interval: 0.1)
QA::Runtime::Logger.debug("with wait: max #{max}; interval #{interval}")
start = Time.now
while Time.now - start < max
result = yield
if result
log_end(Time.now - start)
return result
def wait(max: singleton_class::DEFAULT_MAX_WAIT_TIME, interval: 0.1)
wait_until(max_duration: max, sleep_interval: interval, raise_on_failure: false) do
yield
end
sleep(interval)
end
log_end(Time.now - start)
false
def wait_until(max_duration: singleton_class::DEFAULT_MAX_WAIT_TIME, reload_page: nil, sleep_interval: 0.1, raise_on_failure: false, retry_on_exception: false)
QA::Runtime::Logger.debug(
<<~MSG.tr("\n", ' ')
with wait_until: max_duration: #{max_duration};
reload_page: #{reload_page};
sleep_interval: #{sleep_interval};
raise_on_failure: #{raise_on_failure}
MSG
)
result = nil
self.repeat_until(
max_duration: max_duration,
reload_page: reload_page,
sleep_interval: sleep_interval,
raise_on_failure: raise_on_failure,
retry_on_exception: retry_on_exception
) do
result = yield
end
QA::Runtime::Logger.debug("ended wait_until")
def self.log_end(duration)
QA::Runtime::Logger.debug("ended wait after #{duration} seconds")
result
end
end
end
......
......@@ -12,7 +12,7 @@ module QA
fill_in 'password', with: QA::Runtime::Env.github_password
click_on 'Sign in'
Support::Retrier.retry_until(exit_on_failure: true, sleep_interval: 35) do
Support::Retrier.retry_until(raise_on_failure: true, sleep_interval: 35) do
otp = OnePassword::CLI.new.otp
fill_in 'otp', with: otp
......
......@@ -18,7 +18,7 @@ module QA
dropdown_element = find('.setting-name', text: "Credentials").find(:xpath, "..").find('select')
QA::Support::Retrier.retry_until(exit_on_failure: true) do
QA::Support::Retrier.retry_until(raise_on_failure: true) do
dropdown_element.select "GitLab API token (#{token_description})"
dropdown_element.value != ''
end
......
......@@ -14,7 +14,7 @@ module QA
def visit!
super
QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, exit_on_failure: true) do
QA::Support::Retrier.retry_until(sleep_interval: 3, reload_page: page, max_attempts: 20, raise_on_failure: true) do
page.has_text? 'Welcome to Jenkins!'
end
end
......
......@@ -69,11 +69,11 @@ describe QA::Page::Base do
it 'does not refresh' do
expect(subject).not_to receive(:refresh)
subject.wait(max: 0.01) { true }
subject.wait(max: 0.01, raise_on_failure: false) { true }
end
it 'returns true' do
expect(subject.wait(max: 0.1) { true }).to be_truthy
expect(subject.wait(max: 0.1, raise_on_failure: false) { true }).to be_truthy
end
end
......@@ -81,13 +81,13 @@ describe QA::Page::Base do
it 'refreshes' do
expect(subject).to receive(:refresh).at_least(:once)
subject.wait(max: 0.01) { false }
subject.wait(max: 0.01, raise_on_failure: false) { false }
end
it 'returns false' do
allow(subject).to receive(:refresh)
expect(subject.wait(max: 0.01) { false }).to be_falsey
expect(subject.wait(max: 0.01, raise_on_failure: false) { false }).to be_falsey
end
end
end
......
......@@ -31,18 +31,18 @@ describe QA::Support::Page::Logging do
expect { subject.wait(max: 0) {} }
.to output(/next wait uses reload: true/).to_stdout_from_any_process
expect { subject.wait(max: 0) {} }
.to output(/with wait/).to_stdout_from_any_process
.to output(/with wait_until/).to_stdout_from_any_process
expect { subject.wait(max: 0) {} }
.to output(/ended wait after .* seconds$/).to_stdout_from_any_process
.to output(/ended wait_until$/).to_stdout_from_any_process
end
it 'logs wait with reload false' do
expect { subject.wait(max: 0, reload: false) {} }
.to output(/next wait uses reload: false/).to_stdout_from_any_process
expect { subject.wait(max: 0, reload: false) {} }
.to output(/with wait/).to_stdout_from_any_process
.to output(/with wait_until/).to_stdout_from_any_process
expect { subject.wait(max: 0, reload: false) {} }
.to output(/ended wait after .* seconds$/).to_stdout_from_any_process
.to output(/ended wait_until$/).to_stdout_from_any_process
end
it 'logs scroll_to' do
......
......@@ -33,6 +33,7 @@ describe QA::Resource::Events::Project do
before do
allow(subject).to receive(:max_wait).and_return(0.01)
allow(subject).to receive(:raise_on_failure).and_return(false)
allow(subject).to receive(:parse_body).and_return(all_events)
end
......
This diff is collapsed.
# frozen_string_literal: true
require 'logger'
require 'timecop'
describe QA::Support::Retrier do
before do
logger = ::Logger.new $stdout
logger.level = ::Logger::DEBUG
QA::Runtime::Logger.logger = logger
end
describe '.retry_until' do
context 'when the condition is true' do
it 'logs max attempts (3 by default)' do
expect { subject.retry_until { true } }
.to output(/with retry_until: max_attempts: 3; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process
end
it 'logs max duration' do
expect { subject.retry_until(max_duration: 1) { true } }
.to output(/with retry_until: max_duration: 1; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.retry_until { true } }
.to output(/ended retry_until$/).to_stdout_from_any_process
end
end
context 'when the condition is false' do
it 'logs the start' do
expect { subject.retry_until(max_duration: 0) { false } }
.to output(/with retry_until: max_duration: 0; reload_page: ; sleep_interval: 0; raise_on_failure: false; retry_on_exception: false/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.retry_until(max_duration: 0) { false } }
.to output(/ended retry_until$/).to_stdout_from_any_process
end
end
context 'when max_duration and max_attempts are nil' do
it 'sets max attempts to 3 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(max_attempts: 3))
subject.retry_until
end
end
it 'sets sleep_interval to 0 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0))
subject.retry_until
end
it 'sets raise_on_failure to false by default' do
expect(subject).to receive(:repeat_until).with(hash_including(raise_on_failure: false))
subject.retry_until
end
it 'sets retry_on_exception to false by default' do
expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: false))
subject.retry_until
end
end
describe '.retry_on_exception' do
context 'when the condition is true' do
it 'logs max_attempts, reload_page, and sleep_interval parameters' do
expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { true } }
.to output(/with retry_on_exception: max_attempts: 1; reload_page: ; sleep_interval: 0/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { true } }
.to output(/ended retry_on_exception$/).to_stdout_from_any_process
end
end
context 'when the condition is false' do
it 'logs the start' do
expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { false } }
.to output(/with retry_on_exception: max_attempts: 1; reload_page: ; sleep_interval: 0/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.retry_on_exception(max_attempts: 1, reload_page: nil, sleep_interval: 0) { false } }
.to output(/ended retry_on_exception$/).to_stdout_from_any_process
end
end
it 'does not repeat if no exception is raised' do
loop_counter = 0
return_value = "test passed"
expect(
subject.retry_on_exception(max_attempts: 2) do
loop_counter += 1
return_value
end
).to eq(return_value)
expect(loop_counter).to eq(1)
end
it 'sets retry_on_exception to true' do
expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: true))
subject.retry_on_exception
end
it 'sets max_attempts to 3 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(max_attempts: 3))
subject.retry_on_exception
end
it 'sets sleep_interval to 0.5 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0.5))
subject.retry_on_exception
end
end
end
......@@ -9,29 +9,53 @@ describe QA::Support::Waiter do
QA::Runtime::Logger.logger = logger
end
describe '.wait' do
describe '.wait_until' do
context 'when the condition is true' do
it 'logs the start' do
expect { subject.wait(max: 0) {} }
.to output(/with wait: max 0; interval 0.1/).to_stdout_from_any_process
expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { true } }
.to output(/with wait_until: max_duration: 0; reload_page: ; sleep_interval: 0.1/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.wait(max: 0) {} }
.to output(/ended wait after .* seconds$/).to_stdout_from_any_process
expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { true } }
.to output(/ended wait_until$/).to_stdout_from_any_process
end
end
context 'when the condition is false' do
it 'logs the start' do
expect { subject.wait(max: 0) { false } }
.to output(/with wait: max 0; interval 0.1/).to_stdout_from_any_process
expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { false } }
.to output(/with wait_until: max_duration: 0; reload_page: ; sleep_interval: 0.1/).to_stdout_from_any_process
end
it 'logs the end' do
expect { subject.wait(max: 0) { false } }
.to output(/ended wait after .* seconds$/).to_stdout_from_any_process
expect { subject.wait_until(max_duration: 0, raise_on_failure: false) { false } }
.to output(/ended wait_until$/).to_stdout_from_any_process
end
end
it 'sets max_duration to 60 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(max_duration: 60))
subject.wait_until
end
it 'sets sleep_interval to 0.1 by default' do
expect(subject).to receive(:repeat_until).with(hash_including(sleep_interval: 0.1))
subject.wait_until
end
it 'sets raise_on_failure to false by default' do
expect(subject).to receive(:repeat_until).with(hash_including(raise_on_failure: false))
subject.wait_until
end
it 'sets retry_on_exception to false by default' do
expect(subject).to receive(:repeat_until).with(hash_including(retry_on_exception: false))
subject.wait_until
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment