Skip to content

Commit a5a2267

Browse files
omgitsadsomgitsads
committed
Fix tests
1 parent c33b0ba commit a5a2267

File tree

2 files changed

+43
-57
lines changed

2 files changed

+43
-57
lines changed

‎pkg/github/actions.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,9 +208,15 @@ func ActionsRead(getClient GetClientFn, t translations.TranslationHelperFunc) (t
208208
var resourceIDInt int64
209209
var parseErr error
210210
switch resourceType {
211+
case actionsActionTypeListWorkflows:
212+
// No resource ID required
211213
case actionsActionTypeGetWorkflow, actionsActionTypeListWorkflowRuns:
212214
// Do nothing, we accept both a string workflow ID or filename
213215
default:
216+
if resourceID == "" {
217+
return mcp.NewToolResultError(fmt.Sprintf("missing required parameter for action %s: resource_id", actionTypeStr)), nil
218+
}
219+
214220
// For other actions, resource ID must be an integer
215221
resourceIDInt, parseErr = strconv.ParseInt(resourceID, 10, 64)
216222
if parseErr != nil {

‎pkg/github/actions_test.go‎

Lines changed: 37 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package github
33
import (
44
"context"
55
"encoding/json"
6+
"fmt"
67
"io"
78
"net/http"
89
"net/http/httptest"
@@ -22,19 +23,7 @@ import (
2223
"github.com/stretchr/testify/require"
2324
)
2425

25-
func Test_ListWorkflows(t *testing.T) {
26-
// Verify tool definition once
27-
mockClient := github.NewClient(nil)
28-
tool, _ := ListWorkflows(stubGetClientFn(mockClient), translations.NullTranslationHelper)
29-
30-
assert.Equal(t, "list_workflows", tool.Name)
31-
assert.NotEmpty(t, tool.Description)
32-
assert.Contains(t, tool.InputSchema.Properties, "owner")
33-
assert.Contains(t, tool.InputSchema.Properties, "repo")
34-
assert.Contains(t, tool.InputSchema.Properties, "perPage")
35-
assert.Contains(t, tool.InputSchema.Properties, "page")
36-
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"owner", "repo"})
37-
26+
func Test_ActionsRead_ListWorkflows(t *testing.T) {
3827
tests := []struct {
3928
name string
4029
mockedClient *http.Client
@@ -83,16 +72,18 @@ func Test_ListWorkflows(t *testing.T) {
8372
),
8473
),
8574
requestArgs: map[string]any{
86-
"owner": "owner",
87-
"repo": "repo",
75+
"action": actionsActionTypeListWorkflows.String(),
76+
"owner": "owner",
77+
"repo": "repo",
8878
},
8979
expectError: false,
9080
},
9181
{
9282
name: "missing required parameter owner",
9383
mockedClient: mock.NewMockedHTTPClient(),
9484
requestArgs: map[string]any{
95-
"repo": "repo",
85+
"action": actionsActionTypeListWorkflows.String(),
86+
"repo": "repo",
9687
},
9788
expectError: true,
9889
expectedErrMsg: "missing required parameter: owner",
@@ -103,7 +94,7 @@ func Test_ListWorkflows(t *testing.T) {
10394
t.Run(tc.name, func(t *testing.T) {
10495
// Setup client with mock
10596
client := github.NewClient(tc.mockedClient)
106-
_, handler := ListWorkflows(stubGetClientFn(client), translations.NullTranslationHelper)
97+
_, handler := ActionsRead(stubGetClientFn(client), translations.NullTranslationHelper)
10798

10899
// Create call request
109100
request := createMCPRequest(tc.requestArgs)
@@ -477,7 +468,7 @@ func Test_ListWorkflowRunArtifacts(t *testing.T) {
477468
),
478469
),
479470
requestArgs: map[string]any{
480-
"action": "list_workflow_artifacts",
471+
"action": actionsActionTypeListWorkflowArtifacts.String(),
481472
"owner": "owner",
482473
"repo": "repo",
483474
"resource_id": "12345",
@@ -488,12 +479,12 @@ func Test_ListWorkflowRunArtifacts(t *testing.T) {
488479
name: "missing required parameter resource_id",
489480
mockedClient: mock.NewMockedHTTPClient(),
490481
requestArgs: map[string]any{
491-
"action": "list_workflow_artifacts",
482+
"action": actionsActionTypeListWorkflowArtifacts.String(),
492483
"owner": "owner",
493484
"repo": "repo",
494485
},
495486
expectError: true,
496-
expectedErrMsg: "missing required parameter: resource_id",
487+
expectedErrMsg: fmt.Sprintf("missing required parameter for action %s: resource_id", actionsActionTypeListWorkflowArtifacts.String()),
497488
},
498489
}
499490

@@ -555,7 +546,7 @@ func Test_DownloadWorkflowRunArtifact(t *testing.T) {
555546
),
556547
),
557548
requestArgs: map[string]any{
558-
"action": "download_workflow_artifact",
549+
"action": actionsActionTypeDownloadWorkflowArtifact.String(),
559550
"owner": "owner",
560551
"repo": "repo",
561552
"resource_id": "123",
@@ -566,12 +557,12 @@ func Test_DownloadWorkflowRunArtifact(t *testing.T) {
566557
name: "missing required parameter resource_id",
567558
mockedClient: mock.NewMockedHTTPClient(),
568559
requestArgs: map[string]any{
569-
"action": "download_workflow_artifact",
560+
"action": actionsActionTypeDownloadWorkflowArtifact.String(),
570561
"owner": "owner",
571562
"repo": "repo",
572563
},
573564
expectError: true,
574-
expectedErrMsg: "missing required parameter: resource_id",
565+
expectedErrMsg: fmt.Sprintf("missing required parameter for action %s: resource_id", actionsActionTypeDownloadWorkflowArtifact.String()),
575566
},
576567
}
577568

@@ -719,7 +710,7 @@ func Test_GetWorkflowRunUsage(t *testing.T) {
719710
),
720711
),
721712
requestArgs: map[string]any{
722-
"action": "get_workflow_run_usage",
713+
"action": actionsActionTypeGetWorkflowRunUsage.String(),
723714
"owner": "owner",
724715
"repo": "repo",
725716
"resource_id": "12345",
@@ -730,12 +721,12 @@ func Test_GetWorkflowRunUsage(t *testing.T) {
730721
name: "missing required parameter run_id",
731722
mockedClient: mock.NewMockedHTTPClient(),
732723
requestArgs: map[string]any{
733-
"action": "get_workflow_run_usage",
724+
"action": actionsActionTypeGetWorkflowRunUsage.String(),
734725
"owner": "owner",
735726
"repo": "repo",
736727
},
737728
expectError: true,
738-
expectedErrMsg: "missing required parameter: resource_id",
729+
expectedErrMsg: fmt.Sprintf("missing required parameter for action %s: resource_id", actionsActionTypeGetWorkflowRunUsage.String()),
739730
},
740731
}
741732

@@ -1292,7 +1283,7 @@ func Test_ActionsResourceRead(t *testing.T) {
12921283
assert.Contains(t, tool.InputSchema.Properties, "owner")
12931284
assert.Contains(t, tool.InputSchema.Properties, "repo")
12941285
assert.Contains(t, tool.InputSchema.Properties, "resource_id")
1295-
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"action", "owner", "repo", "resource_id"})
1286+
assert.ElementsMatch(t, tool.InputSchema.Required, []string{"action", "owner", "repo"})
12961287

12971288
tests := []struct {
12981289
name string
@@ -1316,7 +1307,7 @@ func Test_ActionsResourceRead(t *testing.T) {
13161307
name: "missing required parameter owner",
13171308
mockedClient: mock.NewMockedHTTPClient(),
13181309
requestArgs: map[string]any{
1319-
"action": "get_workflow",
1310+
"action": actionsActionTypeGetWorkflow.String(),
13201311
"repo": "repo",
13211312
"resource_id": "123",
13221313
},
@@ -1327,24 +1318,13 @@ func Test_ActionsResourceRead(t *testing.T) {
13271318
name: "missing required parameter repo",
13281319
mockedClient: mock.NewMockedHTTPClient(),
13291320
requestArgs: map[string]any{
1330-
"action": "get_workflow",
1321+
"action": actionsActionTypeGetWorkflow.String(),
13311322
"owner": "owner",
13321323
"resource_id": "123",
13331324
},
13341325
expectError: true,
13351326
expectedErrMsg: "missing required parameter: repo",
13361327
},
1337-
{
1338-
name: "missing required parameter resource_id",
1339-
mockedClient: mock.NewMockedHTTPClient(),
1340-
requestArgs: map[string]any{
1341-
"action": "get_workflow",
1342-
"owner": "owner",
1343-
"repo": "repo",
1344-
},
1345-
expectError: true,
1346-
expectedErrMsg: "missing required parameter: resource_id",
1347-
},
13481328
{
13491329
name: "unknown resource",
13501330
mockedClient: mock.NewMockedHTTPClient(),
@@ -1413,7 +1393,7 @@ func Test_ActionsResourceRead_GetWorkflow(t *testing.T) {
14131393
),
14141394
),
14151395
requestArgs: map[string]any{
1416-
"action": "get_workflow",
1396+
"action": actionsActionTypeGetWorkflow.String(),
14171397
"owner": "owner",
14181398
"repo": "repo",
14191399
"resource_id": "1",
@@ -1431,7 +1411,7 @@ func Test_ActionsResourceRead_GetWorkflow(t *testing.T) {
14311411
),
14321412
),
14331413
requestArgs: map[string]any{
1434-
"action": "get_workflow",
1414+
"action": actionsActionTypeGetWorkflow.String(),
14351415
"owner": "owner",
14361416
"repo": "repo",
14371417
"resource_id": "2",
@@ -1508,7 +1488,7 @@ func Test_ActionsResourceRead_GetWorkflowRun(t *testing.T) {
15081488
),
15091489
),
15101490
requestArgs: map[string]any{
1511-
"action": "get_workflow_run",
1491+
"action": actionsActionTypeGetWorkflowRun.String(),
15121492
"owner": "owner",
15131493
"repo": "repo",
15141494
"resource_id": "12345",
@@ -1526,7 +1506,7 @@ func Test_ActionsResourceRead_GetWorkflowRun(t *testing.T) {
15261506
),
15271507
),
15281508
requestArgs: map[string]any{
1529-
"action": "get_workflow_run",
1509+
"action": actionsActionTypeGetWorkflowRun.String(),
15301510
"owner": "owner",
15311511
"repo": "repo",
15321512
"resource_id": "99999",
@@ -1617,7 +1597,7 @@ func Test_ActionsResourceRead_ListWorkflowRuns(t *testing.T) {
16171597
),
16181598
),
16191599
requestArgs: map[string]any{
1620-
"action": "list_workflow_runs",
1600+
"action": actionsActionTypeListWorkflowRuns.String(),
16211601
"owner": "owner",
16221602
"repo": "repo",
16231603
"resource_id": "1",
@@ -1656,7 +1636,7 @@ func Test_ActionsResourceRead_ListWorkflowRuns(t *testing.T) {
16561636
),
16571637
),
16581638
requestArgs: map[string]any{
1659-
"action": "list_workflow_runs",
1639+
"action": actionsActionTypeListWorkflowRuns.String(),
16601640
"owner": "owner",
16611641
"repo": "repo",
16621642
"resource_id": "1",
@@ -1678,7 +1658,7 @@ func Test_ActionsResourceRead_ListWorkflowRuns(t *testing.T) {
16781658
),
16791659
),
16801660
requestArgs: map[string]any{
1681-
"action": "list_workflow_runs",
1661+
"action": actionsActionTypeListWorkflowRuns.String(),
16821662
"owner": "owner",
16831663
"repo": "repo",
16841664
"resource_id": "99999",
@@ -1746,7 +1726,7 @@ func Test_ActionsResourceRead_GetWorkflowJob(t *testing.T) {
17461726
),
17471727
),
17481728
requestArgs: map[string]any{
1749-
"action": "get_workflow_job",
1729+
"action": actionsActionTypeGetWorkflowJob.String(),
17501730
"owner": "owner",
17511731
"repo": "repo",
17521732
"resource_id": "12345",
@@ -1764,7 +1744,7 @@ func Test_ActionsResourceRead_GetWorkflowJob(t *testing.T) {
17641744
),
17651745
),
17661746
requestArgs: map[string]any{
1767-
"action": "get_workflow_job",
1747+
"action": actionsActionTypeGetWorkflowJob.String(),
17681748
"owner": "owner",
17691749
"repo": "repo",
17701750
"resource_id": "99999",
@@ -1843,7 +1823,7 @@ func Test_ActionsResourceRead_ListWorkflowJobs(t *testing.T) {
18431823
),
18441824
),
18451825
requestArgs: map[string]any{
1846-
"action": "list_workflow_jobs",
1826+
"action": actionsActionTypeListWorkflowJobs.String(),
18471827
"owner": "owner",
18481828
"repo": "repo",
18491829
"resource_id": "1",
@@ -1861,7 +1841,7 @@ func Test_ActionsResourceRead_ListWorkflowJobs(t *testing.T) {
18611841
),
18621842
),
18631843
requestArgs: map[string]any{
1864-
"action": "list_workflow_jobs",
1844+
"action": actionsActionTypeListWorkflowJobs.String(),
18651845
"owner": "owner",
18661846
"repo": "repo",
18671847
"resource_id": "99999",
@@ -1923,7 +1903,7 @@ func Test_ActionsResourceRead_DownloadWorkflowArtifact(t *testing.T) {
19231903
),
19241904
),
19251905
requestArgs: map[string]any{
1926-
"action": "download_workflow_artifact",
1906+
"action": actionsActionTypeDownloadWorkflowArtifact.String(),
19271907
"owner": "owner",
19281908
"repo": "repo",
19291909
"resource_id": "12345",
@@ -1941,7 +1921,7 @@ func Test_ActionsResourceRead_DownloadWorkflowArtifact(t *testing.T) {
19411921
),
19421922
),
19431923
requestArgs: map[string]any{
1944-
"action": "download_workflow_artifact",
1924+
"action": actionsActionTypeDownloadWorkflowArtifact.String(),
19451925
"owner": "owner",
19461926
"repo": "repo",
19471927
"resource_id": "99999",
@@ -2016,7 +1996,7 @@ func Test_ActionsResourceRead_ListWorkflowArtifacts(t *testing.T) {
20161996
),
20171997
),
20181998
requestArgs: map[string]any{
2019-
"action": "list_workflow_artifacts",
1999+
"action": actionsActionTypeListWorkflowArtifacts.String(),
20202000
"owner": "owner",
20212001
"repo": "repo",
20222002
"resource_id": "1",
@@ -2034,7 +2014,7 @@ func Test_ActionsResourceRead_ListWorkflowArtifacts(t *testing.T) {
20342014
),
20352015
),
20362016
requestArgs: map[string]any{
2037-
"action": "list_workflow_artifacts",
2017+
"action": actionsActionTypeListWorkflowArtifacts.String(),
20382018
"owner": "owner",
20392019
"repo": "repo",
20402020
"resource_id": "99999",
@@ -2111,7 +2091,7 @@ func Test_ActionsResourceRead_GetWorkflowUsage(t *testing.T) {
21112091
),
21122092
),
21132093
requestArgs: map[string]any{
2114-
"action": "get_workflow_run_usage",
2094+
"action": actionsActionTypeGetWorkflowRunUsage.String(),
21152095
"owner": "owner",
21162096
"repo": "repo",
21172097
"resource_id": "1",
@@ -2129,7 +2109,7 @@ func Test_ActionsResourceRead_GetWorkflowUsage(t *testing.T) {
21292109
),
21302110
),
21312111
requestArgs: map[string]any{
2132-
"action": "get_workflow_run_usage",
2112+
"action": actionsActionTypeGetWorkflowRunUsage.String(),
21332113
"owner": "owner",
21342114
"repo": "repo",
21352115
"resource_id": "99999",

0 commit comments

Comments
 (0)