fix(bitbucket-datacenter): detect changes on merged PR push#2719
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for accurately detecting file changes in Bitbucket Data Center push events involving merge commits. It implements a new getMergeCommitChanges method using the Bitbucket /changes API and updates the payload parsing logic to identify merge commits via parent counts. Feedback suggests improving the robustness of the API request by using url.Values for safer parameter escaping, refining the merge commit detection condition to be less restrictive, and enhancing error reporting for API failures.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2719 +/- ##
==========================================
+ Coverage 59.25% 59.39% +0.13%
==========================================
Files 208 209 +1
Lines 20573 20663 +90
==========================================
+ Hits 12191 12272 +81
- Misses 7610 7616 +6
- Partials 772 775 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8622c33 to
869ff72
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enables accurate file change detection for Bitbucket Data Center when a pull request is merged. It implements a custom API client to retrieve changes between the previous HEAD and the merge commit, addressing limitations in the underlying scm library. The implementation includes merge commit detection during payload parsing and updated E2E tests for validation. Feedback recommends using the internal client wrapper to maintain API usage metrics, refining HTTP error handling, and improving the robustness of the commit matching logic.
869ff72 to
99dc953
Compare
theakshaypant
left a comment
There was a problem hiding this comment.
Minor comments on the changes
There was a problem hiding this comment.
Some of these structs are duplicated in pkg/provider/bitbucketdatacenter/test/test_types.go, can we avoid this duplication?
There was a problem hiding this comment.
thanks for pointing, didn't pay attention. removed types from test_types.go
When a pull request is merged in Bitbucket Data Center, the resulting push event contains a merge commit that reports no file changes. This caused on-path-change and on-cel-expression filters to silently skip PipelineRuns because the changed files list was always empty. The fix detects merge commits (commits with multiple parents) during payload parsing and uses the Bitbucket /changes API with since/until params to diff the previous HEAD against the current HEAD, retrieving the actual files modified by the merged PR. Key changes: - Parse merge commits in push payloads to capture previousHeadCommit - Add getMergeCommitChanges using /changes API with proper pagination - Add DiffStats/Pagination types for the /changes API response - Update TearDown to skip deletion of already-merged PRs - Allow CreatePR to target a custom base branch for merge test scenarios - Add E2E test for on-path-change annotation surviving a PR merge push - Add unit tests for getMergeCommitChanges and merge commit GetFiles path Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
99dc953 to
e20f740
Compare
📝 Description of the Change
When a pull request is merged in Bitbucket Data Center, the resulting push event contains a merge commit that reports no file changes. This caused on-path-change and on-cel-expression filters to silently skip PipelineRuns because the changed files list was always empty.
The fix detects merge commits (commits with multiple parents) during payload parsing and uses the Bitbucket /changes API with since/until params to diff the previous HEAD against the current HEAD, retrieving the actual files modified by the merged PR.
Key changes:
🔗 Linked GitHub Issue
Fixes #
JIRA
https://redhat.atlassian.net/browse/SRVKP-9638
🧪 Testing Strategy
🤖 AI Assistance
AI assistance can be used for various tasks, such as code generation,
documentation, or testing.
Please indicate whether you have used AI assistance
for this PR and provide details if applicable.
Important
Slop will be simply rejected, if you are using AI assistance you need to make sure you
understand the code generated and that it meets the project's standards. you
need at least know how to run the code and deploy it (if needed). See
startpaac to make it easy
to deploy and test your code changes.
If the majority of the code in this PR was generated by an AI, please add a
Co-authored-bytrailer to your commit message.For example:
Co-authored-by: Claude noreply@anthropic.com
✅ Submitter Checklist
fix:,feat:) matches the "Type of Change" I selected above.make testandmake lintlocally to check for and fix anyissues. For an efficient workflow, I have considered installing
pre-commit and running
pre-commit installtoautomate these checks.