chore: simplify wait_for_sidekiq usage

JohnVillalovos · nejch · commit 196538ba3e23 · 2022-10-05T13:37:14.000+02:00
Simplify usage of `wait_for_sidekiq` by putting the assert if it timed
out inside the function rather than after calling it.
diff --git a/tests/functional/api/test_lazy_objects.py b/tests/functional/api/test_lazy_objects.py
@@ -29,8 +29,7 @@ def test_save_after_lazy_get_with_path(project, lazy_project):
 
 def test_delete_after_lazy_get_with_path(gl, group, wait_for_sidekiq):
     project = gl.projects.create({"name": "lazy_project", "namespace_id": group.id})
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
     lazy_project = gl.projects.get(project.path_with_namespace, lazy=True)
     lazy_project.delete()
 
diff --git a/tests/functional/api/test_merge_requests.py b/tests/functional/api/test_merge_requests.py
@@ -150,8 +150,7 @@ def test_merge_request_should_remove_source_branch(
 
     mr.merge(should_remove_source_branch=True)
 
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
 
     # Wait until it is merged
     mr_iid = mr.iid
@@ -162,8 +161,7 @@ def test_merge_request_should_remove_source_branch(
         time.sleep(0.5)
     assert mr.merged_at is not None
     time.sleep(0.5)
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
 
     # Ensure we can NOT get the MR branch
     with pytest.raises(gitlab.exceptions.GitlabGetError):
@@ -195,8 +193,7 @@ def test_merge_request_large_commit_message(
         merge_commit_message=merge_commit_message, should_remove_source_branch=False
     )
 
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
 
     # Wait until it is merged
     mr_iid = mr.iid
@@ -235,8 +232,7 @@ def test_merge_request_merge_ref_should_fail(
             "commit_message": "Another commit in main branch",
         }
     )
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
 
     # Check for non-existing merge_ref for MR with conflicts
     with pytest.raises(gitlab.exceptions.GitlabGetError):
diff --git a/tests/functional/api/test_users.py b/tests/functional/api/test_users.py
@@ -70,8 +70,7 @@ def test_delete_user(gl, wait_for_sidekiq):
     )
 
     new_user.delete()
-    result = wait_for_sidekiq(timeout=60)
-    assert result is True, "sidekiq process should have terminated but did not"
+    wait_for_sidekiq(timeout=60)
 
     assert new_user.id not in [user.id for user in gl.users.list()]
 
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
@@ -224,7 +224,7 @@ def wait_for_sidekiq(gl):
     Use this with asserts for slow tasks (group/project/user creation/deletion).
     """
 
-    def _wait(timeout=30, step=0.5):
+    def _wait(timeout: int = 30, step: float = 0.5, allow_fail: bool = False) -> bool:
         for count in range(timeout):
             time.sleep(step)
             busy = False
@@ -235,6 +235,7 @@ def _wait(timeout=30, step=0.5):
             if not busy:
                 return True
             logging.info(f"sidekiq busy {count} of {timeout}")
+        assert allow_fail, "sidekiq process should have terminated but did not."
         return False
 
     return _wait

Original file line number	Diff line number	Diff line change
`@@ -70,8 +70,7 @@ def test_delete_user(gl, wait_for_sidekiq):`
`70`	`70`	`)`
`71`	`71`
`72`	`72`	`new_user.delete()`
`73`		`- result = wait_for_sidekiq(timeout=60)`
`74`		`- assert result is True, "sidekiq process should have terminated but did not"`
	`73`	`+ wait_for_sidekiq(timeout=60)`
`75`	`74`
`76`	`75`	`assert new_user.id not in [user.id for user in gl.users.list()]`
`77`	`76`