From c3f051bba5f2b70920fde170a82facfcd5134477 Mon Sep 17 00:00:00 2001
From: "goodolclint-claude[bot]"
 <323206664+goodolclint-claude[bot]@users.noreply.github.com>
Date: Wed, 2 Sep 2026 13:27:43 +0000
Subject: [PATCH] =?UTF-8?q?docs:=20record=20D021=20=E2=80=94=20integration?=
 =?UTF-8?q?=20tests=20prove=20server=20semantics,=20payloads=20offline=20(?=
 =?UTF-8?q?#125)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Records the testing strategy decided while planning #120: an integration test
must earn its place by testing something only a live PVE can answer, and
request-payload correctness is verified offline against the mock IPveHttpClient
harness.

The boundary comes from #92. Set-PveNetwork sent bridge_vlan_aware=0 to clear a
VLAN-aware bridge; the schema advertises a plain boolean, so the request
succeeded, and PVE merged the key onto the stored stanza and ignored the 0. Only
delete=bridge_vlan_aware works, and only a real PVE 9 revealed it.

37 of 194 concrete cmdlets construct PveHttpClient directly and have no offline
seam; they convert before the next large coverage push. The suite tiers by area
on PRs, ACME is covered by contract tests because a CA and DNS reachability
cannot exist in CI, and Ceph lives behind an opt-in provisioning profile.
---
 DECISIONS.md | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/DECISIONS.md b/DECISIONS.md
index 820a032..77027ca 100644
--- a/DECISIONS.md
+++ b/DECISIONS.md
@@ -877,3 +877,68 @@ var task = Wait.IsPresent
     ? InvokeGuestTask(session, sourceNode, Issue)
     : Issue();
 ```
+
+---
+
+## D021 — Integration tests prove server semantics; payloads are proven offline
+
+**Status**: Active
+**Issue refs**: #120 (coverage), #92/#118 (the case that motivated it)
+**Decided**: 2026-09-02
+
+### Decision
+An integration test must earn its place by testing something only a live PVE can answer.
+Request-payload correctness — which keys a cmdlet sends, and with what values — is verified
+offline against the mock `IPveHttpClient` harness.
+
+Concretely:
+- New cmdlets route through a `*Service` that accepts `IPveHttpClient`, so their payload is
+  reachable from `PSProxmoxVE.Core.Tests` without a cluster.
+- The 37 cmdlets that construct `PveHttpClient` directly are converted to that seam before the
+  next large coverage push, and opportunistically when otherwise touched. Measured against 194
+  concrete cmdlet files (`src/PSProxmoxVE/Cmdlets/**/*.cs` less the `PveCmdletBase` base class):
+  155 reach the API only through a `*Service`, 25 only through their own client, 12 do both, and
+  2 do neither. The service and direct-client sets overlap, so they do not sum to 194.
+- The integration suite is tiered: a PR exercises smoke plus the areas its diff touches
+  (`run-integration.sh test <ver> <Area>` already supports this); the full suite runs on merge
+  to `main`.
+- Areas whose dependencies cannot exist in CI (ACME needs a CA plus DNS or HTTP reachability)
+  are covered by mock/contract tests asserting request shape, not by a live lane.
+- Areas needing a differently-shaped cluster (Ceph needs dedicated block devices per node and
+  wants three monitors) live behind an opt-in provisioning profile, so ordinary runs do not pay
+  for them.
+
+### Rationale
+`Set-PveNetwork` sent `bridge_vlan_aware=0` to clear a VLAN-aware bridge. The API schema
+advertises a plain optional boolean, so the request is valid and returns success — and PVE
+merges supplied keys onto the stored stanza and ignores the `0`. The flag never cleared. Only
+`delete=bridge_vlan_aware` works, and only a run against a real PVE 9 revealed it.
+
+That is what a live cluster is for: server behaviour the schema misdescribes. It is not for
+checking that a dictionary has the right keys, which a mock proves in milliseconds.
+
+The distinction matters because it decides whether the suite scales. Coverage is ~36% of 678
+endpoints; the remaining surface is large enough that "every new endpoint gets a live test" puts
+the integration suite on a growth curve the CI budget cannot absorb. Growth is linear in
+*live-only* surface, and how much surface is live-only is a design choice, not a given.
+
+### Anti-pattern (do not reintroduce)
+```csharp
+// A cmdlet that builds its own form and owns its own client has no offline seam:
+// every field below is verifiable only by provisioning a cluster.
+using var client = new PveHttpClient(session);
+var data = new Dictionary<string, string> { ["type"] = Type };
+if (!string.IsNullOrEmpty(Address)) data["address"] = Address!;
+client.PutAsync($"nodes/{node}/network/{iface}", data).GetAwaiter().GetResult();
+```
+
+### Correct pattern
+This is the target shape, not a form the tree already takes. `Set-PveNetwork` is one of the 37,
+and `NetworkService.SetNetwork` has no callers anywhere in the repository.
+
+```csharp
+// Service takes IPveHttpClient, so PSProxmoxVE.Core.Tests can assert the emitted form
+// without a cluster; the integration test then covers only what PVE alone can tell us.
+var service = new NetworkService();
+service.SetNetwork(session, Node, Iface, config);
+```