test(gitlab): use push SHA to avoid stale MR API#2453
Conversation
🔍 PR Lint Feedback
|
Summary of ChangesHello @theakshaypant, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an intermittent failure in the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a flaky test in the GitLab integration by avoiding a race condition when fetching the merge request SHA after a force push. The change correctly uses the locally generated SHA from the push operation instead of relying on the potentially stale data from the GitLab API.
My review identifies a critical issue where the fix is incomplete, leaving subsequent test assertions to use the old, stale SHA, which would break the test. The provided code suggestion fully corrects this.
0372b2f to
a24f32b
Compare
GitLab's GetMergeRequest API may return stale SHA after force push. Use SHA from PushFilesToRefGit directly instead. Signed-off-by: Akshay Pant <akshay.akshaypant@gmail.com>
493fb27 to
1674df1
Compare
📝 Description of the Change
GitLab's GetMergeRequest API may return stale SHA after force push. Use SHA from PushFilesToRefGit directly instead.
Using evidence from failed
TestGitlabConsistentCommitStatusOnMRon another PR.Problem
TestGitlabConsistentCommitStatusOnMRwas intermittently failing with:Root Cause Hypothesis
After a force push, the test called
GetMergeRequestto fetch the updated MR SHA. However, GitLab's API returns a stale SHA before GitLab has finished processing the push webhook internally.Evidence from logs:
{ "level": "info", "ts": "2026-02-05T12:49:52.833Z", "logger": "pipelinesascode", "caller": "pipelineascode/pipelineascode.go:294", "msg": "PipelineRun always-good-pipelinerun-kwq8h has been created in namespace pac-e2e-ns-bd244 with status for SHA: c48223c8f927eaea6a7b24e75afc91d537cfe472 Target Branch: main", "commit": "2612189", "provider": "gitlab", "event-id": "7d06e227c2e926fab7b1be100b49a29e", "event-sha": "c48223c8f927eaea6a7b24e75afc91d537cfe472", "event-type": "Merge Request", "source-repo-url": "https://gitlab.com/openshift-pipelines/pipelines-as-code-e2e-tests", "target-branch": "main", "source-branch": "pac-e2e-test-sx66l", "namespace": "pac-e2e-ns-bd244" }{ "level": "info", "ts": "2026-02-05T12:50:19.039Z", "logger": "pipelinesascode", "caller": "pipelineascode/pipelineascode.go:294", "msg": "PipelineRun always-good-pipelinerun-n6h9m has been created in namespace pac-e2e-ns-bd244 with status for SHA: 4d9a63530d98ca27ac81561bc69728afee6ea5e5 Target Branch: main", "commit": "2612189", "provider": "gitlab", "event-id": "b7397934527619697327ff4125c61e96", "event-sha": "4d9a63530d98ca27ac81561bc69728afee6ea5e5", "event-type": "Merge Request", "source-repo-url": "https://gitlab.com/openshift-pipelines/pipelines-as-code-e2e-tests", "target-branch": "main", "source-branch": "pac-e2e-test-sx66l", "namespace": "pac-e2e-ns-bd244" } { "level": "info", "ts": "2026-02-05T12:50:19.039Z", "logger": "pipelinesascode", "caller": "pipelineascode/pipelineascode.go:294", "msg": "PipelineRun bad-converts-good-pipelinerun-r6mbf has been created in namespace pac-e2e-ns-bd244 with status for SHA: 4d9a63530d98ca27ac81561bc69728afee6ea5e5 Target Branch: main", "commit": "2612189", "provider": "gitlab", "event-id": "b7397934527619697327ff4125c61e96", "event-sha": "4d9a63530d98ca27ac81561bc69728afee6ea5e5", "event-type": "Merge Request", "source-repo-url": "https://gitlab.com/openshift-pipelines/pipelines-as-code-e2e-tests", "target-branch": "main", "source-branch": "pac-e2e-test-sx66l", "namespace": "pac-e2e-ns-bd244" }The test expected
c48223c8(which came frommr.SHAafterGetMergeRequest), but PAC correctly processed4d9a6353(the actual new SHA). This provesGetMergeRequestreturned the old SHA before GitLab updated its internal MR state.Timeline of the race:
Fix
Use the SHA returned directly from
PushFilesToRefGitinstead of fetching it from GitLab's API:As we can see form the assertion failure that
sopts.SHAis stale (SHA1) which is fetched using the gitlab clientWhy This Works
The local
git rev-parse HEADSHA is computed from the commit content before pushing. Since Git SHAs are deterministic, the remote will have exactly the same SHA once the push completes.Additional Benefit
This also removes an unnecessary API call to GitLab, making the test slightly faster.
👨🏻 Linked Jira
N/A
🔗 Linked GitHub Issue
N/A
🚀 Type of Change
fix:)feat:)feat!:,fix!:)docs:)chore:)refactor:)enhance:)deps:)🧪 Testing Strategy
🤖 AI Assistance
If you have used AI assistance, please provide the following details:
Which LLM was used?
Extent of AI Assistance:
Important
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: Gemini gemini@google.com
Co-authored-by: ChatGPT noreply@chatgpt.com
Co-authored-by: Claude noreply@anthropic.com
Co-authored-by: Cursor noreply@cursor.com
Co-authored-by: Copilot Copilot@users.noreply.github.com
**💡You can use the script
./hack/add-llm-coauthor.shto automatically addthese co-author trailers to your commits.
✅ 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.