test(functional): clarify MR fixture factory name

nejch · JohnVillalovos · commit d8fd1a83b588 · 2023-02-06T07:40:10.000-08:00
diff --git a/tests/functional/api/test_merge_requests.py b/tests/functional/api/test_merge_requests.py
@@ -33,12 +33,11 @@ def test_merge_requests(project):
 
 
 def test_merge_requests_get(project, merge_request):
-    new_mr = merge_request(source_branch="test_get")
-    mr_iid = new_mr.iid
-    mr = project.mergerequests.get(mr_iid)
-    assert mr.iid == mr_iid
-    mr = project.mergerequests.get(str(mr_iid))
-    assert mr.iid == mr_iid
+    mr = project.mergerequests.get(merge_request.iid)
+    assert mr.iid == merge_request.iid
+
+    mr = project.mergerequests.get(str(merge_request.iid))
+    assert mr.iid == merge_request.iid
 
 
 @pytest.mark.gitlab_premium
@@ -54,10 +53,8 @@ def test_merge_requests_list_approver_ids(project):
 
 
 def test_merge_requests_get_lazy(project, merge_request):
-    new_mr = merge_request(source_branch="test_get")
-    mr_iid = new_mr.iid
-    mr = project.mergerequests.get(mr_iid, lazy=True)
-    assert mr.iid == mr_iid
+    mr = project.mergerequests.get(merge_request.iid, lazy=True)
+    assert mr.iid == merge_request.iid
 
 
 def test_merge_request_discussion(project):
@@ -181,26 +178,26 @@ def test_merge_request_reset_approvals(gitlab_url, project, wait_for_sidekiq):
     assert mr.reset_approvals()
 
 
-def test_cancel_merge_when_pipeline_succeeds(project, merge_request, wait_for_sidekiq):
-    mr = merge_request(source_branch="test_merge_request_merge", create_pipeline=True)
+def test_cancel_merge_when_pipeline_succeeds(
+    project, merge_request_with_pipeline, wait_for_sidekiq
+):
     wait_for_sidekiq(timeout=60)
     # Set to merge when the pipeline succeeds, which should never happen
-    mr.merge(merge_when_pipeline_succeeds=True)
+    merge_request_with_pipeline.merge(merge_when_pipeline_succeeds=True)
     wait_for_sidekiq(timeout=60)
 
-    mr = project.mergerequests.get(mr.iid)
+    mr = project.mergerequests.get(merge_request_with_pipeline.iid)
     assert mr.merged_at is None
     assert mr.merge_when_pipeline_succeeds is True
     cancel = mr.cancel_merge_when_pipeline_succeeds()
     assert cancel == {"status": "success"}
 
 
 def test_merge_request_merge(project, merge_request, wait_for_sidekiq):
-    mr = merge_request(source_branch="test_merge_request_merge")
-    mr.merge()
+    merge_request.merge()
     wait_for_sidekiq(timeout=60)
 
-    mr = project.mergerequests.get(mr.iid)
+    mr = project.mergerequests.get(merge_request.iid)
     assert mr.merged_at is not None
     assert mr.merge_when_pipeline_succeeds is False
     with pytest.raises(gitlab.GitlabMRClosedError):
