Report known zero usage when smoke tests do not dispatch
This commit is contained in:
@@ -1,6 +1,6 @@
|
|||||||
# Smoke Test Implementation Review Findings
|
# Smoke Test Implementation Review Findings
|
||||||
|
|
||||||
Status: two findings completed; one open. Review completed 2026-10-13.
|
Status: all three findings completed 2026-10-13. Review completed 2026-10-13.
|
||||||
|
|
||||||
Reviewed implementation commits after `8fbd058` through
|
Reviewed implementation commits after `8fbd058` through
|
||||||
`2f735aedf9b5dcc98164292db46ae1eab3e15cde`, against the
|
`2f735aedf9b5dcc98164292db46ae1eab3e15cde`, against the
|
||||||
@@ -101,7 +101,7 @@ passed for the facade, model client, generation, use case, and engine packages.
|
|||||||
|
|
||||||
## Finding 3: Initialize Zero-Dispatch Usage Consistently
|
## Finding 3: Initialize Zero-Dispatch Usage Consistently
|
||||||
|
|
||||||
Priority: P3. Status: open.
|
Priority: P3. Status: completed 2026-10-13.
|
||||||
|
|
||||||
Location: [`SmokeTester.Run`](../../internal/usecase/smoke_tester.go),
|
Location: [`SmokeTester.Run`](../../internal/usecase/smoke_tester.go),
|
||||||
lines 52–87, and its early return paths.
|
lines 52–87, and its early return paths.
|
||||||
@@ -130,6 +130,14 @@ inference and an inapplicable/unsupported operation, reusing accumulator coverag
|
|||||||
for arithmetic. Existing coordinator tests already cover correct zero-dispatch
|
for arithmetic. Existing coordinator tests already cover correct zero-dispatch
|
||||||
accounting after the generation boundary is entered.
|
accounting after the generation boundary is entered.
|
||||||
|
|
||||||
|
Completion: report finalization now applies the shared zero-attempt accumulator
|
||||||
|
whenever no attempt was dispatched, replacing scattered early-return assignments.
|
||||||
|
Unsupported/inapplicable operations and early cancellation return known, complete
|
||||||
|
zero usage; actual dispatches retain their reported or unknown usage unchanged.
|
||||||
|
The strengthened orchestration assertions failed before the fix. Final validation
|
||||||
|
passed with `GOWORK=off`: `go test -race ./...`, `go vet ./...`, the three offline
|
||||||
|
consumer examples, and `git diff --check`.
|
||||||
|
|
||||||
## Review Validation
|
## Review Validation
|
||||||
|
|
||||||
Passed with `GOWORK=off`:
|
Passed with `GOWORK=off`:
|
||||||
@@ -141,7 +149,8 @@ Passed with `GOWORK=off`:
|
|||||||
The three findings were reproduced through the public API using a temporary
|
The three findings were reproduced through the public API using a temporary
|
||||||
external Go program and a controlled HTTP RoundTripper; no live provider was
|
external Go program and a controlled HTTP RoundTripper; no live provider was
|
||||||
contacted and no reproduction source was added to the repository. The passing
|
contacted and no reproduction source was added to the repository. The passing
|
||||||
suite does not currently cover these cases.
|
suite at the reviewed revision did not cover these cases; the completed
|
||||||
|
remediations add the corresponding regression coverage.
|
||||||
|
|
||||||
Both catalogs remain selected at locally validated `v3.20261013.0` candidates.
|
Both catalogs remain selected at locally validated `v3.20261013.0` candidates.
|
||||||
Their publication and any publication-date re-pinning remain separate release
|
Their publication and any publication-date re-pinning remain separate release
|
||||||
|
|||||||
@@ -57,6 +57,12 @@ func (t *SmokeTester) Run(ctx context.Context, request domain.SmokeTestRequest)
|
|||||||
}
|
}
|
||||||
finish := func(err error) (*domain.SmokeTestReport, error) {
|
finish := func(err error) (*domain.SmokeTestReport, error) {
|
||||||
report.Timing.Duration = nonnegativeDuration(now().UTC().Sub(startedAt))
|
report.Timing.Duration = nonnegativeDuration(now().UTC().Sub(startedAt))
|
||||||
|
for _, attempt := range report.Attempts {
|
||||||
|
if attempt.Dispatched {
|
||||||
|
return report, err
|
||||||
|
}
|
||||||
|
}
|
||||||
|
report.Usage = noDispatchUsage()
|
||||||
return report, err
|
return report, err
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -163,7 +169,6 @@ func (t *SmokeTester) runInference(
|
|||||||
if errors.Is(err, ErrAPIKeyRequired) || errors.Is(err, ErrAPIKeyEnvMissing) {
|
if errors.Is(err, ErrAPIKeyRequired) || errors.Is(err, ErrAPIKeyEnvMissing) {
|
||||||
report.Reason = domain.SmokeTestReasonRequiredCredentialMissing
|
report.Reason = domain.SmokeTestReasonRequiredCredentialMissing
|
||||||
report.Failure = domain.ExecutionFailureInvalidRequest
|
report.Failure = domain.ExecutionFailureInvalidRequest
|
||||||
report.Usage = noDispatchUsage()
|
|
||||||
report.Completed = true
|
report.Completed = true
|
||||||
return finish(nil)
|
return finish(nil)
|
||||||
}
|
}
|
||||||
@@ -187,13 +192,11 @@ func (t *SmokeTester) runInference(
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
if contextErr := ctx.Err(); contextErr != nil {
|
if contextErr := ctx.Err(); contextErr != nil {
|
||||||
report.Failure = generation.ClassifyFailure(contextErr)
|
report.Failure = generation.ClassifyFailure(contextErr)
|
||||||
report.Usage = noDispatchUsage()
|
|
||||||
return finish(contextErr)
|
return finish(contextErr)
|
||||||
}
|
}
|
||||||
var capacityErr *CapacityError
|
var capacityErr *CapacityError
|
||||||
if errors.As(err, &capacityErr) {
|
if errors.As(err, &capacityErr) {
|
||||||
report.Failure = domain.ExecutionFailureCapacity
|
report.Failure = domain.ExecutionFailureCapacity
|
||||||
report.Usage = noDispatchUsage()
|
|
||||||
report.Completed = true
|
report.Completed = true
|
||||||
return finish(nil)
|
return finish(nil)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -184,7 +184,8 @@ func TestSmokeTesterReportsUnsupportedAndInapplicableAuthentication(t *testing.T
|
|||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if report.Supported != tc.supported || report.Reason != tc.reason || !report.Completed ||
|
if report.Supported != tc.supported || report.Reason != tc.reason || !report.Completed ||
|
||||||
!sameBool(report.Applicable, tc.applicable) || len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) {
|
!sameBool(report.Applicable, tc.applicable) || len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) ||
|
||||||
|
report.Usage != noDispatchUsage() {
|
||||||
t.Fatalf("report = %+v", report)
|
t.Fatalf("report = %+v", report)
|
||||||
}
|
}
|
||||||
})
|
})
|
||||||
@@ -201,7 +202,7 @@ func TestSmokeTesterReportsReachabilityUnavailableWithoutHTTPClient(t *testing.T
|
|||||||
}
|
}
|
||||||
if report.Supported || report.Applicable != nil || !report.Completed ||
|
if report.Supported || report.Applicable != nil || !report.Completed ||
|
||||||
report.Reason != domain.SmokeTestReasonHTTPDiagnosticsUnavailable ||
|
report.Reason != domain.SmokeTestReasonHTTPDiagnosticsUnavailable ||
|
||||||
len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) {
|
len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) || report.Usage != noDispatchUsage() {
|
||||||
t.Fatalf("unsupported reachability report = %+v", report)
|
t.Fatalf("unsupported reachability report = %+v", report)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -226,14 +227,16 @@ func TestSmokeTesterSelectionAndCancellationDoNotDispatch(t *testing.T) {
|
|||||||
|
|
||||||
ctx, cancel := context.WithCancel(context.Background())
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
cancel()
|
cancel()
|
||||||
report, err := tester.Run(ctx, domain.SmokeTestRequest{
|
for _, kind := range []domain.SmokeTestKind{domain.SmokeTestReachability, domain.SmokeTestTextInference} {
|
||||||
Kind: domain.SmokeTestReachability, BackendID: "alpha",
|
report, err := tester.Run(ctx, domain.SmokeTestRequest{
|
||||||
})
|
Kind: kind, BackendID: "alpha",
|
||||||
if !errors.Is(err, context.Canceled) || report == nil || report.Completed || report.Failure != domain.ExecutionFailureCanceled {
|
})
|
||||||
t.Fatalf("canceled report=%+v error=%v", report, err)
|
if !errors.Is(err, context.Canceled) || report == nil || report.Completed || report.Failure != domain.ExecutionFailureCanceled {
|
||||||
}
|
t.Fatalf("canceled report=%+v error=%v", report, err)
|
||||||
if report.Facts != (domain.SmokeTestFacts{}) || len(report.Attempts) != 0 || calls != 0 {
|
}
|
||||||
t.Fatalf("cancellation dispatched or invented facts: %+v calls=%d", report, calls)
|
if report.Facts != (domain.SmokeTestFacts{}) || len(report.Attempts) != 0 || calls != 0 || report.Usage != noDispatchUsage() {
|
||||||
|
t.Fatalf("cancellation dispatched, invented facts, or left usage unknown: %+v calls=%d", report, calls)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user