Skip to content

[test-improver] Improve tests for internal/util TruncateRunes - #10793

Merged
lpcox merged 4 commits into
mainfrom
test-improver/util-truncate-runes-2e5df222ad6e1cda
Aug 7, 2026
Merged

[test-improver] Improve tests for internal/util TruncateRunes#10793
lpcox merged 4 commits into
mainfrom
test-improver/util-truncate-runes-2e5df222ad6e1cda

Conversation

@github-actions

@github-actions github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

File analyzed

internal/util/truncate_test.go (tests for internal/util/truncate.go)

Why this file

  • Package coverage was already high (99.3%) and uses testify consistently, but TruncateRunes sat at 93.3% coverage with a documented invariant (invalid-UTF-8 normalization) that lacked broad edge-case testing.
  • File was not recently modified in isolation (last touched only via a repo-wide schema bump commit).

Improvements made

  • Added table-driven edge cases to TestTruncateRunes:
    • Multibyte string with more bytes than maxRunes but fewer actual runes (exercises the utf8.RuneCountInString early-return fast path).
    • Mixed ASCII + multibyte truncation mid-string.
    • maxRunes exactly one less than the rune count.
    • Single-rune ASCII string at the boundary.
  • Added a new property-style test, TestTruncateRunes_TruncatedResultIsAlwaysValidUTF8, which asserts that whenever TruncateRunes actually truncates a string, the result is always valid UTF-8 — covering ASCII, multibyte (Japanese), emoji, and malformed-byte inputs, guarding the invariant documented in the function's doc comment.

Coverage

  • Before: internal/util 99.3% overall; TruncateRunes 93.3%.
  • After: internal/util 99.3% overall; TruncateRunes 93.3% (the remaining uncovered line is a defensive return s fallback at the end of the byte-walk loop that is unreachable given the preceding fast-path checks — kept as documented defensive code, not removed).

Test output

go test -count=3 ./internal/util/
ok  	github.com/github/gh-aw-mcpg/internal/util	0.011s
go vet ./internal/util/   # clean
gofmt -l internal/util/truncate_test.go   # no output (formatted)

All existing passing tests were preserved; only new cases and one new test function were added.

Generated by Test Improver · auto · 128.3 AIC · ⊞ 8.1K ·

- Add table-driven cases: multibyte with more bytes than maxRunes but
  fewer runes, mixed ASCII/multibyte truncation, maxRunes one less than
  rune count, and single-rune edge cases.
- Add TestTruncateRunes_TruncatedResultIsAlwaysValidUTF8 property test
  verifying truncated output is always valid UTF-8 across ASCII,
  multibyte, emoji, and malformed-byte inputs.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox
lpcox marked this pull request as ready for review August 7, 2026 02:22
Copilot AI balanced review requested due to automatic review settings August 7, 2026 02:22

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

Expands TruncateRunes tests with boundary, multibyte, and malformed UTF-8 cases.

Changes:

  • Adds table-driven truncation edge cases.
  • Adds UTF-8 validity invariant coverage.
Show a summary per file
File Description
internal/util/truncate_test.go Extends TruncateRunes tests and UTF-8 validation.

Review details

Tip

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

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread internal/util/truncate_test.go Outdated
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox

lpcox commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@copilot fix failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/31141420884/job/92751941789?pr=10793

The failing check was the Rust guard unit test labels::tests::test_apply_tool_labels_search_code, not the Go test changes in this PR. It was broken by fe6fc2f6/6b5cefda on main and has since been fixed upstream.

Merged latest main into this branch in 07729f1. make agent-finished now passes locally, including all 628 Rust guard tests.

Copilot AI requested a review from lpcox August 7, 2026 20:11
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox

lpcox commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>

Copilot AI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

@copilot fix this failing ci check https://github.com/github/gh-aw-mcpg/actions/runs/31213802810/job/92992410203?pr=10793

Fixed in a35c4b1.

Root cause: labels::tests::test_apply_tool_labels_search_code was flaky, not broken. cache_repo_visibility_for_tests wrote its override into the process-wide repo-visibility cache, and test_apply_tool_labels_pull_request_read_matches_get_pull_request uses it to mark github/copilot private. Because get_cached_repo_visibility doesn't take the test lock, any test running concurrently could observe that entry — hence left: ["private:github/copilot"], right: []. Running just those two tests together reproduced the failure 40/40 times.

Overrides are now stored in a thread-local map (cargo test gives each test its own thread), so they can't leak across tests. Verified with 20 consecutive full cargo test runs (628 passed each) plus make agent-finished.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — gVisor

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: gVisor (runsc) kernel-level isolation

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) Error [-32602]: unknown tool BLOCKED
C CLI reads (issues/file) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) unauthenticated — GH_TOKEN not set BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) unauthenticated — GH_TOKEN not set BLOCKED

Overall: PASS

Part B note: gateway enforces read-only by omitting write tools from the allowlist — all write tool calls return MCP error [-32602] unknown tool, confirming gateway-level enforcement.
Parts D & E: gh CLI is unauthenticated; all REST/GraphQL write attempts rejected at auth layer.

References: §31226968355

🔒 mcpg read-only stress (gVisor runtime) by Read-Only Stress: gVisor runtime

@github-actions github-actions Bot mentioned this pull request Aug 7, 2026
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — default AWF

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: default AWF (normal container isolation)

Part Surface Op Result Expected Status
A MCP reads (issues/PRs/file/commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) all refused Error[-32602] unknown tool (gateway, <40ms) BLOCKED
C CLI reads (list_issues/get_file_contents) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) refused — gh unauthenticated (no GH_TOKEN) BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) refused — gh unauthenticated (no GH_TOKEN) BLOCKED

Overall: PASS

References: §31226968339

🔒 mcpg read-only stress (default AWF runtime) by Read-Only Stress: default runtime

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

🔒 mcpg Read-Only Stress — docker-sbx

Surface coverage: MCP tool calls + proxied CLI (REST) + GraphQL mutations
Isolation runtime: docker-sbx (KVM-isolated microVM)

Part Surface Op Result Expected Status
A MCP reads (list_issues, list_prs, get_file, list_commits) data returned ALLOWED
B MCP writes (reaction/star/issue/comment/branch/file/PR) all refused: Error [-32602]: unknown tool (gateway enforcement) BLOCKED
C CLI reads (list_issues, get_file_contents) data returned ALLOWED
D CLI REST writes (reaction/star/issue/comment/file) all refused: HTTP 401 Bad credentials BLOCKED
E CLI GraphQL mutations (addReaction/addStar/createIssue) all refused: HTTP 401 Bad credentials BLOCKED

Overall: PASS

Run: §31226968351

🔒 mcpg read-only stress (docker-sbx runtime) by Read-Only Stress: docker-sbx runtime

@lpcox
lpcox merged commit cbd8c44 into main Aug 7, 2026
37 checks passed
@lpcox
lpcox deleted the test-improver/util-truncate-runes-2e5df222ad6e1cda branch August 7, 2026 23:57
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