@@ -215,28 +212,26 @@ def test_merge_request_should_remove_source_branch(
     https://github.com/python-gitlab/python-gitlab/issues/1120 is fixed.
     Bug reported that they could not use 'should_remove_source_branch' in
     mr.merge() call"""
-
-    source_branch = "remove_source_branch"
-    mr = merge_request(source_branch=source_branch)
-
-    mr.merge(should_remove_source_branch=True)
-
+    merge_request.merge(should_remove_source_branch=True)
     wait_for_sidekiq(timeout=60)
 
     # Wait until it is merged
-    mr_iid = mr.iid
+    mr = None
+    mr_iid = merge_request.iid
     for _ in range(60):
         mr = project.mergerequests.get(mr_iid)
         if mr.merged_at is not None:
             break
         time.sleep(0.5)
+
+    assert mr is not None
     assert mr.merged_at is not None
     time.sleep(0.5)
     wait_for_sidekiq(timeout=60)
 
     # Ensure we can NOT get the MR branch
     with pytest.raises(gitlab.exceptions.GitlabGetError):
-        result = project.branches.get(source_branch)
+        result = project.branches.get(merge_request.source_branch)
         # Help to debug in case the expected exception doesn't happen.
         import pprint
 
@@ -253,51 +248,44 @@ def test_merge_request_large_commit_message(
     Bug reported that very long 'merge_commit_message' in mr.merge() would
     cause an error: 414 Request too large
     """
-
-    source_branch = "large_commit_message"
-    mr = merge_request(source_branch=source_branch)
-
     merge_commit_message = "large_message\r\n" * 1_000
     assert len(merge_commit_message) > 10_000
 
-    mr.merge(
+    merge_request.merge(
         merge_commit_message=merge_commit_message, should_remove_source_branch=False
     )
 
     wait_for_sidekiq(timeout=60)
 
     # Wait until it is merged
-    mr_iid = mr.iid
+    mr = None
+    mr_iid = merge_request.iid
     for _ in range(60):
         mr = project.mergerequests.get(mr_iid)
         if mr.merged_at is not None:
             break
         time.sleep(0.5)
+
+    assert mr is not None
     assert mr.merged_at is not None
     time.sleep(0.5)
 
     # Ensure we can get the MR branch
-    project.branches.get(source_branch)
+    project.branches.get(merge_request.source_branch)
 
 
 def test_merge_request_merge_ref(merge_request) -> None:
-    source_branch = "merge_ref_test"
-    mr = merge_request(source_branch=source_branch)
-
-    response = mr.merge_ref()
+    response = merge_request.merge_ref()
     assert response and "commit_id" in response
 
 
 def test_merge_request_merge_ref_should_fail(
     project, merge_request, wait_for_sidekiq
 ) -> None:
-    source_branch = "merge_ref_test2"
-    mr = merge_request(source_branch=source_branch)
-
     # Create conflict
     project.files.create(
         {
-            "file_path": f"README.{source_branch}",
+            "file_path": f"README.{merge_request.source_branch}",
             "branch": project.default_branch,
             "content": "Different initial content",
             "commit_message": "Another commit in main branch",
@@ -307,5 +295,5 @@ def test_merge_request_merge_ref_should_fail(
 
     # Check for non-existing merge_ref for MR with conflicts
     with pytest.raises(gitlab.exceptions.GitlabGetError):
-        response = mr.merge_ref()
+        response = merge_request.merge_ref()
         assert "commit_id" not in response
diff --git a/tests/functional/cli/test_cli_repository.py b/tests/functional/cli/test_cli_repository.py
@@ -67,17 +67,20 @@ def test_list_merge_request_commits(gitlab_cli, merge_request, project):
 def test_commit_merge_requests(gitlab_cli, project, merge_request, wait_for_sidekiq):
     """This tests the `project-commit merge-requests` command and also tests
     that we can print the result using the `json` formatter"""
-    # create and then merge a merge-request
-    mr = merge_request(source_branch="test_commit_merge_requests")
-    merge_result = mr.merge(should_remove_source_branch=True)
+    # Merge the MR first
+    merge_result = merge_request.merge(should_remove_source_branch=True)
     wait_for_sidekiq(timeout=60)
+
     # Wait until it is merged
-    mr_iid = mr.iid
+    mr = None
+    mr_iid = merge_request.iid
     for _ in range(60):
         mr = project.mergerequests.get(mr_iid)
         if mr.merged_at is not None:
             break
         time.sleep(0.5)
+
+    assert mr is not None
     assert mr.merged_at is not None
     time.sleep(0.5)
     wait_for_sidekiq(timeout=60)
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
@@ -376,8 +376,8 @@ def project(gl):
 
 
 @pytest.fixture(scope="function")
-def merge_request(project, wait_for_sidekiq):
-    """Fixture used to create a merge_request.
+def make_merge_request(project, wait_for_sidekiq):
+    """Fixture factory used to create a merge_request.
 
     It will create a branch, add a commit to the branch, and then create a
     merge request against project.default_branch. The MR will be returned.
@@ -391,7 +391,7 @@ def merge_request(project, wait_for_sidekiq):
 
     to_delete = []
 
-    def _merge_request(*, source_branch: str, create_pipeline: bool = False):
+    def _make_merge_request(*, source_branch: str, create_pipeline: bool = False):
         # Wait for processes to be done before we start...
         # NOTE(jlvillal): Sometimes the CI would give a "500 Internal Server
         # Error". Hoping that waiting until all other processes are done will
@@ -450,12 +450,24 @@ def _merge_request(*, source_branch: str, create_pipeline: bool = False):
         to_delete.extend([mr, mr_branch])
         return mr
 
-    yield _merge_request
+    yield _make_merge_request
 
     for object in to_delete:
         helpers.safe_delete(object)
 
 
+@pytest.fixture(scope="function")
+def merge_request(make_merge_request, project):
+    _id = uuid.uuid4().hex
+    return make_merge_request(source_branch=f"branch-{_id}")
+
+
+@pytest.fixture(scope="function")
+def merge_request_with_pipeline(make_merge_request, project):
+    _id = uuid.uuid4().hex
+    return make_merge_request(source_branch=f"branch-{_id}", create_pipeline=True)
+
+
 @pytest.fixture(scope="module")
 def project_file(project):
     """File fixture for tests requiring a project with files and branches."""
