HYPERFLEET-1017 - refactor: update for LastKnownReconciled rename#107
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
WalkthroughThis change updates test fixtures and mock data across multiple test files, replacing occurrences of the "Available" status condition type with "LastKnownReconciled". Modifications appear in cluster and nodepool mocks, configuration tests, decision engine tests (including an added subtest variant), integration test mocks, and the mock API generator. No production logic, API signatures, or exported declarations are modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Comment |
rafabene
force-pushed
the
rbenevid/HYPERFLEET-1017/rename-available-to-last-known-reconciled
branch
from
1ee97d4 to
e8e6647
Compare
rafabene
force-pushed
the
rbenevid/HYPERFLEET-1017/rename-available-to-last-known-reconciled
branch
from
e8e6647 to
cb05508
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/client/client_test.go (1)
44-45: ⚡ Quick winAssert the renamed condition is actually parsed and present
Line 44 and Line 590 update fixtures to
LastKnownReconciled, but the tests still only validate generic condition status/indexes. That means this rename can regress without failing this test file. Add an explicit assertion that at least one parsed condition hasType == "LastKnownReconciled"in both cluster and nodepool test paths.Proposed test assertion pattern
+ foundLastKnownReconciled := false + for _, cond := range resources[0].Status.Conditions { + if cond.Type == "LastKnownReconciled" { + foundLastKnownReconciled = true + break + } + } + if !foundLastKnownReconciled { + t.Fatal("Expected LastKnownReconciled condition to be present") + }Also applies to: 590-591
🤖 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 `@internal/client/client_test.go` around lines 44 - 45, Tests updated fixtures to use "LastKnownReconciled" but never assert the renamed condition is actually parsed; add an explicit check in the cluster and nodepool test paths in internal/client/client_test.go that verifies at least one parsed condition has Type == "LastKnownReconciled". Locate the places where conditions are parsed into a slice (e.g., variables named parsedConditions / conditions / clusterConditions / nodepoolConditions or the test helpers that call the parsing function) and add an assertion that iterates that slice and expects at least one condition.Type == "LastKnownReconciled" (fail the test if none found).
🤖 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 `@internal/client/client_test.go`:
- Around line 44-45: Tests updated fixtures to use "LastKnownReconciled" but
never assert the renamed condition is actually parsed; add an explicit check in
the cluster and nodepool test paths in internal/client/client_test.go that
verifies at least one parsed condition has Type == "LastKnownReconciled". Locate
the places where conditions are parsed into a slice (e.g., variables named
parsedConditions / conditions / clusterConditions / nodepoolConditions or the
test helpers that call the parsing function) and add an assertion that iterates
that slice and expects at least one condition.Type == "LastKnownReconciled"
(fail the test if none found).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Enterprise
Run ID: b82262c2-eeae-438a-a5a9-228abe68af65
📒 Files selected for processing (5)
internal/client/client_test.gointernal/config/config_test.gointernal/engine/decision_test.gotest/integration/integration_test.gotest/mock-hyperfleet-api/main.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/engine/decision_test.go
…ciled rename The HyperFleet API renamed the aggregated Available condition to LastKnownReconciled. Update all test fixtures, CEL expressions, and the mock API server to match.
rafabene
force-pushed
the
rbenevid/HYPERFLEET-1017/rename-available-to-last-known-reconciled
branch
from
cb05508 to
00b573a
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pnguyen44 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
Bot
merged commit 36b9830
into
openshift-hyperfleet:main
3 participants
Summary
LastKnownReconciledinstead ofAvailablefor the aggregated conditionDependencies
Changed files
internal/client/client_test.go— test fixturesinternal/config/config_test.go— CEL expression in default config testinternal/engine/decision_test.go— decision engine test fixtures and assertionstest/integration/integration_test.go— integration test fixturestest/mock-hyperfleet-api/main.go— mock server condition typeTest plan
go test ./...)Summary by CodeRabbit