Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
tag: adapter-call-unmarshal-helper
author: yvonne-devlin
category: gotcha
created_at: 2026-08-27T15:11:41Z
identity: adapter-call-unmarshal-helper-20260827T151141-yvonne-devlin
tier: draft
---

Testing a generic JSON-RPC helper's own json.Unmarshal-failure branch: the fake analyzer's --malformed-json flag corrupts the JSON-RPC ENVELOPE itself, so protocol.Client.Call fails first and the error surfaces as a TRANSPORT error ("<method> protocol call: %w"), NOT the helper's own result-unmarshal path. To exercise the helper's json.Unmarshal(resp.Result, &result) failure branch you need a VALID envelope carrying a well-formed but type-incompatible result — use a test-only result type like `type mismatchedResult struct{ Functions string \`json:"functions"\` }` (Functions typed as string where the real result has an array), forcing Unmarshal to fail inside the helper while the envelope stays valid. Assert the error contains "parsing <method> result" and errors.Unwrap != nil (%w preserved).
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
tag: adapter-call-unmarshal-helper
author: yvonne-devlin
category: gotcha
created_at: 2026-08-27T15:11:49Z
identity: adapter-call-unmarshal-helper-20260827T151149-yvonne-devlin
tier: draft
---

Go test-binary constraint: only ONE TestMain is allowed per compiled test binary. In internal/adapter/ the existing TestMain lives in `package adapter_test` (external test package) and builds+caches the fake analyzer binary. A NEW internal-package test file (`package adapter`, required to access unexported symbols like callAndUnmarshal) compiles into the SAME test binary and therefore CANNOT declare its own TestMain, nor can it reference the external package's unexported fakeBinaryPath var. Resolution: the internal test file builds its OWN fake-analyzer copy lazily via sync.Once (package-level vars: binaryPath string, buildOnce sync.Once, buildErr error). GOTCHA: this sync.Once-based lazy build canNOT use t.Cleanup for temp-dir teardown — Once.Do runs during the FIRST SUBTEST that calls it and captures that subtest's *testing.T, so the cleanup fires when that first subtest ends, deleting the shared binary before later subtests run (observed failure: "analyzer binary ... not found" in a later subtest). Accept the small (~4-5MB) OS-temp leak instead; there is no package-level teardown hook available without a TestMain.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
tag: adapter-call-unmarshal-helper
author: yvonne-devlin
category: pattern
created_at: 2026-08-27T15:11:58Z
identity: adapter-call-unmarshal-helper-20260827T151158-yvonne-devlin
tier: draft
---

Pattern/design: a collapsed generic error-wrapping helper (e.g. callAndUnmarshal[T] that maps Call→transport-err→protocol-err→unmarshal into uniform "%s protocol call"/"%s protocol error"/"parsing %s result" templates) CANNOT preserve per-branch legacy error wording when a call site's historical strings diverge from the template. In internal/adapter/, the 3 batch sites whose method constants ARE the human prefix (complexity/coverage/analyze) migrated cleanly because the templates reproduce their legacy strings 1:1. But session.go Initialize used THREE distinct non-template strings ("initialize handshake"/"initialize error"/"parsing initialize result") that the collapsed single-error helper cannot reproduce without fragile prefix translation. Resolution sanctioned by design D2: Initialize RETAINS its original inline per-branch error handling (and its per-branch s.client.Close() cleanup) rather than delegating — so effective helper adoption was 3 sites, not the originally-planned 4. Lesson: when planning a DRY error-helper extraction, audit each call site's exact error strings first; sites with multiple distinct historical prefixes are legitimate exclusions, and preserving exact operator-facing error strings (log/alert grep patterns) outranks maximizing migration count.
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ Formatters: gofmt, goimports.
## Recent Changes

