Fix Collision error not reported when paused#546
Conversation
WalkthroughObjectEngine's paused reconciliation behavior changes from skipping the create operation to executing a dry-run create. The ChangesPaused Reconciliation and Options Threading
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
supersedes #344 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #546 +/- ##
==========================================
+ Coverage 82.68% 82.69% +0.01%
==========================================
Files 32 32
Lines 2737 2739 +2
==========================================
+ Hits 2263 2265 +2
Misses 346 346
Partials 128 128 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
machinery/objects_test.go (1)
742-742: ⚡ Quick winTest name is now misleading.
The test name "Paused, create skipped" no longer matches the behavior. Based on the updated mock configuration and the implementation (context snippet showing
create()appendsclient.DryRunAllwhen paused), the create operation is now executed as a dry-run rather than being skipped entirely.Consider renaming to something like "Paused, create dry-run" to accurately reflect the current behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@machinery/objects_test.go` at line 742, The test name "Paused, create skipped" in the test case is misleading because the implementation now executes the create operation as a dry-run (by appending client.DryRunAll when paused) rather than skipping it entirely. Rename the test name field to accurately reflect the actual behavior, such as "Paused, create dry-run" to match the current implementation and mock configuration.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@machinery/objects_test.go`:
- Line 742: The test name "Paused, create skipped" in the test case is
misleading because the implementation now executes the create operation as a
dry-run (by appending client.DryRunAll when paused) rather than skipping it
entirely. Rename the test name field to accurately reflect the actual behavior,
such as "Paused, create dry-run" to match the current implementation and mock
configuration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: 21332906-1d7d-448b-9b00-a911952756c2
📒 Files selected for processing (2)
machinery/objects.gomachinery/objects_test.go
Summary
Change Type
Check List Before Merging
Additional Information