Skip to content

[test] Add tests for config.unmarshalStringListOrExpression#7895

Merged
lpcox merged 1 commit into
mainfrom
add-tests-unmarshal-string-list-expr-e27772db5ba386dc
Jun 21, 2026
Merged

[test] Add tests for config.unmarshalStringListOrExpression#7895
lpcox merged 1 commit into
mainfrom
add-tests-unmarshal-string-list-expr-e27772db5ba386dc

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Coverage Improvement: unmarshalStringListOrExpression

Function Analyzed

  • Package: internal/config
  • Function: unmarshalStringListOrExpression (and related helpers in guard_policy.go)
  • Previous Coverage: unmarshalStringListOrExpression 89.5%, GuardPolicy.UnmarshalJSON 0%, GuardPolicyToMap 0%
  • New Coverage: All three functions at 100%
  • Complexity: Medium — two early-return error branches, delimiter splitting with trim-and-filter loop

Why This Function?

unmarshalStringListOrExpression had two untested error paths that could not be reached by existing tests:

  1. Line 289 — triggered when the JSON value is neither a []string array nor a string (e.g. a JSON number, boolean, or object). No existing test passed such a value through refusal-labels.
  2. Line 296 — triggered when the expression string consists entirely of delimiter characters (comma or newline), so strings.FieldsFunc returns zero parts. The existing test for " " (spaces only) skipped this branch because spaces are not delimiters; it fell through to the second empty-result check instead.

GuardPolicy.UnmarshalJSON and GuardPolicyToMap also had uncovered error paths (non-object JSON input, typed-nil-pointer marshalling to null) that are now covered.

Tests Added

New file: internal/config/guard_policy_str_expr_test.go

  • TestUnmarshalStringListOrExpression (15 table-driven cases)
    • Happy path: JSON array, empty array, comma-delimited string, newline-delimited string, mixed delimiters with whitespace, single label
    • Error path (line 289): JSON number, boolean, object, array-of-integers
    • Error path (line 296): comma-only string, newline-only string, multiple-commas-only
    • Error path (line 308): spaces-around-commas, whitespace-only string
  • TestGuardPolicyUnmarshalJSON_NonObjectJSON (3 cases) — covers line 100–102
  • TestGuardPolicyToMap_TypedNilPointer (1 case) — covers line 175–177 via typed nil marshalling to null
  • TestAllowOnlyPolicyUnmarshalJSON_RefusalLabelsInvalidType (5 cases) — exercises both new error paths end-to-end through AllowOnlyPolicy.UnmarshalJSON

Coverage Report

Before:
  unmarshalStringListOrExpression   89.5%
  GuardPolicy.UnmarshalJSON          0.0%   (line 100-102 uncovered)
  GuardPolicyToMap                   0.0%   (line 175-177 uncovered)

After:
  unmarshalStringListOrExpression  100.0%
  GuardPolicy.UnmarshalJSON        100.0%
  GuardPolicyToMap                 100.0%

Test Execution

All 24 new sub-tests pass:

--- PASS: TestUnmarshalStringListOrExpression (0.00s)
--- PASS: TestGuardPolicyUnmarshalJSON_NonObjectJSON (0.00s)
--- PASS: TestGuardPolicyToMap_TypedNilPointer (0.00s)
--- PASS: TestAllowOnlyPolicyUnmarshalJSON_RefusalLabelsInvalidType (0.00s)
ok  github.com/github/gh-aw-mcpg/internal/config  1.633s

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 · 3.2K AIC · ⊞ 28.8K ·

Cover the two previously untested error paths in unmarshalStringListOrExpression
(guard_policy.go), along with related error paths in GuardPolicy.UnmarshalJSON
and GuardPolicyToMap:

- Line 289: JSON value that is neither array nor string (number, bool, object)
  triggers 'expected array of strings or comma/newline-delimited expression'
- Line 296: string containing only delimiter characters (comma/newline) produces
  zero FieldsFunc parts, triggering 'must include at least one label'
- GuardPolicy.UnmarshalJSON line 100-102: non-object top-level JSON
- GuardPolicyToMap line 175-177: typed nil pointer marshals to JSON null

New test functions:
- TestUnmarshalStringListOrExpression (15 table-driven cases)
- TestGuardPolicyUnmarshalJSON_NonObjectJSON (3 cases)
- TestGuardPolicyToMap_TypedNilPointer (1 case)
- TestAllowOnlyPolicyUnmarshalJSON_RefusalLabelsInvalidType (5 cases)

Coverage improvements:
- unmarshalStringListOrExpression: 89.5% -> 100%
- GuardPolicy.UnmarshalJSON:        0%    -> 100%
- GuardPolicyToMap:                 0%    -> 100%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 21, 2026 17:36
Copilot AI review requested due to automatic review settings June 21, 2026 17:36
@lpcox lpcox merged commit e0b4c2d into main Jun 21, 2026
23 checks passed
@lpcox lpcox deleted the add-tests-unmarshal-string-list-expr-e27772db5ba386dc branch June 21, 2026 17:37

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 a dedicated test file that exercises previously uncovered branches in unmarshalStringListOrExpression and related guard-policy JSON helpers.

Changes:

  • Add table-driven unit tests that cover valid inputs and multiple invalid-type / delimiter-only error paths for unmarshalStringListOrExpression.
  • Add tests for GuardPolicy.UnmarshalJSON rejecting non-object JSON payloads.
  • Add a test covering GuardPolicyToMap behavior when passed a typed-nil pointer that marshals to JSON null.
Show a summary per file
File Description
internal/config/guard_policy_str_expr_test.go Adds new unit tests to reach previously uncovered error branches in guard policy parsing/mapping helpers.

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: 4

Comment on lines +11 to +14
// TestUnmarshalStringListOrExpression directly tests all branches of the private
// unmarshalStringListOrExpression function, including the two uncovered error paths:
// - non-string/non-array JSON type (line 289)
// - string of only delimiter characters with no labels (line 296)
Comment on lines +134 to +136
// TestGuardPolicyUnmarshalJSON_NonObjectJSON covers the error path in
// GuardPolicy.UnmarshalJSON when the top-level JSON is not an object.
// This triggers the return at line 100-102 of guard_policy.go.
Comment on lines +170 to +173
// TestGuardPolicyToMap_TypedNilPointer covers the payload == nil guard in
// GuardPolicyToMap (guard_policy.go line 175-177). A typed nil pointer passes
// the interface nil check but marshals to JSON "null", which then unmarshals
// the map variable to nil.
Comment on lines +182 to +184
// TestAllowOnlyPolicyUnmarshalJSON_RefusalLabelsInvalidType covers the error
// path in unmarshalStringListOrExpression (line 289) when refusal-labels has
// an invalid JSON type (number, boolean) that is neither an array nor a string.
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