Skip to content

Commit 727445f

Browse files
Remove specific get_job_logs test - generic tests are sufficient
Co-authored-by: SamMorrowDrums <4811358+SamMorrowDrums@users.noreply.github.com>
1 parent f760044 commit 727445f

File tree

1 file changed

+0
-68
lines changed

1 file changed

+0
-68
lines changed

pkg/github/actions_test.go

Lines changed: 0 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ import (
1515
"github.com/github/github-mcp-server/internal/profiler"
1616
"github.com/github/github-mcp-server/internal/toolsnaps"
1717
buffer "github.com/github/github-mcp-server/pkg/buffer"
18-
"github.com/github/github-mcp-server/pkg/inventory"
1918
"github.com/github/github-mcp-server/pkg/translations"
2019
"github.com/google/go-github/v79/github"
2120
"github.com/google/jsonschema-go/jsonschema"
@@ -2301,73 +2300,6 @@ func Test_ActionsRunTrigger_CancelWorkflowRun(t *testing.T) {
23012300
})
23022301
}
23032302

2304-
// TestGetJobLogsFeatureFlagScenario tests the specific scenario from GitHub issue #878
2305-
// where get_job_logs tool calls were failing with "unknown tool" errors.
2306-
// This validates that both GetJobLogs and ActionsGetJobLogs tools are properly
2307-
// handled with feature flags and that only one is active at a time.
2308-
func TestGetJobLogsFeatureFlagScenario(t *testing.T) {
2309-
tools := AllTools(stubTranslation)
2310-
2311-
// Find all get_job_logs tools
2312-
var getJobLogsTools []inventory.ServerTool
2313-
for _, tool := range tools {
2314-
if tool.Tool.Name == "get_job_logs" {
2315-
getJobLogsTools = append(getJobLogsTools, tool)
2316-
}
2317-
}
2318-
2319-
// Should have exactly 2 variants
2320-
require.Len(t, getJobLogsTools, 2, "Should have exactly 2 get_job_logs tool variants")
2321-
2322-
// One should have FeatureFlagDisable, one should have FeatureFlagEnable
2323-
var hasEnable, hasDisable bool
2324-
var enableTool, disableTool inventory.ServerTool
2325-
2326-
for _, tool := range getJobLogsTools {
2327-
if tool.FeatureFlagEnable != "" {
2328-
hasEnable = true
2329-
enableTool = tool
2330-
assert.Equal(t, FeatureFlagConsolidatedActions, tool.FeatureFlagEnable,
2331-
"FeatureFlagEnable should be set to FeatureFlagConsolidatedActions")
2332-
}
2333-
if tool.FeatureFlagDisable != "" {
2334-
hasDisable = true
2335-
disableTool = tool
2336-
assert.Equal(t, FeatureFlagConsolidatedActions, tool.FeatureFlagDisable,
2337-
"FeatureFlagDisable should be set to FeatureFlagConsolidatedActions")
2338-
}
2339-
}
2340-
2341-
require.True(t, hasEnable, "One get_job_logs variant should have FeatureFlagEnable")
2342-
require.True(t, hasDisable, "One get_job_logs variant should have FeatureFlagDisable")
2343-
2344-
// Test that feature flag filtering works correctly
2345-
// Build inventory without feature checker (flag OFF by default)
2346-
regFlagOff := NewInventory(stubTranslation).WithToolsets([]string{"all"}).Build()
2347-
filteredOff := regFlagOff.ForMCPRequest(inventory.MCPMethodToolsCall, "get_job_logs")
2348-
availableOff := filteredOff.AvailableTools(context.Background())
2349-
2350-
require.Len(t, availableOff, 1, "With flag OFF, should have exactly 1 get_job_logs tool")
2351-
assert.Equal(t, FeatureFlagConsolidatedActions, availableOff[0].FeatureFlagDisable,
2352-
"With flag OFF, should get the tool with FeatureFlagDisable")
2353-
2354-
// Build inventory with feature checker (flag ON)
2355-
checkerOn := func(_ context.Context, flag string) (bool, error) {
2356-
return flag == FeatureFlagConsolidatedActions, nil
2357-
}
2358-
regFlagOn := NewInventory(stubTranslation).WithToolsets([]string{"all"}).WithFeatureChecker(checkerOn).Build()
2359-
filteredOn := regFlagOn.ForMCPRequest(inventory.MCPMethodToolsCall, "get_job_logs")
2360-
availableOn := filteredOn.AvailableTools(context.Background())
2361-
2362-
require.Len(t, availableOn, 1, "With flag ON, should have exactly 1 get_job_logs tool")
2363-
assert.Equal(t, FeatureFlagConsolidatedActions, availableOn[0].FeatureFlagEnable,
2364-
"With flag ON, should get the tool with FeatureFlagEnable")
2365-
2366-
// Verify both tools have handlers (this was the original issue - tool found but couldn't be called)
2367-
assert.True(t, enableTool.HasHandler(), "Consolidated tool should have handler")
2368-
assert.True(t, disableTool.HasHandler(), "Original tool should have handler")
2369-
}
2370-
23712303
func Test_ActionsGetJobLogs(t *testing.T) {
23722304
// Verify tool definition once
23732305
toolDef := ActionsGetJobLogs(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)