Skip to content

Commit 67a9507

Browse files
SamMorrowDrumsSamMorrowDrums
committed
remove some e2e tests and add a note about why
1 parent 7082815 commit 67a9507

File tree

2 files changed

+4
-299
lines changed

2 files changed

+4
-299
lines changed

‎e2e/README.md‎

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -83,22 +83,14 @@ 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-
9886
## Limitations
9987

10088
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!
10189

10290
The tests are quite repetitive and verbose. This is intentional as we want to see them develop more before committing to abstractions.
10391

10492
Currently, visibility into failures is not particularly good. We're hoping that we can pull apart the mcp-go client and have it hook into streams representing stdio without requiring an exec. This way we can get breakpoints in the debugger easily.
93+
94+
### Global State Mutation Tests
95+
96+
Some tools (such as those that mark all notifications as read) would change the global state for the tester, and are also not idempotent, so they offer little value for end to end tests and instead should rely on unit testing and manual verifications.

‎e2e/e2e_test.go‎

Lines changed: 0 additions & 287 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,6 @@ 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-
3727
var (
3828
// Shared variables and sync.Once instances to ensure one-time execution
3929
getTokenOnce sync.Once
@@ -1559,280 +1549,3 @@ func TestListNotifications(t *testing.T) {
15591549
err = json.Unmarshal([]byte(textContent.Text), &notifications)
15601550
require.NoError(t, err, "expected to unmarshal text content successfully")
15611551
}
1562-
1563-
func TestManageNotificationSubscription(t *testing.T) {
1564-
skipIfGlobalMutationNotOptedIn(t)
1565-
t.Parallel()
1566-
client := setupMCPClient(t)
1567-
ctx := context.Background()
1568-
1569-
// List notifications to get a valid notificationID
1570-
listReq := mcp.CallToolRequest{}
1571-
listReq.Params.Name = "list_notifications"
1572-
listReq.Params.Arguments = map[string]any{}
1573-
resp, err := client.CallTool(ctx, listReq)
1574-
require.NoError(t, err)
1575-
require.False(t, resp.IsError)
1576-
1577-
textContent, ok := resp.Content[0].(mcp.TextContent)
1578-
require.True(t, ok)
1579-
var notifications []struct {
1580-
ID string `json:"id"`
1581-
}
1582-
err = json.Unmarshal([]byte(textContent.Text), &notifications)
1583-
require.NoError(t, err)
1584-
require.NotEmpty(t, notifications)
1585-
if len(notifications) == 0 {
1586-
t.Skip("No notifications available to test subscription management")
1587-
}
1588-
notificationID := notifications[0].ID
1589-
1590-
// Ignore notification
1591-
ignoreReq := mcp.CallToolRequest{}
1592-
ignoreReq.Params.Name = "manage_notification_subscription"
1593-
ignoreReq.Params.Arguments = map[string]any{
1594-
"notificationID": notificationID,
1595-
"action": "ignore",
1596-
}
1597-
resp, err = client.CallTool(ctx, ignoreReq)
1598-
require.NoError(t, err)
1599-
require.False(t, resp.IsError)
1600-
textContent, ok = resp.Content[0].(mcp.TextContent)
1601-
require.True(t, ok)
1602-
require.Contains(t, textContent.Text, "ignored")
1603-
1604-
// Validate with REST client
1605-
restClient := getRESTClient(t)
1606-
sub, _, err := restClient.Activity.GetThreadSubscription(ctx, notificationID)
1607-
require.NoError(t, err)
1608-
require.NotNil(t, sub)
1609-
require.True(t, sub.GetIgnored(), "expected notification subscription to be ignored")
1610-
1611-
// Watch notification
1612-
watchReq := mcp.CallToolRequest{}
1613-
watchReq.Params.Name = "manage_notification_subscription"
1614-
watchReq.Params.Arguments = map[string]any{
1615-
"notificationID": notificationID,
1616-
"action": "watch",
1617-
}
1618-
resp, err = client.CallTool(ctx, watchReq)
1619-
require.NoError(t, err)
1620-
require.False(t, resp.IsError)
1621-
textContent, ok = resp.Content[0].(mcp.TextContent)
1622-
require.True(t, ok)
1623-
require.Contains(t, textContent.Text, "subscribed")
1624-
1625-
// Validate with REST client
1626-
sub, _, err = restClient.Activity.GetThreadSubscription(ctx, notificationID)
1627-
require.NoError(t, err)
1628-
require.NotNil(t, sub)
1629-
require.False(t, sub.GetIgnored(), "expected notification subscription to not be ignored (watch)")
1630-
require.True(t, sub.GetSubscribed(), "expected notification subscription to be subscribed")
1631-
1632-
// Delete notification subscription
1633-
deleteReq := mcp.CallToolRequest{}
1634-
deleteReq.Params.Name = "manage_notification_subscription"
1635-
deleteReq.Params.Arguments = map[string]any{
1636-
"notificationID": notificationID,
1637-
"action": "delete",
1638-
}
1639-
resp, err = client.CallTool(ctx, deleteReq)
1640-
require.NoError(t, err)
1641-
require.False(t, resp.IsError)
1642-
textContent, ok = resp.Content[0].(mcp.TextContent)
1643-
require.True(t, ok)
1644-
require.Contains(t, textContent.Text, "deleted")
1645-
1646-
// Validate with REST client
1647-
sub, resp2, err := restClient.Activity.GetThreadSubscription(ctx, notificationID)
1648-
require.NoError(t, err)
1649-
require.NotNil(t, sub)
1650-
require.False(t, sub.GetSubscribed())
1651-
require.True(t, sub.GetIgnored())
1652-
require.Equal(t, 204, resp2.StatusCode)
1653-
}
1654-
1655-
func TestManageRepositoryNotificationSubscription(t *testing.T) {
1656-
skipIfGlobalMutationNotOptedIn(t)
1657-
t.Parallel()
1658-
client := setupMCPClient(t)
1659-
ctx := context.Background()
1660-
1661-
// Use a well-known repo for the test (e.g., the user's own repo)
1662-
owner := "github"
1663-
repo := "github-mcp-server"
1664-
1665-
// Ignore repo notifications
1666-
ignoreReq := mcp.CallToolRequest{}
1667-
ignoreReq.Params.Name = "manage_repository_notification_subscription"
1668-
ignoreReq.Params.Arguments = map[string]any{
1669-
"owner": owner,
1670-
"repo": repo,
1671-
"action": "ignore",
1672-
}
1673-
resp, err := client.CallTool(ctx, ignoreReq)
1674-
require.NoError(t, err)
1675-
require.False(t, resp.IsError)
1676-
textContent, ok := resp.Content[0].(mcp.TextContent)
1677-
require.True(t, ok)
1678-
require.Contains(t, textContent.Text, "ignored")
1679-
1680-
// Validate with REST client
1681-
restClient := getRESTClient(t)
1682-
sub, _, err := restClient.Activity.GetRepositorySubscription(ctx, owner, repo)
1683-
require.NoError(t, err)
1684-
require.NotNil(t, sub)
1685-
require.True(t, sub.GetIgnored(), "expected repository subscription to be ignored")
1686-
1687-
// Watch repo notifications
1688-
watchReq := mcp.CallToolRequest{}
1689-
watchReq.Params.Name = "manage_repository_notification_subscription"
1690-
watchReq.Params.Arguments = map[string]any{
1691-
"owner": owner,
1692-
"repo": repo,
1693-
"action": "watch",
1694-
}
1695-
resp, err = client.CallTool(ctx, watchReq)
1696-
require.NoError(t, err)
1697-
require.False(t, resp.IsError)
1698-
textContent, ok = resp.Content[0].(mcp.TextContent)
1699-
require.True(t, ok)
1700-
require.Contains(t, textContent.Text, "subscribed")
1701-
1702-
// Validate with REST client
1703-
sub, _, err = restClient.Activity.GetRepositorySubscription(ctx, owner, repo)
1704-
require.NoError(t, err)
1705-
require.NotNil(t, sub)
1706-
require.False(t, sub.GetIgnored(), "expected repository subscription to not be ignored (watch)")
1707-
require.True(t, sub.GetSubscribed(), "expected repository subscription to be subscribed")
1708-
1709-
// Delete repo notification subscription
1710-
deleteReq := mcp.CallToolRequest{}
1711-
deleteReq.Params.Name = "manage_repository_notification_subscription"
1712-
deleteReq.Params.Arguments = map[string]any{
1713-
"owner": owner,
1714-
"repo": repo,
1715-
"action": "delete",
1716-
}
1717-
resp, err = client.CallTool(ctx, deleteReq)
1718-
require.NoError(t, err)
1719-
require.False(t, resp.IsError)
1720-
textContent, ok = resp.Content[0].(mcp.TextContent)
1721-
require.True(t, ok)
1722-
require.Contains(t, textContent.Text, "deleted")
1723-
1724-
// Validate with REST client
1725-
sub, resp2, err := restClient.Activity.GetRepositorySubscription(ctx, owner, repo)
1726-
require.NoError(t, err)
1727-
require.NotNil(t, sub)
1728-
require.False(t, sub.GetSubscribed())
1729-
require.True(t, sub.GetIgnored())
1730-
require.Equal(t, 204, resp2.StatusCode)
1731-
}
1732-
1733-
func TestDismissNotification(t *testing.T) {
1734-
skipIfGlobalMutationNotOptedIn(t)
1735-
t.Parallel()
1736-
client := setupMCPClient(t)
1737-
ctx := context.Background()
1738-
1739-
// List notifications to get a valid threadID
1740-
listReq := mcp.CallToolRequest{}
1741-
listReq.Params.Name = "list_notifications"
1742-
listReq.Params.Arguments = map[string]any{}
1743-
resp, err := client.CallTool(ctx, listReq)
1744-
require.NoError(t, err)
1745-
require.False(t, resp.IsError)
1746-
textContent, ok := resp.Content[0].(mcp.TextContent)
1747-
require.True(t, ok)
1748-
var notifications []struct {
1749-
ID string `json:"id"`
1750-
}
1751-
err = json.Unmarshal([]byte(textContent.Text), &notifications)
1752-
require.NoError(t, err)
1753-
if len(notifications) == 0 {
1754-
t.Skip("No notifications available to test dismissal")
1755-
}
1756-
require.NotEmpty(t, notifications)
1757-
threadID := notifications[0].ID
1758-
1759-
// Dismiss notification (mark as read)
1760-
dismissReq := mcp.CallToolRequest{}
1761-
dismissReq.Params.Name = "dismiss_notification"
1762-
dismissReq.Params.Arguments = map[string]any{
1763-
"threadID": threadID,
1764-
"state": "read",
1765-
}
1766-
resp, err = client.CallTool(ctx, dismissReq)
1767-
require.NoError(t, err)
1768-
require.False(t, resp.IsError)
1769-
textContent, ok = resp.Content[0].(mcp.TextContent)
1770-
require.True(t, ok)
1771-
require.Contains(t, textContent.Text, "read")
1772-
}
1773-
1774-
func TestMarkAllNotificationsRead(t *testing.T) {
1775-
skipIfGlobalMutationNotOptedIn(t)
1776-
t.Parallel()
1777-
client := setupMCPClient(t)
1778-
ctx := context.Background()
1779-
1780-
// Limit to notifications updated within the last hour
1781-
oneHourAgo := nowMinusOneHourRFC3339()
1782-
markAllReq := mcp.CallToolRequest{}
1783-
markAllReq.Params.Name = "mark_all_notifications_read"
1784-
markAllReq.Params.Arguments = map[string]any{
1785-
"since": oneHourAgo,
1786-
}
1787-
resp, err := client.CallTool(ctx, markAllReq)
1788-
require.NoError(t, err)
1789-
require.False(t, resp.IsError)
1790-
textContent, ok := resp.Content[0].(mcp.TextContent)
1791-
require.True(t, ok)
1792-
require.Contains(t, textContent.Text, "All notifications marked as read")
1793-
1794-
}
1795-
1796-
// nowMinusOneHourRFC3339 returns the RFC3339 timestamp for one hour ago from now (UTC)
1797-
func nowMinusOneHourRFC3339() string {
1798-
return time.Now().UTC().Add(-1 * time.Hour).Format(time.RFC3339)
1799-
}
1800-
1801-
func TestGetNotificationDetails(t *testing.T) {
1802-
t.Parallel()
1803-
client := setupMCPClient(t)
1804-
ctx := context.Background()
1805-
1806-
// List notifications to get a valid notificationID
1807-
listReq := mcp.CallToolRequest{}
1808-
listReq.Params.Name = "list_notifications"
1809-
listReq.Params.Arguments = map[string]any{}
1810-
resp, err := client.CallTool(ctx, listReq)
1811-
require.NoError(t, err)
1812-
require.False(t, resp.IsError)
1813-
textContent, ok := resp.Content[0].(mcp.TextContent)
1814-
require.True(t, ok)
1815-
var notifications []struct {
1816-
ID string `json:"id"`
1817-
}
1818-
err = json.Unmarshal([]byte(textContent.Text), &notifications)
1819-
require.NoError(t, err)
1820-
require.NotEmpty(t, notifications)
1821-
if len(notifications) == 0 {
1822-
t.Skip("No notifications available to test dismissal")
1823-
}
1824-
notificationID := notifications[0].ID
1825-
1826-
// Get notification details
1827-
detailsReq := mcp.CallToolRequest{}
1828-
detailsReq.Params.Name = "get_notification_details"
1829-
detailsReq.Params.Arguments = map[string]any{
1830-
"notificationID": notificationID,
1831-
}
1832-
resp, err = client.CallTool(ctx, detailsReq)
1833-
require.NoError(t, err)
1834-
require.False(t, resp.IsError)
1835-
textContent, ok = resp.Content[0].(mcp.TextContent)
1836-
require.True(t, ok)
1837-
require.Contains(t, textContent.Text, notificationID)
1838-
}

0 commit comments

Comments
 (0)