Skip to content

Commit f20c2da

Browse files
make e2e more safe to run
1 parent ea51752 commit f20c2da

File tree

2 files changed

+33
-6
lines changed

2 files changed

+33
-6
lines changed

e2e/README.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,18 @@ It is possible to provide `GITHUB_MCP_SERVER_E2E_DEBUG=true` to run the e2e test
8383

8484
One might argue that the lack of visibility into failures for the black box tests also indicates a product need, but this solves for the immediate pain point felt as a maintainer.
8585

86+
## Skipping Global State Mutation Tests
87+
88+
Some tools (such as those that mark all notifications as read) will change the real global state of the authenticated user. For end-to-end (e2e) tests, these tests are **skipped by default** to avoid unwanted side effects.
89+
90+
To opt-in and run these tests (which will modify your global state on GitHub), set the following environment variable:
91+
92+
```
93+
GITHUB_MCP_SERVER_E2E_MUTATE_GLOBAL_STATE=1
94+
```
95+
96+
If this variable is not set, tests that would mutate global state will be skipped.
97+
8698
## Limitations
8799

88100
The current test suite is intentionally very limited in scope. This is because the maintenance costs on e2e tests tend to increase significantly over time. To read about some challenges with GitHub integration tests, see [go-github integration tests README](https://github.com/google/go-github/blob/5b75aa86dba5cf4af2923afa0938774f37fa0a67/test/README.md). We will expand this suite circumspectly!

e2e/e2e_test.go

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,16 @@ import (
2424
"github.com/stretchr/testify/require"
2525
)
2626

27+
// Should be set to opt-in to tests that mutate global state
28+
const E2EGlobalMutationOptInEnv = "GITHUB_MCP_SERVER_E2E_MUTATE_GLOBAL_STATE"
29+
30+
// skipIfGlobalMutationNotOptedIn skips the test if the opt-in env var is not set
31+
func skipIfGlobalMutationNotOptedIn(t *testing.T) {
32+
if os.Getenv(E2EGlobalMutationOptInEnv) == "" {
33+
t.Skipf("Skipping test: set %s=1 to opt-in to global state mutation tests", E2EGlobalMutationOptInEnv)
34+
}
35+
}
36+
2737
var (
2838
// Shared variables and sync.Once instances to ensure one-time execution
2939
getTokenOnce sync.Once
@@ -1550,6 +1560,7 @@ func TestE2E_ListNotifications(t *testing.T) {
15501560
}
15511561

15521562
func TestE2E_ManageNotificationSubscription(t *testing.T) {
1563+
skipIfGlobalMutationNotOptedIn(t)
15531564
t.Parallel()
15541565
client := setupMCPClient(t)
15551566
ctx := context.Background()
@@ -1561,9 +1572,7 @@ func TestE2E_ManageNotificationSubscription(t *testing.T) {
15611572
resp, err := client.CallTool(ctx, listReq)
15621573
require.NoError(t, err)
15631574
require.False(t, resp.IsError)
1564-
if len(resp.Content) == 0 {
1565-
return t.Skip("No notifications available to test subscription management")
1566-
}
1575+
15671576
textContent, ok := resp.Content[0].(mcp.TextContent)
15681577
require.True(t, ok)
15691578
var notifications []struct {
@@ -1572,6 +1581,9 @@ func TestE2E_ManageNotificationSubscription(t *testing.T) {
15721581
err = json.Unmarshal([]byte(textContent.Text), &notifications)
15731582
require.NoError(t, err)
15741583
require.NotEmpty(t, notifications)
1584+
if len(notifications) == 0 {
1585+
t.Skip("No notifications available to test subscription management")
1586+
}
15751587
notificationID := notifications[0].ID
15761588

15771589
// Ignore notification
@@ -1618,6 +1630,7 @@ func TestE2E_ManageNotificationSubscription(t *testing.T) {
16181630
}
16191631

16201632
func TestE2E_ManageRepositoryNotificationSubscription(t *testing.T) {
1633+
skipIfGlobalMutationNotOptedIn(t)
16211634
t.Parallel()
16221635
client := setupMCPClient(t)
16231636
ctx := context.Background()
@@ -1673,6 +1686,7 @@ func TestE2E_ManageRepositoryNotificationSubscription(t *testing.T) {
16731686
}
16741687

16751688
func TestE2E_DismissNotification(t *testing.T) {
1689+
skipIfGlobalMutationNotOptedIn(t)
16761690
t.Parallel()
16771691
client := setupMCPClient(t)
16781692
ctx := context.Background()
@@ -1684,16 +1698,16 @@ func TestE2E_DismissNotification(t *testing.T) {
16841698
resp, err := client.CallTool(ctx, listReq)
16851699
require.NoError(t, err)
16861700
require.False(t, resp.IsError)
1687-
if len(resp.Content) == 0 {
1688-
return t.Skip("No notifications available to test dismissal")
1689-
}
16901701
textContent, ok := resp.Content[0].(mcp.TextContent)
16911702
require.True(t, ok)
16921703
var notifications []struct {
16931704
ID string `json:"id"`
16941705
}
16951706
err = json.Unmarshal([]byte(textContent.Text), &notifications)
16961707
require.NoError(t, err)
1708+
if len(notifications) == 0 {
1709+
t.Skip("No notifications available to test dismissal")
1710+
}
16971711
require.NotEmpty(t, notifications)
16981712
threadID := notifications[0].ID
16991713

@@ -1713,6 +1727,7 @@ func TestE2E_DismissNotification(t *testing.T) {
17131727
}
17141728

17151729
func TestE2E_MarkAllNotificationsRead(t *testing.T) {
1730+
skipIfGlobalMutationNotOptedIn(t)
17161731
t.Parallel()
17171732
client := setupMCPClient(t)
17181733
ctx := context.Background()

0 commit comments

Comments
 (0)