Conversation
…beledData These two functions in internal/guard/wasm.go were completely untested despite having high cyclomatic complexity: - parseResourceResponse: 8 branches across resource map validation, description/secrecy/integrity tag parsing, and operation type switching - parseCollectionLabeledData: 11+ branches with nested type assertions, array iteration, and per-item label parsing New test file covers: - Happy paths: full labels, partial labels, empty labels - All four operation types (read, write, read-write, default) - Non-string tags silently skipped in secrecy/integrity arrays - Non-map item entries silently skipped in collection - Non-array secrecy/integrity values default to empty labels - Non-string description fields gracefully ignored - Missing/nil/wrong-type resource key returns error - Empty and nil items slices produce empty collections - Multiple items with mixed label configurations - LabeledData interface contract validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds comprehensive unit tests for the WASM guard response parsing helpers in internal/guard, improving coverage for logic used on the WasmGuard.LabelResponse hot path.
Changes:
- Introduces extensive table-driven tests for
parseResourceResponse, including operation parsing and type-safety cases. - Adds extensive table-driven tests for
parseCollectionLabeledData, including mixed item types, label parsing behavior, and order preservation. - Adds interface/return-type sanity checks for returned labeled data structures.
Comments suppressed due to low confidence (1)
internal/guard/wasm_response_parse_test.go:308
- This test function name duplicates an existing
TestParseCollectionLabeledDataininternal/guard/guard_test.go(around line 942). The package will not compile with both present. Rename this test (e.g.,TestParseCollectionLabeledData_Comprehensive) or consolidate with the existing one.
func TestParseCollectionLabeledData(t *testing.T) {
tests := []struct {
name string
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // ─── parseResourceResponse ──────────────────────────────────────────────────── | ||
|
|
||
| func TestParseResourceResponse(t *testing.T) { |
There was a problem hiding this comment.
This test function name duplicates an existing TestParseResourceResponse in internal/guard/guard_test.go (around line 802). Go will fail to compile with duplicate declarations in the same package. Rename this test (e.g., TestParseResourceResponse_Comprehensive) or remove/merge the older test so only one remains.
This issue also appears on line 306 of the same file.
| func TestParseResourceResponse(t *testing.T) { | |
| func TestParseResourceResponse_WASM(t *testing.T) { |
| tests := []struct { | ||
| name string | ||
| items []interface{} | ||
| wantItemCount int | ||
| wantErr bool | ||
| checkItems func(t *testing.T, collection *difc.CollectionLabeledData) | ||
| }{ |
There was a problem hiding this comment.
wantErr is never set to true in this table, and parseCollectionLabeledData currently never returns an error. Consider removing wantErr (and the associated branch) to reduce noise, or add explicit error cases if you expect the function to start returning errors later.
Test Coverage Improvement
Functions Analyzed
internal/guardparseResourceResponseandparseCollectionLabeledDataWhy These Functions?
Both functions are called in the hot path of the
LabelResponsemethod onWasmGuard, which is invoked for every tool response that passes through a WASM security guard. Despite their centrality, they had zero direct test coverage. They contain complex branching logic (type assertions, nested maps, array iteration with per-element type checks) that is easy to break silently.Tests Added (
internal/guard/wasm_response_parse_test.go)TestParseResourceResponse— 20 table-driven cases:"read","write","read-write", and missing/unknown (default to write)"READ"defaults to writeresourcekey → errorresourcevalue (string, array, number, nil) → errorTestParseResourceResponse_OperationCoverage— 6 focused cases covering every operation branchTestParseCollectionLabeledData— 22 table-driven cases:labelskey →Labelsfield isnillabelsvalue →Labelsfield isnillabelsmap → default empty secrecy/integrity labelsdatafield variety: nil, string, nested map, missing keyTestParseCollectionLabeledData_CollectionInterface— verifies*CollectionLabeledDatasatisfiesdifc.LabeledDatainterface (compile-time + runtime contract)TestParseResourceResponse_ReturnTypeSanity— verifiesContains()lookups on returned labelsCoverage Impact
Generated by Test Coverage Improver
Next run will target the next most complex under-tested function