Skip to content

Commit

Permalink
refactor: removed print statement and updated tests
Browse files Browse the repository at this point in the history
  • Loading branch information
BilalQamar95 committed Jan 24, 2025
1 parent 4fbca3f commit aa7864e
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 19 deletions.
1 change: 0 additions & 1 deletion edx_repo_tools/pull_request_creator/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,6 @@ def _create_new_pull_request(self):
)
if self.output_pr_url_for_github_action:
pr_url = f"https://github.com/{self.repository.full_name}/pull/{pr.number}"
print(f"generated_pr={pr_url} >> $GITHUB_OUTPUT")

# need to append to the special file that GitHub actions exposes
# as using print here won't set the output, it simply logs the message
Expand Down
36 changes: 18 additions & 18 deletions tests/test_pull_request_creator.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# pylint: disable=missing-module-docstring,missing-class-docstring

from os import path
import os
from unittest import TestCase
from unittest.mock import MagicMock, Mock, mock_open, patch

Expand Down Expand Up @@ -199,9 +199,9 @@ def test_changes(self, delete_branch_mock, get_user_mock, create_branch_mock, cr
create_pr_mock.diff_url = "/"
create_pr_mock.repository.name = 'repo-health-data'

basepath = path.dirname(__file__)
basepath = os.path.dirname(__file__)

filepath = path.abspath(path.join(basepath, "pull_request_creator_test_data", "diff.txt"))
filepath = os.path.abspath(os.path.join(basepath, "pull_request_creator_test_data", "diff.txt"))
with open(filepath, "r") as f:
content = f.read().encode('utf-8')
with patch('requests.get') as mock_request:
Expand All @@ -224,8 +224,9 @@ def test_changes(self, delete_branch_mock, get_user_mock, create_branch_mock, cr
@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.update_list_of_files', return_value=None)
@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.create_pull_request')
@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.create_branch', return_value=None)
@patch('builtins.print')
def test_outputs_url_on_success(self, print_mock, create_branch_mock, create_pr_mock,
@patch.dict(os.environ, {"GITHUB_OUTPUT": "/tmp/fake_github_output"})
@patch("builtins.open", new_callable=mock_open)
def test_outputs_url_on_success(self, mock_file, create_branch_mock, create_pr_mock,
update_files_mock, branch_exists_mock, *args):
"""
Ensure that a successful run outputs the URL consumable by github actions
Expand All @@ -241,12 +242,12 @@ def test_outputs_url_on_success(self, print_mock, create_branch_mock, create_pr_
assert update_files_mock.called
self.assertEqual(update_files_mock.call_count, 1)
assert create_pr_mock.called
assert print_mock.call_count == 1
found_matching_call = False
for call in print_mock.call_args_list:
if '$GITHUB_OUTPUT' in call.args[0]:
found_matching_call = True
assert found_matching_call

mock_file.assert_called_once_with("/tmp/fake_github_output", "a", encoding="utf-8")
handle = mock_file()
all_writes = "".join(call_args[0][0] for call_args in handle.write.call_args_list)

assert "generated_pr=" in all_writes, f"Expected 'generated_pr=' in writes, but got: {all_writes}"

@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.close_existing_pull_requests',
return_value=[])
Expand Down Expand Up @@ -295,7 +296,6 @@ def test_branch_exists(self, delete_branch_mock, get_user_mock, create_branch_mo
@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.create_pull_request')
@patch('edx_repo_tools.pull_request_creator.PullRequestCreator.github_helper.create_branch', return_value=None)
@patch('edx_repo_tools.pull_request_creator.GitHubHelper.delete_branch', return_value=None)
@patch('builtins.print')
def test_branch_deletion(self, create_branch_mock, create_pr_mock,
update_files_mock, branch_exists_mock, delete_branch_mock, *args):
"""
Expand All @@ -314,8 +314,8 @@ def test_branch_deletion(self, create_branch_mock, create_pr_mock,
assert create_pr_mock.called

def test_compare_upgrade_difference_with_major_changes(self):
basepath = path.dirname(__file__)
filepath = path.abspath(path.join(basepath, "pull_request_creator_test_data", "diff.txt"))
basepath = os.path.dirname(__file__)
filepath = os.path.abspath(os.path.join(basepath, "pull_request_creator_test_data", "diff.txt"))
with open(filepath, "r") as f:
valid, suspicious = GitHubHelper().compare_pr_differnce(f.read())
assert sorted(
Expand All @@ -327,8 +327,8 @@ def test_compare_upgrade_difference_with_major_changes(self):
) == [g['name'] for g in suspicious]

def test_compare_upgrade_difference_with_minor_changes(self):
basepath = path.dirname(__file__)
filepath = path.abspath(path.join(basepath, "pull_request_creator_test_data", "minor_diff.txt"))
basepath = os.path.dirname(__file__)
filepath = os.path.abspath(os.path.join(basepath, "pull_request_creator_test_data", "minor_diff.txt"))
with open(filepath, "r") as f:
valid, suspicious = GitHubHelper().compare_pr_differnce(f.read())
assert sorted(
Expand Down Expand Up @@ -414,9 +414,9 @@ def test_changes_with_minor_versions_and_variable(
create_pr_mock.diff_url = "/"
create_pr_mock.repository.name = 'xblock-lti-consumer'

basepath = path.dirname(__file__)
basepath = os.path.dirname(__file__)

filepath = path.abspath(path.join(basepath, "pull_request_creator_test_data", "minor_diff.txt"))
filepath = os.path.abspath(os.path.join(basepath, "pull_request_creator_test_data", "minor_diff.txt"))
with open(filepath, "r") as f:
content = f.read().encode('utf-8')
with patch('requests.get') as mock_request:
Expand Down

0 comments on commit aa7864e

Please sign in to comment.