Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid mutating _filter dict for subsequent calls #229

Merged
merged 10 commits into from
Nov 19, 2024
15 changes: 10 additions & 5 deletions sync2jira/upstream_issue.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import logging
from urllib.parse import urlencode
from copy import deepcopy

import requests
from github import Github
Expand Down Expand Up @@ -247,13 +248,17 @@ def github_issues(upstream, config):
.get('github', {})\
.get(upstream, {})

if 'labels' in _filter:
# We have to flatten the labels list to a comma-separated string
_filter['labels'] = ','.join(_filter['labels'])

url = 'https://api.github.com/repos/%s/issues' % upstream
if _filter:
url += '?' + urlencode(_filter)
labels = _filter.get('labels')
if isinstance(labels, list):
# We have to flatten the labels list to a comma-separated string,
# so make a copy to avoid mutating the config object
url_filter = deepcopy(_filter)
url_filter['labels'] = ','.join(labels)
else:
url_filter = _filter # Use the existing filter, unmodified
url += '?' + urlencode(url_filter)

issues = get_all_github_data(url, headers)

Expand Down
15 changes: 10 additions & 5 deletions sync2jira/upstream_pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
# Authors: Ralph Bean <[email protected]>

import logging
from copy import deepcopy

try:
from urllib.parse import urlencode # py3
Expand Down Expand Up @@ -142,14 +143,18 @@ def github_prs(upstream, config):
.get('github', {}) \
.get(upstream, {})

if 'labels' in _filter:
# We have to flatten the labels list to a comma-separated string
_filter['labels'] = ','.join(_filter['labels'])

# Build our URL
url = 'https://api.github.com/repos/%s/pulls' % upstream
if _filter:
url += '?' + urlencode(_filter)
labels = _filter.get('labels')
if isinstance(labels, list):
# We have to flatten the labels list to a comma-separated string,
# so make a copy to avoid mutating the config object
url_filter = deepcopy(_filter)
url_filter['labels'] = ','.join(labels)
else:
url_filter = _filter # Use the existing filter, unmodified
url += '?' + urlencode(url_filter)

# Get our issues using helper functions
prs = u_issue.get_all_github_data(url, headers)
Expand Down
55 changes: 55 additions & 0 deletions tests/test_upstream_issue.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest
import unittest.mock as mock
from unittest.mock import MagicMock
from copy import deepcopy

import sync2jira.upstream_issue as u

Expand Down Expand Up @@ -396,6 +397,60 @@ def test_github_issues_no_token(self,
self.mock_github_repo.get_issue.assert_not_called()
self.mock_github_issue.get_comments.assert_not_called()

@mock.patch('sync2jira.intermediary.Issue.from_github')
@mock.patch(PATH + 'Github')
@mock.patch(PATH + 'get_all_github_data')
def test_filter_multiple_labels(self,
mock_get_all_github_data,
mock_github,
mock_issue_from_github):
"""
This function tests 'github_issues' function with a filter including multiple labels
"""
# Set up return values
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'].extend(['another_tag', 'and_another'])
mock_github.return_value = self.mock_github_client
mock_issue_from_github.return_value = 'Successful Call!'
# We mutate the issue object so we need to pass a copy here
mock_get_all_github_data.return_value = [deepcopy(self.mock_github_issue_raw)]

# Call the function
list(u.github_issues(
upstream='org/repo',
config=self.mock_config
))

# Assert that the labels filter is correct
self.assertIn(
'labels=custom_tag%2Canother_tag%2Cand_another',
mock_get_all_github_data.call_args[0][0]
)
# Assert the config value was not mutated
self.assertEqual(
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'],
['custom_tag','another_tag','and_another']
)

# Restore the return value to the original object
mock_get_all_github_data.return_value = [deepcopy(self.mock_github_issue_raw)]

# Call the function again to ensure consistency for subsequent calls
list(u.github_issues(
upstream='org/repo',
config=self.mock_config
))

# Assert that the labels filter is correct
self.assertIn(
'labels=custom_tag%2Canother_tag%2Cand_another',
mock_get_all_github_data.call_args[0][0]
)
# Assert the config value was not mutated
self.assertEqual(
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'],
['custom_tag','another_tag','and_another']
)

@mock.patch(PATH + 'Github')
@mock.patch('sync2jira.intermediary.Issue.from_github')
def test_handle_github_message_not_in_mapped(self,
Expand Down
55 changes: 55 additions & 0 deletions tests/test_upstream_pr.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import unittest
import unittest.mock as mock
from unittest.mock import MagicMock
from copy import deepcopy

import sync2jira.upstream_pr as u

Expand Down Expand Up @@ -201,3 +202,57 @@ def test_github_issues(self,
self.mock_github_repo.get_pull.assert_called_with(number='1234')
self.mock_github_pr.get_issue_comments.assert_any_call()
self.assertEqual(response[0], 'Successful Call!')

@mock.patch('sync2jira.intermediary.PR.from_github')
@mock.patch(PATH + 'Github')
@mock.patch(PATH + 'u_issue.get_all_github_data')
def test_filter_multiple_labels(self,
mock_get_all_github_data,
mock_github,
mock_issue_from_github):
"""
This function tests 'github_issues' function with a filter including multiple labels
"""
# Set up return values
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'].extend(['another_tag', 'and_another'])
mock_github.return_value = self.mock_github_client
mock_issue_from_github.return_value = 'Successful Call!'
# We mutate the issue object so we need to pass a copy here
mock_get_all_github_data.return_value = [deepcopy(self.mock_github_issue_raw)]

# Call the function
list(u.github_prs(
upstream='org/repo',
config=self.mock_config
))

# Assert that the labels filter is correct
self.assertIn(
'labels=custom_tag%2Canother_tag%2Cand_another',
mock_get_all_github_data.call_args[0][0]
)
# Assert the config value was not mutated
self.assertEqual(
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'],
['custom_tag','another_tag','and_another']
)

# Restore the return value to the original object
mock_get_all_github_data.return_value = [deepcopy(self.mock_github_issue_raw)]

# Call the function again to ensure consistency for subsequent calls
list(u.github_prs(
upstream='org/repo',
config=self.mock_config
))

# Assert that the labels filter is correct
self.assertIn(
'labels=custom_tag%2Canother_tag%2Cand_another',
mock_get_all_github_data.call_args[0][0]
)
# Assert the config value was not mutated
self.assertEqual(
self.mock_config['sync2jira']['filters']['github']['org/repo']['labels'],
['custom_tag','another_tag','and_another']
)