test: cover API clients, zip and seal pipeline#36
Conversation
Adds xUnit coverage for the previously-untested critical paths called out in #10: - CryptifyClient: full chunked-upload flow via a recording HttpMessageHandler (init/chunk/finalize wiring, per-chunk Content-Range, cryptifytoken rotation across chunks, notify-option mapping, error paths). - PkgClient: MPK and signing-key JSON parsing (string/object publicKey, absent/null privSignKey, Bearer auth + request body, NetworkException on non-2xx). - ZipHelper: archive round-trips for single/multiple/binary/empty inputs. - SealPipeline: non-ApiKey signing guard and parallel key-fetch failure propagation. The CI step to run these (`dotnet test` in .github/workflows/build.yml) cannot be pushed by the bot (no `workflows` permission) and is described in the PR body for a maintainer to apply. Refs #10 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Rules + review gatekeeper pass (cycle 1) — SIGN-OFF (posted as COMMENT)
Verdict is approve. Posted as COMMENT only because GitHub blocks approving a self-authored PR — treat this as a clean sign-off, not a withheld review.
Reviewed against the full binding rule set (per-rule check over the rule set) and merged with the upstream code review. The diff is test-only — five new xUnit files plus a recording HttpMessageHandler — and accurately mirrors the source behaviour. All 54 tests pass on net10.0 and both target frameworks compile. No blocking issues.
Two non-blocking notes (do not block merge):
- CI not yet running the tests —
.github/workflows/build.ymlstill runs only Build + Pack. The test project is in the.slnxso it compiles in CI, but the 54 tests never execute. The bot lacksworkflowspermission to push this; the PR body documents the exactTeststep for a maintainer to apply. Pre-existing constraint, not a regression — no rework needed from the author. - Overlap with #34 — flagged inline below.
Reviewer assigned (@rubenhensen). Refs #10 is intentionally correct here rather than a closing keyword: this PR delivers the tests but not the CI wiring (a maintainer follow-up), so #10 should stay open until that lands.
| // NOTE: the current SDK emits an exclusive range end (`end = offset + len`). | ||
| // encryption4all/postguard-dotnet#34 changes this to an inclusive end; that | ||
| // PR must update these two assertions when it lands. | ||
| Assert.Equal($"bytes 0-{ChunkSize}/*", puts[0].ContentRange); |
There was a problem hiding this comment.
Non-blocking / awareness only: this asserts the current exclusive Content-Range end (bytes 0-1048576/*). Open PR #34 changes the SDK to an inclusive end (bytes 0-1048575/*); when #34 merges, this assertion and the one below will need updating. The author already flagged it in the inline NOTE — no action needed in this PR.
The test project (61 tests) was merged in #36 but never hooked into CI. Add a Test step between Build and Pack so tests run on every push/PR.
What
Adds xUnit coverage for the critical paths that previously had no tests, per #10. All run on both
net8.0andnet10.0.New tests
CryptifyClientTests— full chunked-upload flow driven by a recordingHttpMessageHandler(no live Cryptify): init → chunk → finalize wiring, per-chunkContent-Rangeformatting,cryptifytokenrotation across chunks and into finalize, recipient-email joining,Notifyoption mapping (silent-by-default + propagation), and the missing-uuid / missing-token / non-2xx error paths.PkgClientTests— MPK and signing-key JSON parsing: string vs objectpublicKey, present / absent /nullprivSignKey,Bearerauth header + request body, andNetworkExceptionon non-2xx.ZipHelperTests— archive round-trips for single, multiple, binary and empty inputs.SealPipelineTests— the non-ApiKeysigning guard and parallel key-fetch failure propagation (asserts both PKG endpoints are hit).BuildPolicyJsonwas already covered by the existingBuildPolicyJsonTests.TestHelpers/RecordingHttpMessageHandler— reusable, thread-safe (the seal pipeline fires its two PKG calls concurrently) mock that records requests and replays queued responses.Local run: 54 tests pass on
net10.0(dotnet test --framework net10.0 --configuration Release). Per repo notes, the workspace has nonet8.0runtime, so thenet8.0leg runs in CI; both frameworks compile locally.CI change for a maintainer to apply
The bot cannot push workflow changes (no
workflowspermission), so the test project is in the solution but not yet executed by CI. Please add aTeststep between Build and Pack in.github/workflows/build.yml:--no-buildreuses the Build step's output, so both TFMs are exercised without a second compile.Note on overlap with #34
ChunkContentRange_IsFormattedPerChunkasserts the current exclusive range-end (bytes 0-1048576/*). #34 changes this to an inclusive end — when it lands, those two assertions need updating tobytes 0-1048575/*etc. Flagged inline in the test.Refs #10
🤖 Generated with Claude Code