Skip to content

[test] Add tests for config.stripExtensionFieldsForValidation and assignLegacyIntAlias#5762

Merged
lpcox merged 2 commits into
mainfrom
add-tests-config-stdin-helpers-1eb99d80d480dbeb
May 15, 2026
Merged

[test] Add tests for config.stripExtensionFieldsForValidation and assignLegacyIntAlias#5762
lpcox merged 2 commits into
mainfrom
add-tests-config-stdin-helpers-1eb99d80d480dbeb

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Test Coverage Improvement

Functions Analyzed

  • Package: internal/config
  • Functions: stripExtensionFieldsForValidation, assignLegacyIntAlias
  • File: internal/config/config_stdin.go
  • Previous Coverage: 0% (no direct tests)
  • Complexity: Medium — both have multiple conditional branches

Why These Functions?

Static analysis of all source files identified these two private functions as having zero direct test coverage (confirmed via codebase-wide grep for function name references in *_test.go files).

Both functions are called on every LoadFromStdin() invocation, making them part of a critical hot path. They contain multiple branches that previously relied entirely on integration-level tests to exercise.

stripExtensionFieldsForValidation (11 branches):

  • Strips top-level guards field
  • Strips per-server guard, auth, and tool_response_filters fields from all servers
  • Preserves guard-policies (already injected into the upstream schema)
  • Handles missing mcpServers gracefully
  • Returns error on malformed JSON

assignLegacyIntAlias (5 branches):

  • Skips when target pointer is already set (priority: new field wins)
  • No-ops when alias key is absent
  • Unmarshals and assigns a valid integer
  • Returns an error for non-integer JSON values
  • Returns an error for malformed JSON

Tests Added

  • ✅ Happy path test cases for both functions
  • ✅ Edge cases: empty config, missing fields, multiple servers
  • ✅ Error handling: invalid JSON, malformed values, wrong types
  • ✅ Branch coverage: all conditionals exercised
  • ✅ Preservation verification: guard-policies kept, non-extension fields kept
  • ✅ Zero and negative integer values for assignLegacyIntAlias

Test Structure

All tests follow the codebase's established table-driven pattern with testify/assert and testify/require.


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

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • proxy.golang.org

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

network:
  allowed:
    - defaults
    - "proxy.golang.org"

See Network Configuration for more information.

Generated by Test Coverage Improver · ● 6.4M ·

…cyIntAlias

These two private functions in internal/config/config_stdin.go had zero
direct test coverage:

- stripExtensionFieldsForValidation: removes gateway-specific extension
  fields (guards, guard, auth, tool_response_filters) from JSON config
  before upstream schema validation. Tests cover all branches: top-level
  guards removal, per-server field stripping for each extension field,
  multiple servers, guard-policies preservation, and invalid JSON error.

- assignLegacyIntAlias: maps a legacy field name to a typed int pointer
  only when the target has not already been set. Tests cover all branches:
  target already set (skip), alias absent (no-op), valid int assignment,
  zero value, negative value, and invalid/malformed JSON (error path).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review May 15, 2026 17:14
Copilot AI review requested due to automatic review settings May 15, 2026 17:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds direct unit coverage for private stdin configuration helpers in internal/config, focusing on extension-field stripping before schema validation and legacy integer alias assignment.

Changes:

  • Adds table-driven tests for stripExtensionFieldsForValidation.
  • Adds branch-focused tests for assignLegacyIntAlias.
  • Covers success, no-op, and error cases for both helpers.
Show a summary per file
File Description
internal/config/config_stdin_helpers_test.go New unit tests for stdin config helper functions.

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

"s1": {
"type": "http",
"url": "https://example.com",
"guard-policies": [{"allow-only": {"repos": "public", "min-integrity": "none"}}]
Comment on lines +230 to +234
"timeout": json.RawMessage(`42`),
}
err := assignLegacyIntAlias(fields, "timeout", &target)
require.NoError(t, err)
// target should remain unchanged
Comment on lines +172 to +173
name: "empty config is preserved",
input: `{}`,
@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 15, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor

Copilot AI commented May 15, 2026

@copilot address review feedback

Addressed all three review findings in the latest commit:

  1. guard-policies shape (line 130): Changed fixture from array to object {"allow-only": {"repos": "public", "min-integrity": "none"}} to match the actual schema.

  2. "target already set" skips unmarshalling (lines 230–234): Replaced the valid 42 alias with malformed JSON {not valid json} — the test still passes (no error returned), which proves the already-set guard fires before any parsing attempt.

  3. "empty config is preserved" assertion (lines 172–173): Added wantOutputEmpty bool to the test struct and assert.Empty(t, out) in the loop, so the case now verifies the decoded output contains no keys.

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