- decompose-high-complexity-functions: Decomposed 5 high-complexity functions to reduce cyclomatic complexity (CC) and CRAPload (issue #200, continuation of #166). `isPointerArgStore` (CC 13→3): removed structurally unreachable branches — `tracesToParam` already walks FieldAddr/IndexAddr/UnOp chains. `detectASTReceiverMutations` (CC 24→11): extracted 3 per-node-type handlers (`handleReceiverAssignStmt`, `handleReceiverIncDecStmt`, `handleReceiverCallExpr`) from `ast.Inspect` closure. `runCrap` (CC 19→14): extracted `resolveBaselineAndCompare`, `writeCrapOutputAndSummary`, `evaluateCrapGates` preserving D7 gate ordering (baseline before threshold). `runQuality` (CC 32→14): extracted `loadQualityConfig`, `setupQualityDeps`, `writeQualityReport`, `handleQualityEmptyResults`, plus previously extracted `runQualityPerPackage` and `writeQualityEmptyResults`. `writeOneResult` (CC 32→4): extracted `buildEffectsTable`, `writeTierSummary`, plus previously extracted `writeEffectRows` and `writeVerboseSignals`. Added ~35 new unit tests across 3 packages using synthetic AST, DI structs, and `testdata/src/` fixtures. No behavioral changes — all existing tests pass without modification.
- adapter-call-unmarshal-helper: Extracted a generic `callAndUnmarshal[T any](ctx context.Context, client *protocol.Client, method string, params any) (T, error)` helper (`internal/adapter/call.go`, unexported) that centralizes the repeated JSON-RPC `Call → transport-error check → protocol-error check → json.Unmarshal[T]` pattern. Migrated 3 hard-error batch call sites: `ExternalComplexityProvider.Analyze` (`complexity.go`, `callAndUnmarshal[protocol.ComplexityResult]`), `ExternalLineCoverageProvider.Coverage` (`coverage.go`, `[protocol.CoverageResult]`), and `ExternalSideEffectAnalyzer.loadBatch` (`sideeffect.go`, `[protocol.AnalyzeResult]`). Because the method constants are literally `"complexity"`/`"coverage"`/`"analyze"`, the helper's D2 error templates (`"%s protocol call: %w"`, `"%s protocol error: %s (code %d)"`, `"parsing %s result: %w"`) reproduce the exact legacy error strings. Error-chain contract (design D6): transport and unmarshal errors wrap with `%w` (`errors.Is`/`errors.As` preserved); protocol errors format with `%s` (structured JSON-RPC error object, not a Go error chain). `Session.Initialize` (`session.go`) intentionally retains its inline per-branch error handling (design D2 sanctioned option) because the collapsed generic helper cannot reproduce its three distinct legacy strings (`"initialize handshake"`/`"initialize error"`/`"parsing initialize result"`); it also keeps `s.client.Close()` cleanup on each error branch. Explicitly excluded from migration: `CallStream`/streaming side-effect path, `fetchTestMappings` `p.warn()` graceful-degradation path (design D3), and `internal/protocol/` (transport layer stays usage-agnostic). Added table-driven `TestCallAndUnmarshal` (`call_test.go`, `package adapter` internal) with 5 subtests (success, transport error, protocol error, unmarshal failure, generic instantiation across a second result type) driven through the fake analyzer binary; the internal test builds its own fake-analyzer copy lazily via `sync.Once` because only one `TestMain` is allowed per test binary (the existing one lives in `package adapter_test`). Split from #201; closes #237.
- quality-empty-results-gate: Fixed silent exit-0 bug (#103) when `gaze quality` encounters Ginkgo/BDD suites or packages where no test functions can be resolved to targets. Added `SkippedTests int` and `SkippedTestNames []string` fields to `taxonomy.PackageSummary` (`internal/taxonomy/types.go`). `quality.Assess` (`internal/quality/quality.go`) now counts and names skipped test functions at the `len(targets) == 0` continue site. `runQuality` (`cmd/gaze/main.go`) now prints structured stdout summary when `allReports` is empty (total test count, skipped names truncated at 20, `--target` hint), returns error when `--min-contract-coverage` or `--max-over-specification` thresholds are set (quality gate failure), and produces valid JSON for `--format=json`. Added `writeSkippedTests` section to `quality.WriteText` (`internal/quality/report.go`) with 20-name truncation. Updated `mergeSummaries` to aggregate skipped test data. Updated JSON Schema (`internal/report/schema.go`). Propagated through report pipeline: `runQualityForPackage` now returns 3 values, `qualityStepResult.SkippedTests`, `ReportSummary.SkippedTests`, `compactSummary.SkippedTests`, `compactPackageSummary.SkippedTests`/`SkippedTestNames`. Added `testdata/src/bddstyle/` fixture. 13 new tests across 3 packages. Closes #103.
- crapload-add-tests-pr2c: Added test coverage for 3 high-CRAP functions to reduce CRAPload from 29 to 26. `ResolvePackagePaths` (CRAP 81.6→10.1): removed `testing.Short()` guards from 5 existing tests (packages.NeedName is lightweight), added 2 new tests for nil-stderr and dedup branches. `BuildContractCoverageFunc` (CRAP 56.0→2.0): added `buildContractCoverageFuncDeps` DI struct with 4 injectable fields following the existing `contractCoverageDeps` pattern, added `buildContractCoverageFuncImpl` wrapper, added 7 synthetic unit tests covering all 7 code paths. `loadStreaming` (CRAP 42.0→12.0): extracted `parseSideEffectStream` helper from JSONL scanner loop, added 6 unit tests using synthetic `bufio.Scanner` from `bytes.Reader`. Final phase (2c) of issue #166.
- crapload-decompose-pr2b: Decomposed three highest-complexity functions in `internal/quality/mapping.go`: `matchContainerUnwrap` (50→8), `isTransformationCall` (26→5), `matchAssertionToEffect` (25→5). Extracted 7+ helpers: `isByteLikeParam`, `isPointerDestParam`, `resolveCallSignature`, `findParamIndex` (from isTransformationCall); `matchDirect`, `matchIndirectRoot` (from matchAssertionToEffect); `collectTrackedVars`, `traceForwardDataFlow`, `matchTrackedInExpr` (from matchContainerUnwrap). Added 25 new unit tests using synthetic AST via `parseAndTypeCheck`. Filled 3 test gaps in `isTransformationCall` (io.Reader, empty interface, mixed ordering). `TestSC003_MappingAccuracy` ratchet (85.0%) passes unchanged. Phase 2b of issue #166.
Expand Down
55 changes: 55 additions & 0 deletions internal/adapter/call.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package adapter

import (
"context"
"encoding/json"
"fmt"

"github.com/unbound-force/gaze/internal/protocol"
)

// callAndUnmarshal issues a JSON-RPC call for method with params on
// client, checks the transport and protocol errors, and unmarshals the
// raw result into a value of type T.
//
// It centralizes the Call -> transport-error check -> protocol-error
// check -> json.Unmarshal sequence that every batch provider adapter
// would otherwise open-code. All three failure paths wrap the error
// with per-method context derived from method so operators can
// distinguish, for example, a "complexity" failure from a "coverage"
// failure in logs.
//
// Error-chain contract (design D6): transport and unmarshal errors wrap
// the underlying error with %w, so errors.Is / errors.As unwrapping is
// preserved. Protocol errors are formatted with %s because resp.Error
// is a structured JSON-RPC error object carrying a message and code,
// not a Go error chain value.
//
// On any error the zero value of T is returned alongside the wrapped
// error.
func callAndUnmarshal[T any](
ctx context.Context,
client *protocol.Client,
method string,
params any,
) (T, error) {
var result T

// method is the wire method constant (e.g. protocol.MethodComplexity
// == "complexity"). Reusing it as the error prefix is intentional: it
// reproduces the exact legacy per-method log strings the migrated call
// sites emitted before this helper existed.
resp, err := client.Call(ctx, method, params)
if err != nil {
return result, fmt.Errorf("%s protocol call: %w", method, err)
}
if resp.Error != nil {
return result, fmt.Errorf("%s protocol error: %s (code %d)", method, resp.Error.Message, resp.Error.Code)
}

if err := json.Unmarshal(resp.Result, &result); err != nil {
return result, fmt.Errorf("parsing %s result: %w", method, err)
}

return result, nil
}
Loading
Loading