Skip to content

Commit 61e1f48

Browse files
committed
resmgr: the cluster role import test verifies the imported state
1 parent c243436 commit 61e1f48

2 files changed

Lines changed: 13 additions & 9 deletions

File tree

docs/superpowers/plans/2026-09-03-host-cluster-role-import.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,4 +512,4 @@ Restore `REPO_DIR` in `suite.env`. Nothing from this task is committed.
512512
- **Spec coverage.** Design §1 (import writes the default) → Task 3. Design §2 (helper + Update short-circuit, equality rules for null/""/unknown/empty/order) → Tasks 1–2; every equality rule in the spec has a table row in Task 1. Testing section: helper table test → Task 1; ImportState unit test via `tfsdk.State` → Task 3; Update no-op through framework types → Task 2; lab check as the live fallback → Task 5. Changelog → Task 4. Non-goals respected: no schema change, no read-back of `host_cluster`/`backends` (Task 3 asserts they stay null).
513513
- **Placeholders.** None: every step carries its code or its exact command and expected output.
514514
- **Type consistency.** `roleOptionsChanged(plan, state *hostClusterRoleModel) bool`, `hostClusterOption`, `backendsOption` are defined in Task 1 and used with the same signatures in Task 2. `roleSchema(t)` and `roleState(t, m)` are defined in Task 2 and reused in Task 3. `roleModel` and `backendsList` are defined in Task 1's test file and reused in Tasks 2–3.
515-
- **Post-approval addition.** At the final whole-branch review, the reviewer asked for the acceptance test described in Task 5/the Testing section to actually exist rather than stay a documented manual fallback. Added `TestAccResmgrHostClusterRoleImport` (and `testAccCheckHostClusterRoleDestroy`) to `internal/services/resmgr/resmgr_test.go`, gated on `PCD_ACC_RESMGR` and `PCD_ACC_HOST_ID`, by user decision — the plan and the branch now agree.
515+
- **Post-approval addition.** At the final whole-branch review, the reviewer asked for the acceptance test described in Task 5/the Testing section to actually exist rather than stay a documented manual fallback. Added `TestAccResmgrHostClusterRoleImport` (and `testAccCheckHostClusterRoleDestroy`) to `internal/services/resmgr/resmgr_test.go`, gated on `PCD_ACC_RESMGR` and `PCD_ACC_HOST_ID`, by user decision — the plan and the branch now agree. Its steps are create, ImportStateVerify, destroy-poll: create the role and check its attributes, import it into a fresh working directory and let `ImportStateVerify` compare the imported attributes against the created state, then `testAccCheckHostClusterRoleDestroy` polls the host record after destroy until the role clears.

internal/services/resmgr/resmgr_test.go

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -118,9 +118,12 @@ func testAccCheckHostConfigDestroy(t *testing.T) resource.TestCheckFunc {
118118
}
119119
}
120120

121-
// TestAccResmgrHostClusterRoleImport assigns a cluster role, imports it, and
122-
// checks the plan is clean afterwards — the import used to leave
123-
// wait_until_converged null and plan a spurious update that re-PUT the role.
121+
// TestAccResmgrHostClusterRoleImport assigns a cluster role, then imports it
122+
// into a fresh working directory and lets ImportStateVerify compare every
123+
// imported attribute against the state created in the first step. Before the
124+
// fix, the imported state left wait_until_converged null while the created
125+
// state held false, so ImportStateVerify failed on the pre-fix code and
126+
// passes on the fixed code.
124127
// Mutates the control plane, so it is opt-in: PCD_ACC_RESMGR=1 and
125128
// PCD_ACC_HOST_ID=<host uuid> (an onboarded host that has a host configuration
126129
// assigned). PCD_ACC_CLUSTER_ROLE picks the role (default image-library); the
@@ -156,11 +159,12 @@ resource "pcd_host_cluster_role" "test" {
156159
resource.TestCheckResourceAttr(rn, "wait_until_converged", "false"),
157160
),
158161
},
159-
// Import into the working state (ImportStatePersist) so the plan-only
160-
// step below runs against the *imported* state — that is the plan that
161-
// used to show `+ wait_until_converged = false`.
162-
{ResourceName: rn, ImportState: true, ImportStateId: hostID + "/" + role, ImportStateVerify: true, ImportStatePersist: true},
163-
{Config: cfg, PlanOnly: true},
162+
// ImportState runs in its own, fresh working directory; ImportStateVerify
163+
// then diffs every attribute of that imported state against the state
164+
// created above. That diff is the regression guard: before the fix, the
165+
// imported state left wait_until_converged null while the created state
166+
// held false, so this comparison failed.
167+
{ResourceName: rn, ImportState: true, ImportStateId: hostID + "/" + role, ImportStateVerify: true},
164168
},
165169
})
166170
}

0 commit comments

Comments
 (0)