Skip to content

Commit 2951f8e

Browse files
Ilanit Steinclaude
andcommitted
Improve storypoints error logging and add test coverage
Address code review feedback for PR release-engineering#452: - Make error log messages distinct between Single Select and Number field paths - Include the actual value that failed conversion in log messages for easier debugging - Add comprehensive test coverage for all error handling scenarios Test coverage includes: - Single Select field with invalid mapped value - Number field with invalid number value - Single Select field with missing name - Single Select field with unmapped value All tests pass with 82.58% coverage (exceeds 70% requirement). Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent 85975df commit 2951f8e

2 files changed

Lines changed: 182 additions & 2 deletions

File tree

sync2jira/upstream_issue.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,8 @@ def add_project_values(issue, upstream, headers, config):
317317
issue["storypoints"] = int(sp_number)
318318
except (ValueError, TypeError) as err:
319319
log.info(
320-
"Error while processing storypoints for issue %s/%s#%s: %s",
320+
"Error converting Single Select storypoints value '%s' to int for issue %s/%s#%s: %s",
321+
sp_number,
321322
orgname,
322323
reponame,
323324
issuenumber,
@@ -329,7 +330,8 @@ def add_project_values(issue, upstream, headers, config):
329330
issue["storypoints"] = int(item["number"])
330331
except (ValueError, TypeError, KeyError) as err:
331332
log.info(
332-
"Error while processing storypoints for issue %s/%s#%s: %s",
333+
"Error converting Number field storypoints value '%s' to int for issue %s/%s#%s: %s",
334+
item.get("number", "missing"),
333335
orgname,
334336
reponame,
335337
issuenumber,

tests/test_upstream_issue.py

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,184 @@ def test_add_project_values_early_exit(self, mock_requests_post):
768768
# Reset mock
769769
mock_requests_post.reset_mock()
770770

771+
@mock.patch(PATH + "requests.post")
772+
def test_add_project_values_storypoints_error_handling(self, mock_requests_post):
773+
"""
774+
Test 'add_project_values' error handling for storypoints conversion failures.
775+
Tests both Single Select (Size) and Number (Estimate) field error paths.
776+
"""
777+
# Set up base config
778+
upstream_config = {
779+
"issue_updates": ["github_project_fields"],
780+
"github_project_number": 1,
781+
}
782+
self.mock_config["sync2jira"]["map"]["github"]["org/repo"] = upstream_config
783+
784+
mock_issue = {
785+
"number": 1234,
786+
"storypoints": None,
787+
"priority": None,
788+
}
789+
790+
# Test case 1: Single Select field (Size) with invalid mapped value (not convertible to int)
791+
upstream_config["github_project_fields"] = {
792+
"storypoints": {
793+
"gh_field": "Size",
794+
"options": {"Small": "not_a_number"},
795+
}
796+
}
797+
mock_requests_post.return_value.json.return_value = {
798+
"data": {
799+
"repository": {
800+
"issue": {
801+
"projectItems": {
802+
"nodes": [
803+
{
804+
"project": {"title": "Project 1", "number": 1},
805+
"fieldValues": {
806+
"nodes": [
807+
{
808+
"fieldName": {"name": "Size"},
809+
"name": "Small",
810+
}
811+
]
812+
},
813+
}
814+
]
815+
}
816+
}
817+
}
818+
}
819+
}
820+
u.add_project_values(
821+
issue=mock_issue,
822+
upstream="org/repo",
823+
headers={},
824+
config=self.mock_config,
825+
)
826+
# Storypoints should not be set due to conversion error
827+
self.assertIsNone(mock_issue.get("storypoints"))
828+
mock_requests_post.reset_mock()
829+
830+
# Test case 2: Number field (Estimate) with invalid number value
831+
upstream_config["github_project_fields"] = {
832+
"storypoints": {"gh_field": "Estimate"}
833+
}
834+
mock_requests_post.return_value.json.return_value = {
835+
"data": {
836+
"repository": {
837+
"issue": {
838+
"projectItems": {
839+
"nodes": [
840+
{
841+
"project": {"title": "Project 1", "number": 1},
842+
"fieldValues": {
843+
"nodes": [
844+
{
845+
"fieldName": {"name": "Estimate"},
846+
"number": "invalid",
847+
}
848+
]
849+
},
850+
}
851+
]
852+
}
853+
}
854+
}
855+
}
856+
}
857+
mock_issue["storypoints"] = None
858+
u.add_project_values(
859+
issue=mock_issue,
860+
upstream="org/repo",
861+
headers={},
862+
config=self.mock_config,
863+
)
864+
# Storypoints should not be set due to conversion error
865+
self.assertIsNone(mock_issue.get("storypoints"))
866+
mock_requests_post.reset_mock()
867+
868+
# Test case 3: Single Select field (Size) with missing name
869+
upstream_config["github_project_fields"] = {
870+
"storypoints": {
871+
"gh_field": "Size",
872+
"options": {"Small": 5},
873+
}
874+
}
875+
mock_requests_post.return_value.json.return_value = {
876+
"data": {
877+
"repository": {
878+
"issue": {
879+
"projectItems": {
880+
"nodes": [
881+
{
882+
"project": {"title": "Project 1", "number": 1},
883+
"fieldValues": {
884+
"nodes": [
885+
{
886+
"fieldName": {"name": "Size"},
887+
# name is missing
888+
}
889+
]
890+
},
891+
}
892+
]
893+
}
894+
}
895+
}
896+
}
897+
}
898+
mock_issue["storypoints"] = None
899+
u.add_project_values(
900+
issue=mock_issue,
901+
upstream="org/repo",
902+
headers={},
903+
config=self.mock_config,
904+
)
905+
# Storypoints should not be set due to missing name
906+
self.assertIsNone(mock_issue.get("storypoints"))
907+
mock_requests_post.reset_mock()
908+
909+
# Test case 4: Single Select field (Size) with value not in options mapping
910+
upstream_config["github_project_fields"] = {
911+
"storypoints": {
912+
"gh_field": "Size",
913+
"options": {"Small": 5, "Medium": 8},
914+
}
915+
}
916+
mock_requests_post.return_value.json.return_value = {
917+
"data": {
918+
"repository": {
919+
"issue": {
920+
"projectItems": {
921+
"nodes": [
922+
{
923+
"project": {"title": "Project 1", "number": 1},
924+
"fieldValues": {
925+
"nodes": [
926+
{
927+
"fieldName": {"name": "Size"},
928+
"name": "Large", # Not in options
929+
}
930+
]
931+
},
932+
}
933+
]
934+
}
935+
}
936+
}
937+
}
938+
}
939+
mock_issue["storypoints"] = None
940+
u.add_project_values(
941+
issue=mock_issue,
942+
upstream="org/repo",
943+
headers={},
944+
config=self.mock_config,
945+
)
946+
# Storypoints should not be set due to unmapped value
947+
self.assertIsNone(mock_issue.get("storypoints"))
948+
771949
def test_passes_github_filters(self):
772950
"""
773951
Test passes_github_filters for labels, milestone, and other fields.

0 commit comments

Comments
 (0)