Skip to content

Commit 08c794a

Browse files
committed
[OJ-54730] Add regression test for BB Cloud merge commit 404 handling
Verifies that when client.get_commit raises HTTPError(404), the PR is still returned with merge_commit=None instead of being skipped.
1 parent 250b858 commit 08c794a

1 file changed

Lines changed: 100 additions & 4 deletions

File tree

tests/test_bitbucket_cloud_adapter.py

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import json
22
import unittest
3-
4-
from dateutil import parser
53
from unittest import TestCase
64
from unittest.mock import MagicMock
75

6+
import requests
7+
from dateutil import parser
8+
89
from jf_agent.git import StandardizedShortRepository
910
from jf_agent.git.bitbucket_cloud_adapter import BitbucketCloudAdapter
1011

@@ -310,7 +311,11 @@ def test_get_prs_merge_commit_uses_destination_repo(self):
310311
'name': 'fork_repo',
311312
'type': 'repository',
312313
'uuid': source_uuid,
313-
'links': {'self': {'href': 'https://api.bitbucket.org/2.0/repositories/some-fork/fork'}},
314+
'links': {
315+
'self': {
316+
'href': 'https://api.bitbucket.org/2.0/repositories/some-fork/fork'
317+
}
318+
},
314319
},
315320
},
316321
'destination': {
@@ -320,7 +325,11 @@ def test_get_prs_merge_commit_uses_destination_repo(self):
320325
'name': dest_slug,
321326
'type': 'repository',
322327
'uuid': '{cccccccc-0000-0000-0000-dddddddddddd}',
323-
'links': {'self': {'href': f'https://api.bitbucket.org/2.0/repositories/test_project/{dest_slug}'}},
328+
'links': {
329+
'self': {
330+
'href': f'https://api.bitbucket.org/2.0/repositories/test_project/{dest_slug}'
331+
}
332+
},
324333
},
325334
},
326335
'links': {'html': {'href': 'https://bitbucket.org/test_project/pull-requests/99'}},
@@ -349,6 +358,93 @@ def test_get_prs_merge_commit_uses_destination_repo(self):
349358
'test_project', dest_slug, 'abc123merge'
350359
)
351360

361+
def test_get_prs_merge_commit_404_does_not_skip_pr(self):
362+
# Regression test for OJ-54730: when Bitbucket Cloud has garbage-collected a PR's
363+
# merge commit, get_commit raises HTTPError(404). The PR must still be returned,
364+
# just with merge_commit left unset.
365+
test_commits = _get_test_data('test_commits.json')
366+
dest_slug = 'destination_repo_slug'
367+
368+
merged_pr = {
369+
'id': 2394,
370+
'title': 'Merged PR with stale merge commit',
371+
'description': '',
372+
'state': 'MERGED',
373+
'merge_commit': {'hash': '9a9601098ec2'}, # pragma: allowlist secret
374+
'created_on': '2020-01-01T00:00:00+00:00',
375+
'updated_on': '2020-01-02T00:00:00+00:00',
376+
'author': {
377+
'display_name': 'test_user',
378+
'uuid': '{1cd06601-cd0e-4fce-be03-e9ac226978b7}',
379+
'links': {'html': {'href': 'https://bitbucket.org/test_user/'}},
380+
},
381+
'source': {
382+
'branch': {'name': 'feature'},
383+
'repository': {
384+
'full_name': f'test_project/{dest_slug}',
385+
'name': dest_slug,
386+
'type': 'repository',
387+
'uuid': '{cccccccc-0000-0000-0000-dddddddddddd}',
388+
'links': {
389+
'self': {
390+
'href': f'https://api.bitbucket.org/2.0/repositories/test_project/{dest_slug}'
391+
}
392+
},
393+
},
394+
},
395+
'destination': {
396+
'branch': {'name': 'master'},
397+
'repository': {
398+
'full_name': f'test_project/{dest_slug}',
399+
'name': dest_slug,
400+
'type': 'repository',
401+
'uuid': '{cccccccc-0000-0000-0000-dddddddddddd}',
402+
'links': {
403+
'self': {
404+
'href': f'https://api.bitbucket.org/2.0/repositories/test_project/{dest_slug}'
405+
}
406+
},
407+
},
408+
},
409+
'links': {'html': {'href': 'https://bitbucket.org/test_project/pull-requests/2394'}},
410+
}
411+
412+
mock_standardized_repo = MagicMock()
413+
mock_standardized_repo.id = '{cccccccc-0000-0000-0000-dddddddddddd}'
414+
mock_standardized_repo.full_name = f'test_project/{dest_slug}'
415+
mock_standardized_repo.project.id = 'test_project'
416+
mock_standardized_repos = [mock_standardized_repo]
417+
418+
mock_response = MagicMock()
419+
mock_response.status_code = 404
420+
http_error = requests.exceptions.HTTPError(
421+
'404 Client Error: Not Found', response=mock_response
422+
)
423+
424+
self.mock_client.get_pullrequests.return_value = [merged_pr]
425+
self.mock_client.pr_diff.return_value = ""
426+
self.mock_client.pr_comments.return_value = []
427+
self.mock_client.pr_activity.return_value = []
428+
self.mock_client.pr_commits.return_value = test_commits
429+
self.mock_client.get_commit.side_effect = http_error
430+
431+
test_git_instance_info = {'pull_from': '1900-07-23', 'repos_dict_v2': {}}
432+
433+
resulting_prs = list(
434+
self.adapter.get_pull_requests(mock_standardized_repos, test_git_instance_info)
435+
)
436+
437+
self.assertEqual(
438+
len(resulting_prs), 1, "PR must still be ingested despite 404 on merge commit"
439+
)
440+
resulting_pr = resulting_prs[0]
441+
self.assertEqual(resulting_pr.id, 2394)
442+
self.assertTrue(resulting_pr.is_merged)
443+
self.assertIsNone(
444+
resulting_pr.merge_commit,
445+
"merge_commit should be None when Bitbucket returns 404 for the merge commit hash",
446+
)
447+
352448

353449
def _get_test_data(file_name):
354450
with open(f'{TEST_INPUT_FILE_PATH}{file_name}', 'r') as f:

0 commit comments

Comments
 (0)