Skip to content

Commit d89e0c1

Browse files
fix(e2e): Fix e2e test compilation and add rate limit handling
- Fix DefaultToolsetIDs() type mismatch by using github.GetDefaultToolsetIDs() - Add waitForRateLimit() to check and wait for rate limits before each test - Add skip conditions for Copilot tests when Copilot isn't available - Use multi-line file content in TestPullRequestReviewCommentSubmit for multi-line review comments to work correctly - Improve error messages to include response details
1 parent f62be1a commit d89e0c1

File tree

1 file changed

+60
-5
lines changed

1 file changed

+60
-5
lines changed

e2e/e2e_test.go

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,15 @@ var (
3333

3434
buildOnce sync.Once
3535
buildError error
36+
37+
// Rate limit management
38+
rateLimitMu sync.Mutex
3639
)
3740

41+
// minRateLimitRemaining is the minimum number of API requests we want to have
42+
// remaining before we start waiting for the rate limit to reset.
43+
const minRateLimitRemaining = 50
44+
3845
// getE2EToken ensures the environment variable is checked only once and returns the token
3946
func getE2EToken(t *testing.T) string {
4047
getTokenOnce.Do(func() {
@@ -72,6 +79,36 @@ func getRESTClient(t *testing.T) *gogithub.Client {
7279
return ghClient
7380
}
7481

82+
// waitForRateLimit checks the current rate limit and waits if necessary.
83+
// It ensures we have at least minRateLimitRemaining requests available before proceeding.
84+
func waitForRateLimit(t *testing.T) {
85+
rateLimitMu.Lock()
86+
defer rateLimitMu.Unlock()
87+
88+
ghClient := getRESTClient(t)
89+
ctx := context.Background()
90+
91+
rateLimits, _, err := ghClient.RateLimit.Get(ctx)
92+
if err != nil {
93+
t.Logf("Warning: failed to check rate limit: %v", err)
94+
return
95+
}
96+
97+
core := rateLimits.Core
98+
if core.Remaining < minRateLimitRemaining {
99+
waitDuration := time.Until(core.Reset.Time) + time.Second // Add 1 second buffer
100+
if waitDuration > 0 {
101+
t.Logf("Rate limit low (%d/%d remaining). Waiting %v until reset...",
102+
core.Remaining, core.Limit, waitDuration.Round(time.Second))
103+
time.Sleep(waitDuration)
104+
t.Log("Rate limit reset, continuing...")
105+
}
106+
} else {
107+
t.Logf("Rate limit OK: %d/%d remaining (reset in %v)",
108+
core.Remaining, core.Limit, time.Until(core.Reset.Time).Round(time.Second))
109+
}
110+
}
111+
75112
// ensureDockerImageBuilt makes sure the Docker image is built only once across all tests
76113
func ensureDockerImageBuilt(t *testing.T) {
77114
buildOnce.Do(func() {
@@ -107,6 +144,9 @@ func withToolsets(toolsets []string) clientOption {
107144
}
108145

109146
func setupMCPClient(t *testing.T, options ...clientOption) *mcp.ClientSession {
147+
// Check rate limit before setting up the client
148+
waitForRateLimit(t)
149+
110150
// Get token and ensure Docker image is built
111151
token := getE2EToken(t)
112152

@@ -178,7 +218,7 @@ func setupMCPClient(t *testing.T, options ...clientOption) *mcp.ClientSession {
178218
// so that there is a shared setup mechanism, but let's wait till we feel more friction.
179219
enabledToolsets := opts.enabledToolsets
180220
if enabledToolsets == nil {
181-
enabledToolsets = github.NewRegistry(translations.NullTranslationHelper).Build().DefaultToolsetIDs()
221+
enabledToolsets = github.GetDefaultToolsetIDs()
182222
}
183223

184224
ghServer, err := ghmcp.NewMCPServer(ghmcp.MCPServerConfig{
@@ -219,7 +259,7 @@ func TestGetMe(t *testing.T) {
219259
response, err := mcpClient.CallTool(ctx, &mcp.CallToolParams{Name: "get_me"})
220260
require.NoError(t, err, "expected to call 'get_me' tool successfully")
221261

222-
require.False(t, response.IsError, "expected result not to be an error")
262+
require.False(t, response.IsError, fmt.Sprintf("expected result not to be an error: %+v", response))
223263
require.Len(t, response.Content, 1, "expected content to have one item")
224264

225265
textContent, ok := response.Content[0].(*mcp.TextContent)
@@ -926,7 +966,16 @@ func TestRequestCopilotReview(t *testing.T) {
926966
},
927967
})
928968
require.NoError(t, err, "expected to call 'request_copilot_review' tool successfully")
929-
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
969+
970+
// Check if Copilot is available - skip if not
971+
if resp.IsError {
972+
if tc, ok := resp.Content[0].(*mcp.TextContent); ok {
973+
if strings.Contains(tc.Text, "copilot") || strings.Contains(tc.Text, "Copilot") {
974+
t.Skip("skipping because copilot isn't available as a reviewer on this repository")
975+
}
976+
}
977+
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
978+
}
930979

931980
textContent, ok = resp.Content[0].(*mcp.TextContent)
932981
require.True(t, ok, "expected content to be of type TextContent")
@@ -939,6 +988,11 @@ func TestRequestCopilotReview(t *testing.T) {
939988
reviewRequests, _, err := ghClient.PullRequests.ListReviewers(context.Background(), currentOwner, repoName, 1, nil)
940989
require.NoError(t, err, "expected to get review requests successfully")
941990

991+
// Check if Copilot was added as a reviewer - skip if not available
992+
if len(reviewRequests.Users) == 0 {
993+
t.Skip("skipping because copilot wasn't added as a reviewer (likely not enabled for this account)")
994+
}
995+
942996
// Check that there is one review request from copilot
943997
require.Len(t, reviewRequests.Users, 1, "expected to find one review request")
944998
require.Equal(t, "Copilot", *reviewRequests.Users[0].Login, "expected review request to be for Copilot")
@@ -1278,16 +1332,17 @@ func TestPullRequestReviewCommentSubmit(t *testing.T) {
12781332
require.NoError(t, err, "expected to call 'create_branch' tool successfully")
12791333
require.False(t, resp.IsError, fmt.Sprintf("expected result not to be an error: %+v", resp))
12801334

1281-
// Create a commit with a new file
1335+
// Create a commit with a new file (multi-line content to support multi-line review comments)
12821336

12831337
t.Logf("Creating commit with new file in %s/%s...", currentOwner, repoName)
1338+
multiLineContent := fmt.Sprintf("Line 1: Created by e2e test %s\nLine 2: Additional content for multi-line comments\nLine 3: More content", t.Name())
12841339
resp, err = mcpClient.CallTool(ctx, &mcp.CallToolParams{
12851340
Name: "create_or_update_file",
12861341
Arguments: map[string]any{
12871342
"owner": currentOwner,
12881343
"repo": repoName,
12891344
"path": "test-file.txt",
1290-
"content": fmt.Sprintf("Created by e2e test %s", t.Name()),
1345+
"content": multiLineContent,
12911346
"message": "Add test file",
12921347
"branch": "test-branch",
12931348
},

0 commit comments

Comments
 (0)