Repository navigation
test(helpers): shared helpers for the formerly held checks-lane test files - #3246
Conversation
…ed test helpers temporaryDirectory for 26 temp-dir sites, selfSigned for the kubernetes-client and static-egress certificates, listen for local fixture servers, waitUntil for hand-rolled polls, writeCommand for the egress-enrollment codex stand-in.
…in the formerly held checks-browser files
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P3
|
Codex review: needs maintainer review before merge. What this changesThis PR replaces duplicated test setup, certificate generation, server cleanup, polling, and command scaffolding with existing shared helpers. Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep open: this PR is useful, focused test cleanup with no actionable defect found; PostgreSQL Platform CI remains in progress. Priority: P3 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherConformance and integration tests use these helpers to create temporary files, loopback peers, certificates, and command fixtures, then exercise existing controller and Driver behavior. flowchart TB
Runner[Test runner] --> Tests[Conformance and integration tests]
Tests --> Helpers[Shared test helpers]
Helpers --> Fixtures[Temporary files and loopback peers]
Tests --> Owners[Controller and Driver implementations]
Fixtures --> Owners
Owners --> Assertions[Existing behavioral assertions]
Technical reviewBest possible solution: Land the existing helper consolidation after the remaining required CI completes successfully. Do we have a high-confidence way to reproduce the issue? This is test maintenance rather than a product bug; source inspection confirms the duplicated setup on current main. Is this the best way to solve the issue? Reusing existing helpers is the smallest complete cleanup, while retaining specialized certificate and CONNECT-proxy setup where callers require it. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 9c10e6d57133. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 22, 2026, 8:36 PM ET / October 23, 2026, 00:36 UTC. |
…ess, report the exit code on the control-socket wait
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Problem
The test files held back while the Credential Proxy stack was open still carried their own copies of logic that
tests/helpers/already provides:mkdtemp+t.after(rm)pairs, a privateselfSignedCertificateopensslwrapper, private
listenandwaitForfunctions, hand-rolled poll loops and a hand-written command stand-in.Change
Test-only. The formerly held files in the checks lanes (checks-baseline-1, checks-baseline-2, checks-browser) now use
the shared helpers:
temporaryDirectory(t, prefix): 32 sites in 14 files (kubernetes-client x11, configuration-controller x4,repository-credentials-config x3, repository-credentials-package x3, skipper-private-git-origin x2, and one each in
kubernetes-configuration, openshell-prepared-git-provider, repository-credentials-registry,
repository-credentials-source-boundary, common-logging, egress-enrollment and the harness-tls, static-egress and
protected-driver-package helpers).
selfSigned(certificates.mjs): kubernetes-client's private openssl wrapper (CN kept as the certificate's name,which the API server stand-in records per connection), the static-egress fixture certificate (its PEM text is
reused as Buffers for gRPC, no second read) and repository-credentials-client-config's native Git peer.
listen(listen.mjs): static-egress's private copy and its raw TCP origin, kubernetes-client's API serverstand-ins, egress-connected-preflight, protected-driver-registration, egress-enrollment (was importing the
container-mounted
process.mjscopy).waitUntil(wait-for.mjs): kubernetes-client's privatewaitFor, repository-credentials-package's two startpolls, skipper-private-git-origin's discovery poll and repository-credentials-driver's disposal poll.
writeCommand(fake-commands.mjs): egress-enrollment's codex stand-in (PATH stays the stand-in directory plusNode only, as before).
Kept as is: harness-tls's CA and leaf generation (callers sign extra leaves with
ca.key/ca.pemin its directory),kubernetes-client's CONNECT proxy (it tracks tunnel sockets too), and repository-credentials-package's RSA
certificate for the detached runtime.
Verification
per-test kill sets for all 20 (redirect following, keep-alive, provisioning cancellation, protected-file and
registry checks, the detached service's start event, the boundary verifier's symlink check, native Git redirect
defaults, egress enrollment revision binding, authority TLS, filesystem Configuration validation, prepared Git CAS,
connected-request redirect, default log level, the private origin's held discovery, session disposal; the last
hangs the driver test on both sides). The 9 static-egress callers were rerun after the review nits: identical,
365/365, 51.0 s on main vs 48.2 s here.
the 8-file subset 453/453, 10.4 s on both.
seven groups; mean group walls 9-58 s.
tsc --build tests/tsconfig.jsonreports nothing new.Risks
Test-only. Failure messages from the converted polls now read "Timed out waiting for ...". Certificates from
selfSignedare P-256 rather than RSA in static-egress and the client-config Git peer; every suite that uses thempassed locally.