Merge branch 'acunskis-eventually_matcher_improvements' into 'master'

QA: Unify eventually matcher arguments

See merge request gitlab-org/gitlab!67823

doc/development/testing_guide/end_to_end/best_practices.md

@@ -338,6 +338,16 @@ Page::Project::Pipeline::Show.perform do |pipeline|
 end
 ```
 
+### Use `eventually_` matchers for expectations that require waiting
+
+When something requires waiting to be matched, use `eventually_` matchers with clear wait duration definition.
+
+`Eventually` matchers use the following naming pattern: `eventually_${rspec_matcher_name}`. They are defined in [eventually_matcher.rb](https://gitlab.com/gitlab-org/gitlab/-/blob/master/qa/spec/support/matchers/eventually_matcher.rb).
+
+```ruby
+expect { async_value }.to eventually_eq(value).within(max_duration: 120, max_attempts: 60, reload_page: page)
+```
+
 ### Create negatable matchers to speed `expect` checks
 
 However, sometimes we want to check that something is _not_ as we _don't_ want it to be. In other

qa/qa/specs/features/api/1_manage/bulk_import_group_spec.rb

@@ -81,7 +81,9 @@ module QA
         Page::Group::BulkImport.perform do |import_page|
           imported_group
 
-          expect { imported_group.import_status }.to eventually_eq('finished').within(duration: 300, interval: 2)
+          expect { imported_group.import_status }.to(
+            eventually_eq('finished').within(max_duration: 300, sleep_interval: 2)
+          )
 
           aggregate_failures do
             expect(imported_group.reload!).to eq(source_group)

qa/qa/specs/features/api/1_manage/import_github_repo_spec.rb

@@ -33,7 +33,7 @@ module QA
       it 'imports Github repo via api', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/issues/1858' do
         imported_project # import the project
 
-        expect { imported_project.reload!.import_status }.to eventually_eq('finished').within(duration: 90)
+        expect { imported_project.reload!.import_status }.to eventually_eq('finished').within(max_duration: 90)
 
         aggregate_failures do
           verify_repository_import

qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb

@@ -138,7 +138,7 @@ module QA
             raise "Import of '#{imported_project.name}' failed!" if status == 'failed'
           end
         end
-        expect(import_status).to eventually_eq('finished').within(duration: import_max_duration, interval: 30)
+        expect(import_status).to eventually_eq('finished').within(max_duration: import_max_duration, sleep_interval: 30)
         @import_time = Time.now - start
 
         aggregate_failures do

qa/qa/specs/features/ee/browser_ui/fulfillment/purchase/upgrade_group_spec.rb

@@ -71,7 +71,7 @@ module QA
           Gitlab::Page::Group::Settings::Billing.perform do |billing|
             expect do
               billing.billing_plan_header
-            end.to eventually_include("#{group.name} is currently using the Ultimate SaaS Plan").within(duration: 120, attempts: 60, reload_page: page)
+            end.to eventually_include("#{group.name} is currently using the Ultimate SaaS Plan").within(max_duration: 120, max_attempts: 60, reload_page: page)
           end
         end
       end

qa/spec/support/matchers/eventually_matcher.rb

@@ -9,7 +9,7 @@
 # expect { Something.that.takes.time.to_appear }.not_to eventually_eq(expected_result)
 #
 # With duration and attempts override
-# expect { Something.that.takes.time.to_appear }.to eventually_eq(expected_result).within(duration: 10, attempts: 5)
+# expect { Something.that.takes.time.to_appear }.to eventually_eq(expected_result).within(max_duration: 10, max_attempts: 5)
 
 module Matchers
   %w[
@@ -21,11 +21,9 @@ module Matchers
     be_empty
   ].each do |op|
     RSpec::Matchers.define(:"eventually_#{op}") do |*expected|
-      chain(:within) do |options = {}|
-        @duration = options[:duration]
-        @attempts = options[:attempts]
-        @interval = options[:interval]
-        @reload_page = options[:reload_page]
+      chain(:within) do |kwargs = {}|
+        @retry_args = kwargs
+        @retry_args[:sleep_interval] = 0.5 unless @retry_args[:sleep_interval]
       end
 
       def supports_block_expectations?
@@ -57,12 +55,7 @@ module Matchers
         attempt = 0
 
         QA::Runtime::Logger.debug("Running eventually matcher with '#{operator_msg}' operator")
-        QA::Support::Retrier.retry_until(
-          max_attempts: @attempts,
-          max_duration: @duration,
-          sleep_interval: @interval || 0.5,
-          reload_page: @reload_page
-        ) do
+        QA::Support::Retrier.retry_until(**@retry_args) do
           QA::Runtime::Logger.debug("evaluating expectation, attempt: #{attempt += 1}")
 
           public_send(expectation_name, actual)