Skip to content

[test] Add tests for schema compilation and guard policy error paths#7112

Merged
lpcox merged 4 commits into
mainfrom
test/coverage-improve-schema-guard-policy-0fc3812a499be615
Jun 6, 2026
Merged

[test] Add tests for schema compilation and guard policy error paths#7112
lpcox merged 4 commits into
mainfrom
test/coverage-improve-schema-guard-policy-0fc3812a499be615

Conversation

@github-actions

@github-actions github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Test Coverage Improvement

Functions Analyzed

Function Package Before After Δ
getOrCompileSchema internal/config 56.0% 92.0% +36 pp
validateJSONSchema internal/config 89.5% 100% +10.5 pp
fixSchemaBytes internal/config 92.8% 98.6% +5.8 pp
ValidateWriteSinkPolicy internal/config 100%
NormalizeGuardPolicy internal/config 100%
normalizeToolCallLimits internal/config 90.9% 100% +9.1 pp
validateAgainstCustomSchema internal/config 82.8% 86.2% +3.4 pp

Why These Functions?

getOrCompileSchema had the lowest coverage (56%) of all non-trivial functions in the testable packages. It uses sync.Once to cache schema compilation on first use, making its error branches (inside the Do() callback) unreachable from normal test paths. The function is critical — a bug here would silently break all JSON Schema validation on startup.

fixSchemaBytes had four uncovered log/skip branches in the customServerConfig.type fixing logic, and validateJSONSchema had two unreached defensive-error branches.

The guard policy functions (ValidateWriteSinkPolicy, NormalizeGuardPolicy, normalizeToolCallLimits) had small gaps in empty-input and error-propagation paths.

Tests Added

schema_error_paths_test.go (new file):

  • fixSchemaBytes — type field with no pattern key
  • fixSchemaBytes — type field pattern is non-string
  • fixSchemaBytes — type field pattern has no negative lookahead
  • fixSchemaBytes — type field value is not a map object
  • getOrCompileSchemafixSchemaBytes failure (invalid JSON in embedded schema)
  • getOrCompileSchema — schema without $id uses fallback embeddedSchemaID
  • getOrCompileSchemaAddResource failure (malformed percent-encoded URL)
  • getOrCompileSchemaCompile failure (duplicate canonical URIs in definitions)
  • validateJSONSchema — invalid JSON config input
  • validateJSONSchema — schema compilation error propagation

guard_policy_coverage_test.go (new file):

  • ValidateWriteSinkPolicy — empty/whitespace accept entry
  • NormalizeGuardPolicy[]string Repos case with invalid scopes
  • normalizeToolCallLimits — empty/whitespace-only tool name keys

validate_against_custom_schema_test.go (extended):

  • validateAgainstCustomSchema — cache hit with unexpected non-schema type

Implementation Notes

Tests for getOrCompileSchema error paths reset the sync.Once and package-level cache variables with t.Cleanup restoration. This is safe because Go runs tests within a package sequentially by default (no t.Parallel() is used).

The remaining uncovered lines are dead code (e.g., json.Marshal on a map[string]interface{} cannot fail; json.Unmarshal after fixSchemaBytes always succeeds; io.ReadAll failures over in-memory readers are impossible).

Test Execution

ok  github.com/github/gh-aw-mcpg/internal/config   1.709s
ok  github.com/github/gh-aw-mcpg/internal/config/rules   0.006s

All tests pass. No production code was modified.


Generated by Test Coverage Improver

Warning

Firewall blocked 8 domains

The following domains were blocked by the firewall during workflow execution:

  • go.opentelemetry.io
  • go.yaml.in
  • golang.org
  • google.golang.org
  • gopkg.in
  • proxy.golang.org
  • releaseassets.githubusercontent.com
  • sum.golang.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "go.opentelemetry.io"
    - "go.yaml.in"
    - "golang.org"
    - "google.golang.org"
    - "gopkg.in"
    - "proxy.golang.org"
    - "releaseassets.githubusercontent.com"
    - "sum.golang.org"

See Network Configuration for more information.

Generated by Test Coverage Improver · sonnet46 18.4M ·

Improve test coverage for the most complex under-tested functions in
internal/config by adding comprehensive tests for error branches that
were previously unreachable via existing test paths.

Coverage improvements:
- getOrCompileSchema: 56% → 92% (+36 pp)
  - fixSchemaBytes failure path
  - schema without $id uses fallback embeddedSchemaID
  - AddResource failure (invalid percent-encoded URL)
  - Compile failure (duplicate canonical URI in definitions)
- validateJSONSchema: 89.5% → 100% (+10.5 pp)
  - getOrCompileSchema error propagation
  - invalid JSON config data
- fixSchemaBytes: 92.8% → 98.6% (+5.8 pp)
  - type field with no pattern key
  - type field with non-string pattern value
  - type field with pattern that has no negative lookahead
  - typeValue that is not a map[string]interface{}
- ValidateWriteSinkPolicy: → 100%
  - empty/whitespace accept entry
- NormalizeGuardPolicy: → 100%
  - []string Repos case error path
- normalizeToolCallLimits: 90.9% → 100% (+9.1 pp)
  - empty/whitespace tool name key
- validateAgainstCustomSchema: 82.8% → 86.2% (+3.4 pp)
  - cache hit with wrong (non-schema) type

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 6, 2026 20:46
Copilot AI review requested due to automatic review settings June 6, 2026 20:46
@lpcox

lpcox commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves test coverage in internal/config by adding/expanding unit tests that exercise previously-uncovered error/edge branches in JSON schema compilation/validation and guard policy normalization/validation logic.

Changes:

  • Added schema_error_paths_test.go to cover fixSchemaBytes, getOrCompileSchema, and validateJSONSchema error/defensive branches.
  • Added guard_policy_coverage_test.go to cover empty-input and error-propagation branches in guard policy validation/normalization.
  • Extended validate_against_custom_schema_test.go to cover the “cache hit but wrong cached type” branch.
Show a summary per file
File Description
internal/config/validate_against_custom_schema_test.go Adds coverage for unexpected (non-schema) values stored in the custom schema cache.
internal/config/schema_error_paths_test.go New tests for schema fixing/compilation/validation error paths to raise coverage.
internal/config/guard_policy_coverage_test.go New tests for guard policy validation/normalization edge cases.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 3

Comment thread internal/config/schema_error_paths_test.go Outdated
Comment thread internal/config/schema_error_paths_test.go Outdated
Comment thread internal/config/validate_against_custom_schema_test.go Outdated
lpcox and others added 3 commits June 6, 2026 13:53
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

@copilot fix failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/27068675226/job/79906620380?pr=7112

Fixed on the branch in 122e405, 293b461, and 8016e31. I rechecked the current tip locally with make lint and go test ./internal/config/..., and both pass.

@lpcox lpcox merged commit 5e85d73 into main Jun 6, 2026
16 checks passed
@lpcox lpcox deleted the test/coverage-improve-schema-guard-policy-0fc3812a499be615 branch June 6, 2026 20:55
Copilot finished work on behalf of lpcox June 6, 2026 20:55
Copilot AI requested a review from lpcox June 6, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants