From 7d66115573c6c029ce6aa00e244f8bdfbb907e33 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sun, 23 May 2021 10:44:27 -0700 Subject: chore: add a functional test for issue #1120 Going to switch to putting parameters from in the query string to having them in the 'data' body section. Add a functional test to make sure that we don't break anything. https://github.com/python-gitlab/python-gitlab/issues/1120 --- tools/functional/api/test_merge_requests.py | 59 +++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) (limited to 'tools/functional/api') diff --git a/tools/functional/api/test_merge_requests.py b/tools/functional/api/test_merge_requests.py index c5de5eb..80a650c 100644 --- a/tools/functional/api/test_merge_requests.py +++ b/tools/functional/api/test_merge_requests.py @@ -1,6 +1,9 @@ +import time + import pytest import gitlab +import gitlab.v4.objects def test_merge_requests(project): @@ -95,3 +98,59 @@ def test_merge_request_merge(project): with pytest.raises(gitlab.GitlabMRClosedError): # Two merge attempts should raise GitlabMRClosedError mr.merge() + + +def test_merge_request_should_remove_source_branch( + project: gitlab.v4.objects.Project, wait_for_sidekiq +): + """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1120 + is fixed""" + + source_branch = "remove_source_branch" + project.branches.create({"branch": source_branch, "ref": "master"}) + + # NOTE(jlvillal): Must create a commit in the new branch before we can + # create an MR that will work. + project.files.create( + { + "file_path": f"README.{source_branch}", + "branch": source_branch, + "content": "Initial content", + "commit_message": "New commit in new branch", + } + ) + + mr = project.mergerequests.create( + { + "source_branch": source_branch, + "target_branch": "master", + "title": "Should remove source branch", + "remove_source_branch": True, + } + ) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + mr_iid = mr.iid + for _ in range(60): + mr = project.mergerequests.get(mr_iid) + if mr.merge_status != "checking": + break + time.sleep(0.5) + assert mr.merge_status != "checking" + + # Ensure we can get the MR branch + project.branches.get(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" + + # Ensure we can NOT get the MR branch + with pytest.raises(gitlab.exceptions.GitlabGetError): + project.branches.get(source_branch) + + mr = project.mergerequests.get(mr.iid) + assert mr.merged_at is not None # Now is merged -- cgit v1.2.1 From cd5993c9d638c2a10879d7e3ac36db06df867e54 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sun, 23 May 2021 14:39:55 -0700 Subject: chore: add functional test mr.merge() with long commit message Functional test to show that https://github.com/python-gitlab/python-gitlab/issues/1452 is fixed. Added a functional test to ensure that we can use large commit message (10_000+ bytes) in mr.merge() Related to: #1452 --- tools/functional/api/test_merge_requests.py | 59 +++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) (limited to 'tools/functional/api') diff --git a/tools/functional/api/test_merge_requests.py b/tools/functional/api/test_merge_requests.py index 80a650c..abb1668 100644 --- a/tools/functional/api/test_merge_requests.py +++ b/tools/functional/api/test_merge_requests.py @@ -154,3 +154,62 @@ def test_merge_request_should_remove_source_branch( mr = project.mergerequests.get(mr.iid) assert mr.merged_at is not None # Now is merged + + +def test_merge_request_large_commit_message( + project: gitlab.v4.objects.Project, wait_for_sidekiq +): + """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1452 + is fixed""" + source_branch = "large_commit_message" + project.branches.create({"branch": source_branch, "ref": "master"}) + + # NOTE(jlvillal): Must create a commit in the new branch before we can + # create an MR that will work. + project.files.create( + { + "file_path": f"README.{source_branch}", + "branch": source_branch, + "content": "Initial content", + "commit_message": "New commit in new branch", + } + ) + + mr = project.mergerequests.create( + { + "source_branch": source_branch, + "target_branch": "master", + "title": "Large Commit Message", + "remove_source_branch": True, + } + ) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + mr_iid = mr.iid + for _ in range(60): + mr = project.mergerequests.get(mr_iid) + if mr.merge_status != "checking": + break + time.sleep(0.5) + assert mr.merge_status != "checking" + + # Ensure we can get the MR branch + project.branches.get(source_branch) + + commit_message = "large_message\r\n" * 1_000 + assert len(commit_message) > 10_000 + assert mr.merged_at is None # Not yet merged + + mr.merge(merge_commit_message=commit_message, should_remove_source_branch=True) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + # Ensure we can NOT get the MR branch + with pytest.raises(gitlab.exceptions.GitlabGetError): + project.branches.get(source_branch) + + mr = project.mergerequests.get(mr.iid) + assert mr.merged_at is not None # Now is merged -- cgit v1.2.1 From df9b5f9226f704a603a7e49c78bc4543b412f898 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Sun, 23 May 2021 17:38:21 -0700 Subject: chore: simplify functional tests Add a helper function to have less code duplication in the functional testing. --- tools/functional/api/test_merge_requests.py | 116 ++++++++++++++-------------- 1 file changed, 57 insertions(+), 59 deletions(-) (limited to 'tools/functional/api') diff --git a/tools/functional/api/test_merge_requests.py b/tools/functional/api/test_merge_requests.py index abb1668..d6b1e95 100644 --- a/tools/functional/api/test_merge_requests.py +++ b/tools/functional/api/test_merge_requests.py @@ -100,13 +100,21 @@ def test_merge_request_merge(project): mr.merge() -def test_merge_request_should_remove_source_branch( - project: gitlab.v4.objects.Project, wait_for_sidekiq +def merge_request_create_helper( + *, + project: gitlab.v4.objects.Project, + source_branch: str, + wait_for_sidekiq, + branch_will_be_deleted: bool, + **kwargs, ): - """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1120 - is fixed""" + # 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 help + # with that. + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" - source_branch = "remove_source_branch" project.branches.create({"branch": source_branch, "ref": "master"}) # NOTE(jlvillal): Must create a commit in the new branch before we can @@ -143,73 +151,63 @@ def test_merge_request_should_remove_source_branch( # Ensure we can get the MR branch project.branches.get(source_branch) - mr.merge(should_remove_source_branch=True) + mr.merge(**kwargs) result = wait_for_sidekiq(timeout=60) assert result is True, "sidekiq process should have terminated but did not" - # Ensure we can NOT get the MR branch - with pytest.raises(gitlab.exceptions.GitlabGetError): - project.branches.get(source_branch) + # Wait until it is merged + mr_iid = mr.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.merged_at is not None + time.sleep(0.5) - mr = project.mergerequests.get(mr.iid) - assert mr.merged_at is not None # Now is merged + if branch_will_be_deleted: + # Ensure we can NOT get the MR branch + with pytest.raises(gitlab.exceptions.GitlabGetError): + project.branches.get(source_branch) -def test_merge_request_large_commit_message( +def test_merge_request_should_remove_source_branch( project: gitlab.v4.objects.Project, wait_for_sidekiq ): - """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1452 - is fixed""" - source_branch = "large_commit_message" - project.branches.create({"branch": source_branch, "ref": "master"}) + """Test to ensure + 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""" - # NOTE(jlvillal): Must create a commit in the new branch before we can - # create an MR that will work. - project.files.create( - { - "file_path": f"README.{source_branch}", - "branch": source_branch, - "content": "Initial content", - "commit_message": "New commit in new branch", - } - ) + source_branch = "remove_source_branch" - mr = project.mergerequests.create( - { - "source_branch": source_branch, - "target_branch": "master", - "title": "Large Commit Message", - "remove_source_branch": True, - } + merge_request_create_helper( + project=project, + source_branch=source_branch, + wait_for_sidekiq=wait_for_sidekiq, + branch_will_be_deleted=True, + should_remove_source_branch=True, ) - result = wait_for_sidekiq(timeout=60) - assert result is True, "sidekiq process should have terminated but did not" - - mr_iid = mr.iid - for _ in range(60): - mr = project.mergerequests.get(mr_iid) - if mr.merge_status != "checking": - break - time.sleep(0.5) - assert mr.merge_status != "checking" - # Ensure we can get the MR branch - project.branches.get(source_branch) - - commit_message = "large_message\r\n" * 1_000 - assert len(commit_message) > 10_000 - assert mr.merged_at is None # Not yet merged - - mr.merge(merge_commit_message=commit_message, should_remove_source_branch=True) - - result = wait_for_sidekiq(timeout=60) - assert result is True, "sidekiq process should have terminated but did not" +def test_merge_request_large_commit_message( + project: gitlab.v4.objects.Project, wait_for_sidekiq +): + """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1452 + is fixed. + Bug reported that very long 'merge_commit_message' in mr.merge() would + cause an error: 414 Request too large + """ + source_branch = "large_commit_message" - # Ensure we can NOT get the MR branch - with pytest.raises(gitlab.exceptions.GitlabGetError): - project.branches.get(source_branch) + merge_commit_message = "large_message\r\n" * 1_000 + assert len(merge_commit_message) > 10_000 - mr = project.mergerequests.get(mr.iid) - assert mr.merged_at is not None # Now is merged + merge_request_create_helper( + project=project, + source_branch=source_branch, + wait_for_sidekiq=wait_for_sidekiq, + branch_will_be_deleted=False, + merge_commit_message=merge_commit_message, + ) -- cgit v1.2.1 From 8be2838a9ee3e2440d066e2c4b77cb9b55fc3da2 Mon Sep 17 00:00:00 2001 From: "John L. Villalovos" Date: Mon, 24 May 2021 15:30:08 -0700 Subject: chore: add a merge_request() pytest fixture and use it Added a pytest.fixture for merge_request(). Use this fixture in tools/functional/api/test_merge_requests.py --- tools/functional/api/test_merge_requests.py | 116 ++++++++-------------------- 1 file changed, 34 insertions(+), 82 deletions(-) (limited to 'tools/functional/api') diff --git a/tools/functional/api/test_merge_requests.py b/tools/functional/api/test_merge_requests.py index d6b1e95..e768234 100644 --- a/tools/functional/api/test_merge_requests.py +++ b/tools/functional/api/test_merge_requests.py @@ -100,58 +100,18 @@ def test_merge_request_merge(project): mr.merge() -def merge_request_create_helper( - *, - project: gitlab.v4.objects.Project, - source_branch: str, - wait_for_sidekiq, - branch_will_be_deleted: bool, - **kwargs, -): - # 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 help - # with that. - result = wait_for_sidekiq(timeout=60) - assert result is True, "sidekiq process should have terminated but did not" - - project.branches.create({"branch": source_branch, "ref": "master"}) - - # NOTE(jlvillal): Must create a commit in the new branch before we can - # create an MR that will work. - project.files.create( - { - "file_path": f"README.{source_branch}", - "branch": source_branch, - "content": "Initial content", - "commit_message": "New commit in new branch", - } - ) - - mr = project.mergerequests.create( - { - "source_branch": source_branch, - "target_branch": "master", - "title": "Should remove source branch", - "remove_source_branch": True, - } - ) - - result = wait_for_sidekiq(timeout=60) - assert result is True, "sidekiq process should have terminated but did not" - - mr_iid = mr.iid - for _ in range(60): - mr = project.mergerequests.get(mr_iid) - if mr.merge_status != "checking": - break - time.sleep(0.5) - assert mr.merge_status != "checking" +def test_merge_request_should_remove_source_branch( + project, merge_request, wait_for_sidekiq +) -> None: + """Test to ensure + 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""" - # Ensure we can get the MR branch - project.branches.get(source_branch) + source_branch = "remove_source_branch" + mr = merge_request(source_branch=source_branch) - mr.merge(**kwargs) + 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" @@ -166,48 +126,40 @@ def merge_request_create_helper( assert mr.merged_at is not None time.sleep(0.5) - if branch_will_be_deleted: - # Ensure we can NOT get the MR branch - with pytest.raises(gitlab.exceptions.GitlabGetError): - project.branches.get(source_branch) - - -def test_merge_request_should_remove_source_branch( - project: gitlab.v4.objects.Project, wait_for_sidekiq -): - """Test to ensure - 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" - - merge_request_create_helper( - project=project, - source_branch=source_branch, - wait_for_sidekiq=wait_for_sidekiq, - branch_will_be_deleted=True, - should_remove_source_branch=True, - ) + # Ensure we can NOT get the MR branch + with pytest.raises(gitlab.exceptions.GitlabGetError): + project.branches.get(source_branch) def test_merge_request_large_commit_message( - project: gitlab.v4.objects.Project, wait_for_sidekiq -): + project, merge_request, wait_for_sidekiq +) -> None: """Test to ensure https://github.com/python-gitlab/python-gitlab/issues/1452 is fixed. 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 - merge_request_create_helper( - project=project, - source_branch=source_branch, - wait_for_sidekiq=wait_for_sidekiq, - branch_will_be_deleted=False, - merge_commit_message=merge_commit_message, - ) + mr.merge(merge_commit_message=merge_commit_message) + + result = wait_for_sidekiq(timeout=60) + assert result is True, "sidekiq process should have terminated but did not" + + # Wait until it is merged + mr_iid = mr.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.merged_at is not None + time.sleep(0.5) + + # Ensure we can get the MR branch + project.branches.get(source_branch) -- cgit v1.2.1