Skip to content

[test] Add tests for config.AllowOnlyPolicy.UnmarshalJSON#7639

Merged
lpcox merged 1 commit into
mainfrom
test/coverage-allow-only-unmarshal-error-paths-fe262a4a0ad3ca04
Jun 16, 2026
Merged

[test] Add tests for config.AllowOnlyPolicy.UnmarshalJSON#7639
lpcox merged 1 commit into
mainfrom
test/coverage-allow-only-unmarshal-error-paths-fe262a4a0ad3ca04

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Coverage Improvement: AllowOnlyPolicy.UnmarshalJSON

Function Analyzed

  • Package: internal/config
  • Function: (*AllowOnlyPolicy).UnmarshalJSON
  • Previous Coverage: 91.9%
  • New Coverage: 97.3%
  • Complexity: High (switch statement with 12 cases, each with error handling)

Why This Function?

AllowOnlyPolicy.UnmarshalJSON is the most complex under-tested function in the testable packages. It manually parses JSON for a security-critical policy type using a 12-case switch, validating each field's type individually. Despite most error paths being covered, two branches for promotion-label and demotion-label were completely uncovered — fields added later that weren't added to the existing error-path test table.

Tests Added

Two new table-driven cases in TestAllowOnlyPolicyUnmarshalJSON_FieldErrorPaths:

  • promotion-label field rejects non-string JSON values (e.g. numeric 123)
  • demotion-label field rejects non-string JSON values (e.g. array [1,2,3])

These follow the exact same pattern as the nine existing error-path cases for min-integrity, blocked-users, tool-call-limits, approval-labels, trusted-users, endorsement-reactions, disapproval-reactions, disapproval-integrity, and endorser-min-integrity.

Coverage Report

Before: 91.9% (AllowOnlyPolicy.UnmarshalJSON)
After:  97.3% (AllowOnlyPolicy.UnmarshalJSON)
Improvement: +5.4%

The remaining 2.7% is the repos error branch at line 191 — unreachable dead code because json.Unmarshal into interface{} never fails on valid JSON input (and all values come from map[string]json.RawMessage so they are always valid).

Test Execution

=== RUN   TestAllowOnlyPolicyUnmarshalJSON_FieldErrorPaths
    --- PASS: .../promotion-label_field_invalid_JSON_type (0.00s)
    --- PASS: .../demotion-label_field_invalid_JSON_type (0.00s)
--- PASS: TestAllowOnlyPolicyUnmarshalJSON_FieldErrorPaths (0.00s)
ok  	github.com/github/gh-aw-mcpg/internal/config	1.677s

Generated by Test Coverage Improver
Next run will target the next most complex under-tested function

Warning

Firewall blocked 7 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
  • 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"
    - "sum.golang.org"

See Network Configuration for more information.

Generated by Test Coverage Improver · 2.6K AIC · ⊞ 28.7K ·

…ion-label error paths

Cover the two remaining uncovered error branches in AllowOnlyPolicy.UnmarshalJSON:
- promotion-label field rejects non-string JSON values (e.g. numbers)
- demotion-label field rejects non-string JSON values (e.g. arrays)

These are added as additional table-driven cases in the existing
TestAllowOnlyPolicyUnmarshalJSON_FieldErrorPaths test, consistent with
the pattern used for the other nine field error paths.

Coverage for AllowOnlyPolicy.UnmarshalJSON improves from 91.9% to 97.3%.
The remaining 2.7% is the repos error branch, which is unreachable dead code
because json.Unmarshal into interface{} never fails on valid JSON input.

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

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 unit test coverage for the security-critical internal/config.(*AllowOnlyPolicy).UnmarshalJSON parser by adding missing error-path coverage for two recently added fields.

Changes:

  • Add a table-driven error-path test case ensuring promotion-label rejects non-string JSON values.
  • Add a table-driven error-path test case ensuring demotion-label rejects non-string JSON values.
Show a summary per file
File Description
internal/config/guard_policy_unmarshal_coverage_test.go Extends TestAllowOnlyPolicyUnmarshalJSON_FieldErrorPaths with new cases covering invalid JSON types for promotion-label and demotion-label.

Copilot's findings

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 0

@lpcox lpcox merged commit cc95c11 into main Jun 16, 2026
24 checks passed
@lpcox lpcox deleted the test/coverage-allow-only-unmarshal-error-paths-fe262a4a0ad3ca04 branch June 16, 2026 18:42
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.

2 participants