Skip to content

Add Go low-level tool-definition E2E test [2/6]#1724

Merged
edburns merged 2 commits into
mainfrom
edburns/1682-02-go-new-test
Jun 18, 2026
Merged

Add Go low-level tool-definition E2E test [2/6]#1724
edburns merged 2 commits into
mainfrom
edburns/1682-02-go-new-test

Conversation

@edburns

@edburns edburns commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds the Go low-level tool-definition E2E scenario to go/internal/e2e/tools_e2e_test.go as part of the PR-1692 breakout sequence.

This PR is related to issue #1682 but does not fix #1682.

What changed

  • Adds low_level_tool_definition coverage in Go E2E tests.
  • Reuses the shared snapshot introduced by PR Add Java low-level tool definition E2E test and skill [1/6] #1721:
    • test/snapshots/tools/low_level_tool_definition.yaml
  • Keeps the test aligned with snapshot behavior by defining only the tools that the snapshot actually exercises (set_current_phase, search_items).
  • Validates decoded keyword arguments in the search tool path.

Dependency / sequencing

Related

Related to issue #1682 but does not fix #1682.

Align low_level_tool_definition coverage with PR #1721 snapshot behavior by only defining tools exercised by the shared snapshot.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 18, 2026 19:57
@edburns edburns requested a review from a team as a code owner June 18, 2026 19:57

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

Adds a Go E2E subtest that exercises the shared tools/low_level_tool_definition.yaml snapshot by defining the two custom tools the snapshot calls (set_current_phase, search_items) and validating the final assistant response.

Changes:

  • Add a new Go E2E subtest low_level_tool_definition under TestToolsE2E.
  • Define two custom tools used by the snapshot and assert phase + result content in the final assistant message.
Show a summary per file
File Description
go/internal/e2e/tools_e2e_test.go Adds the new low_level_tool_definition Go E2E scenario using the shared replay snapshot and custom tool definitions.

Copilot's findings

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

Comment on lines +97 to +111
currentPhase := ""

setCurrentPhaseTool := copilot.DefineTool("set_current_phase", "Sets the current phase of the agent",
func(params PhaseArgs, inv copilot.ToolInvocation) (string, error) {
currentPhase = params.Phase
return "Phase set to " + params.Phase, nil
})

searchItemsTool := copilot.DefineTool("search_items", "Search for items by keyword",
func(params SearchArgs, inv copilot.ToolInvocation) (string, error) {
if params.Keyword != "copilot" {
t.Fatalf("Expected keyword to be 'copilot', got %q", params.Keyword)
}
return "Found: item_alpha, item_beta", nil
})
Comment thread go/internal/e2e/tools_e2e_test.go Outdated
Comment on lines +156 to +158
if currentPhase != "analyzing" {
t.Errorf("Expected currentPhase to be 'analyzing', got %q", currentPhase)
}
@github-actions

This comment has been minimized.

Synchronize handler-updated state with a mutex and move keyword assertion to the main test goroutine to avoid calling t.Fatalf from a tool handler goroutine.

Related to issue #1682 but does not fix #1682.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Cross-SDK Consistency Review ✅

This PR is part of a deliberate 6-PR breakout sequence and maintains cross-SDK consistency well.

Coverage status across all SDKs:

SDK Status PR
Java ✅ Already present (LowLevelToolDefinitionIT.java) (pre-existing)
Node.js ✅ Added #1725 (merged)
Go ✅ Added this PR
Python 🔄 In progress #1726
Rust 🔄 In progress #1727
.NET 🔄 In progress #1728

Observations:

  1. Consistent semantics: Go uses the same prompt, same tool names (set_current_phase, search_items), and same assertions (currentPhase == "analyzing", content contains "item_alpha"/"item_beta") as Node.js and Java.

  2. Correct snapshot reuse: The test reuses test/snapshots/tools/low_level_tool_definition.yaml as intended and only registers the 2 tools the snapshot actually exercises — which is an intentional improvement over Java's LowLevelToolDefinitionIT, which also registers a non-exercised grep override tool.

  3. Language-idiomatic: Go captures the searchKeyword in a post-execution assertion; Node.js validates inline. Both are appropriate for their respective language idioms.

No cross-SDK consistency issues found in this PR.

Generated by SDK Consistency Review Agent for issue #1724 · sonnet46 1.2M ·

@edburns edburns merged commit 0f22ce5 into main Jun 18, 2026
23 checks passed
@edburns edburns deleted the edburns/1682-02-go-new-test branch June 18, 2026 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Java: Ergonomics: Defining tools

2 participants