Conversation
|
| frappe.set_user("Administrator") | ||
| try: | ||
| for pipeline_build in self.pipeline_builds: | ||
| self._cancel_build_if_running(pipeline_build.build) | ||
| finally: | ||
| frappe.set_user(current_user) |
There was a problem hiding this comment.
Regression Coverage Missing
This bug fix adds Administrator impersonation and restoration without a regression test. The repository requires every bug fix to have coverage, so add a test proving force-fail cancels builds as Administrator and restores the original user after both success and exceptions before merging.
Context Used: Guidelines for reviewing Frappe Framework applications. (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: press/press/doctype/release_pipeline/release_pipeline.py
Line: 235-240
Comment:
**Regression Coverage Missing**
This bug fix adds Administrator impersonation and restoration without a regression test. The repository requires every bug fix to have coverage, so add a test proving force-fail cancels builds as Administrator and restores the original user after both success and exceptions before merging.
**Context Used:** Guidelines for reviewing Frappe Framework applications. ([source](https://github.com/frappe/skills/blob/main/skills/quality-code-review/SKILL.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #7456 +/- ##
===========================================
+ Coverage 61.94% 62.07% +0.13%
===========================================
Files 1067 1067
Lines 102449 102679 +230
Branches 1777 1777
===========================================
+ Hits 63460 63738 +278
+ Misses 38944 38896 -48
Partials 45 45
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tick the box to add this pull request to the merge queue (same as
|
The earlier fix broke because it ran as the regular user, who isn't allowed to touch Agent Job records, so cancelling silently failed every time. Now it runs as admin for just that step