Consolidate request validation parity assertions
This commit is contained in:
@@ -5,9 +5,10 @@ Reviewed commits `f315620`, `f3148bf`, and `60784e7` against the finalized
|
||||
|
||||
## 1. Reuse The Existing Preparation-Parity Test Helper
|
||||
|
||||
Priority: P3 (test maintainability). Status: open.
|
||||
Priority: P3 (test maintainability). Status: complete.
|
||||
|
||||
In [validate_request_contract_test.go](../../validate_request_contract_test.go),
|
||||
At reviewed commit `60784e7`, in
|
||||
[validate_request_contract_test.go](../../validate_request_contract_test.go),
|
||||
`TestValidateRequestMatchesPrepareContract` defines a local `testParity` closure
|
||||
at lines 25–45 that duplicates the package-level
|
||||
`assertValidateRequestParity` helper at lines 87–108. Both invoke `Prepare` and
|
||||
@@ -31,6 +32,16 @@ This is a small cleanup, not a demonstrated functional or release-blocking bug.
|
||||
Verify with `go test -race . -run '^TestValidateRequest'` and
|
||||
`git diff --check`, formatting the changed Go file.
|
||||
|
||||
### Completion
|
||||
|
||||
Removed the local closure and reused `assertValidateRequestParity` for all four
|
||||
cases, including the nil-engine `ErrInvalidConfig` expectation. Existing cases,
|
||||
public-error assertions, and no-model-call checks are preserved; production code
|
||||
and public contracts are unchanged.
|
||||
|
||||
Formatted the Go file. The uncached, race-enabled `TestValidateRequest` tests
|
||||
and `git diff --check` passed.
|
||||
|
||||
## Review Outcome
|
||||
|
||||
The public method and shared facade helper match the intended architecture.
|
||||
|
||||
@@ -21,43 +21,16 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
|
||||
return requirementsEngine(t, oneVersionPrompt("synthetic", "v1", definition), profiles,
|
||||
promptkit.WithLLMClient(countingLLMClient{calls: &modelCalls}))
|
||||
}
|
||||
testParity := func(t *testing.T, engine *promptkit.Engine, request promptkit.RunRequest, want error) {
|
||||
t.Helper()
|
||||
prepared, prepareErr := engine.Prepare(context.Background(), request)
|
||||
validateErr := engine.ValidateRequest(context.Background(), request)
|
||||
if want == nil {
|
||||
if prepareErr != nil || prepared == nil {
|
||||
t.Fatalf("Prepare() = (%+v, %v), want a prepared result", prepared, prepareErr)
|
||||
}
|
||||
if validateErr != nil {
|
||||
t.Fatalf("ValidateRequest() error = %v, want nil", validateErr)
|
||||
}
|
||||
return
|
||||
}
|
||||
if prepared != nil || !errors.Is(prepareErr, want) {
|
||||
t.Fatalf("Prepare() = (%+v, %v), want error matching %v", prepared, prepareErr, want)
|
||||
}
|
||||
if !errors.Is(validateErr, want) {
|
||||
t.Fatalf("ValidateRequest() error = %v, want error matching %v", validateErr, want)
|
||||
}
|
||||
}
|
||||
|
||||
t.Run("successful request", func(t *testing.T) {
|
||||
engine := newEngine(t, contractPrompt("offline", "hello", nil, "", noValidationOutput))
|
||||
testParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, nil)
|
||||
assertValidateRequestParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, nil)
|
||||
})
|
||||
|
||||
t.Run("nil engine", func(t *testing.T) {
|
||||
var engine *promptkit.Engine
|
||||
request := promptkit.RunRequest{PromptID: "synthetic"}
|
||||
prepared, prepareErr := engine.Prepare(context.Background(), request)
|
||||
validateErr := engine.ValidateRequest(context.Background(), request)
|
||||
if prepared != nil || !errors.Is(prepareErr, promptkit.ErrInvalidConfig) {
|
||||
t.Fatalf("Prepare() = (%+v, %v), want ErrInvalidConfig", prepared, prepareErr)
|
||||
}
|
||||
if !errors.Is(validateErr, promptkit.ErrInvalidConfig) {
|
||||
t.Fatalf("ValidateRequest() error = %v, want ErrInvalidConfig", validateErr)
|
||||
}
|
||||
assertValidateRequestParity(t, engine, request, promptkit.ErrInvalidConfig)
|
||||
})
|
||||
|
||||
t.Run("invalid appended message", func(t *testing.T) {
|
||||
@@ -68,7 +41,7 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
|
||||
Role: "unsupported-role", Content: "not sent to a model",
|
||||
}},
|
||||
}
|
||||
testParity(t, engine, request, promptkit.ErrInvalidRequest)
|
||||
assertValidateRequestParity(t, engine, request, promptkit.ErrInvalidRequest)
|
||||
})
|
||||
|
||||
t.Run("missing required input", func(t *testing.T) {
|
||||
@@ -76,7 +49,7 @@ func TestValidateRequestMatchesPrepareContract(t *testing.T) {
|
||||
map[string]any{"name": "required", "required": true},
|
||||
}, "", noValidationOutput)
|
||||
engine := newEngine(t, definition)
|
||||
testParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, promptkit.ErrPromptRender)
|
||||
assertValidateRequestParity(t, engine, promptkit.RunRequest{PromptID: "synthetic"}, promptkit.ErrPromptRender)
|
||||
})
|
||||
|
||||
if got := modelCalls.Load(); got != 0 {
|
||||
|
||||
Reference in New Issue
Block a user