Improve `wait_for_push` reliability
@godfat suggested making wait_for_push
the default option for ProjectPush.fabricate!
.
That would be useful as it's easy to forget to call Page::Project::Show.perform(&:wait_for_push)
, which can cause problems if a test expects a page to display the results of the push and the push isn't complete yet.
It would also be good to improve how the framework detects when the wait should end. Currently it just sleeps for 5 seconds:
def wait_for_push
sleep 5
refresh
end
Expand for details including diffs
The following discussion from !22633 should be addressed:
-
@godfat started a discussion: I feel this is very easy to miss. What do you think if we make this a default option for
ProjectPush.fabricate!
?diff --git a/qa/qa/factory/repository/push.rb b/qa/qa/factory/repository/push.rb index 4b30c051a5c..829e3722699 100644 --- a/qa/qa/factory/repository/push.rb +++ b/qa/qa/factory/repository/push.rb @@ -8,7 +8,7 @@ module QA :branch_name, :new_branch, :output, :repository_http_uri, :repository_ssh_uri, :ssh_key, :user - attr_writer :remote_branch + attr_writer :remote_branch, :no_wait_for_push def initialize @file_name = 'file.txt' @@ -86,6 +86,10 @@ module QA repository.delete_ssh_key end + + unless @no_wait_for_push + Page::Project::Show.perform(&:wait_for_push) + end end end end diff --git a/qa/qa/factory/resource/branch.rb b/qa/qa/factory/resource/branch.rb index b05d1e252ec..56784011ba8 100644 --- a/qa/qa/factory/resource/branch.rb +++ b/qa/qa/factory/resource/branch.rb @@ -25,6 +25,7 @@ module QA resource.project = project resource.file_name = 'kick-off.txt' resource.commit_message = 'First commit' + resource.no_wait_for_push = true end branch = Factory::Repository::ProjectPush.fabricate! do |resource| @@ -34,6 +35,7 @@ module QA resource.branch_name = 'master' resource.new_branch = false resource.remote_branch = @branch_name + resource.no_wait_for_push = true end Page::Project::Show.perform do |page| diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/ee_code_owners_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/ee_code_owners_spec.rb index a75cd713628..a0f0dfadd36 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/ee_code_owners_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/ee_code_owners_spec.rb @@ -55,9 +55,6 @@ module QA push.files = files push.commit_message = 'Add CODEOWNERS and test files' end - Page::Project::Show.perform do |project_page| - project_page.wait_for_push - end # Check the files and code owners Page::Project::Show.perform do |project_page| diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_spec.rb index 2f63a07e0c3..42387e6340b 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/push_over_http_spec.rb @@ -13,8 +13,6 @@ module QA push.commit_message = 'Add README.md' end - Page::Project::Show.act { wait_for_push } - expect(page).to have_content('README.md') expect(page).to have_content('This is a test project') end diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb index 36068ffba69..7c6a7c38889 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/use_ssh_key_spec.rb @@ -23,8 +23,6 @@ module QA push.commit_message = 'Add README.md' end - Page::Project::Show.act { wait_for_push } - expect(page).to have_content('README.md') expect(page).to have_content('Test Use SSH Key') diff --git a/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb b/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb index d66bcce879b..66457b8f82f 100644 --- a/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb +++ b/qa/qa/specs/features/browser_ui/4_verify/pipeline/create_and_process_pipeline_spec.rb @@ -60,8 +60,6 @@ module QA EOF end - Page::Project::Show.act { wait_for_push } - expect(page).to have_content('Add .gitlab-ci.yml') Page::Project::Menu.act { click_ci_cd_pipelines } diff --git a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb index caf014c89ea..14afa43621d 100644 --- a/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb +++ b/qa/qa/specs/features/browser_ui/6_release/deploy_key/clone_using_deploy_key_spec.rb @@ -89,7 +89,6 @@ module QA sha1sum = Digest::SHA1.hexdigest(gitlab_ci) - Page::Project::Show.act { wait_for_push } Page::Project::Menu.act { click_ci_cd_pipelines } Page::Project::Pipeline::Index.act { go_to_latest_pipeline } Page::Project::Pipeline::Show.act { go_to_first_job } diff --git a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb index 40cae0793dd..de64b86192d 100644 --- a/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb +++ b/qa/qa/specs/features/browser_ui/7_configure/auto_devops/create_project_with_auto_devops_spec.rb @@ -37,8 +37,6 @@ module QA push.commit_message = 'Create Auto DevOps compatible rack application' end - Page::Project::Show.act { wait_for_push } - # Create and connect K8s cluster @cluster = Service::KubernetesCluster.new(rbac: rbac).create! kubernetes_cluster = Factory::Resource::KubernetesCluster.fabricate! do |cluster|
My concern is wow, that's how we implemented it??
def wait_for_push sleep 5 refresh end
But that's probably another issue...
What do you think?
Edited by Mark Lapierre