Skip to content

Fix handling of 405 merge in progress #13770

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 26 additions & 15 deletions .ci/magician/cmd/delete_branches_test.go
Original file line number Diff line number Diff line change
@@ -1,32 +1,43 @@
package cmd

import (
"magician/exec"
"magician/github"
"os"
"testing"
)

func TestFetchPRNumber(t *testing.T) {
rnr, err := exec.NewRunner()
if err != nil {
t.Errorf("error creating Runner: %s", err)
}

githubToken, ok := os.LookupEnv("GITHUB_TOKEN_CLASSIC")
if !ok {
t.Errorf("did not provide GITHUB_TOKEN_CLASSIC environment variable")
mr := NewMockRunner()
gh := &mockGithub{
calledMethods: make(map[string][][]any),
commitMessage: "Add `additional_group_keys` attribute to `google_cloud_identity_group` resource (#9217) (#6504)\n\n* Add `additional_group_keys` attribute to `google_cloud_identity_group` resource\n\n* Update acceptance test to check for attribute\n\n* Fix test check\n\n* Add `output: true` to nested properties in output field\n[upstream:49d3741f9d4d810a0a4768363bb8498afa21c688]\n\nSigned-off-by: Modular Magician <[email protected]>",
}

gh := github.NewClient(githubToken)

prNumber, err := fetchPRNumber("8c6e61bb62d52c950008340deafc1e2a2041898a", "main", rnr, gh)

// Call function with mocks
prNumber, err := fetchPRNumber("8c6e61bb62d52c950008340deafc1e2a2041898a", "main", mr, gh)
if err != nil {
t.Errorf("error fetching PR number: %s", err)
}

if prNumber != "6504" {
t.Errorf("PR number is %s, expected 6504", prNumber)
}

// Verify GitHub API was called
if calls, ok := gh.calledMethods["GetCommitMessage"]; !ok || len(calls) == 0 {
t.Errorf("Expected GetCommitMessage to be called")
} else {
args := calls[0]
if len(args) != 3 {
t.Errorf("Expected GetCommitMessage to be called with 3 arguments, got %d", len(args))
} else {
if args[0] != "hashicorp" {
t.Errorf("Expected owner to be 'hashicorp', got '%s'", args[0])
}
if args[1] != "terraform-provider-google-beta" {
t.Errorf("Expected repo to be 'terraform-provider-google-beta', got '%s'", args[1])
}
if args[2] != "8c6e61bb62d52c950008340deafc1e2a2041898a" {
t.Errorf("Expected SHA to be '8c6e61bb62d52c950008340deafc1e2a2041898a', got '%s'", args[2])
}
}
}
}
3 changes: 2 additions & 1 deletion .ci/magician/cmd/mock_github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ type mockGithub struct {
pullRequestComments []github.PullRequestComment
teamMembers map[string][]github.User
calledMethods map[string][][]any
commitMessage string
}

func (m *mockGithub) GetPullRequest(prNumber string) (github.PullRequest, error) {
Expand Down Expand Up @@ -63,7 +64,7 @@ func (m *mockGithub) GetPullRequestComments(prNumber string) ([]github.PullReque

func (m *mockGithub) GetCommitMessage(owner, repo, sha string) (string, error) {
m.calledMethods["GetCommitMessage"] = append(m.calledMethods["GetCommitMessage"], []any{owner, repo, sha})
return "commit message", nil
return m.commitMessage, nil
}

func (m *mockGithub) GetTeamMembers(organization, team string) ([]github.User, error) {
Expand Down
6 changes: 5 additions & 1 deletion .ci/magician/github/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"fmt"
utils "magician/utility"
"strings"
"time"
)

func (gh *Client) PostBuildStatus(prNumber, title, state, targetURL, commitSha string) error {
Expand Down Expand Up @@ -165,7 +166,10 @@ func (gh *Client) MergePullRequest(owner, repo, prNumber, commitSha string) erro
// Check if the error is "Merge already in progress" (405)
if strings.Contains(err.Error(), "Merge already in progress") {
fmt.Printf("Pull request %s is already being merged\n", prNumber)
return nil
// This status does not indicate that the Pull Request was merged
// Try again after 20s
time.Sleep(20 * time.Second)
return gh.MergePullRequest(owner, repo, prNumber, commitSha)
}
// Check if the PR is already merged (returns 405 Pull Request is not mergeable)
if strings.Contains(err.Error(), "Pull Request is not mergeable") {
Expand Down
2 changes: 2 additions & 0 deletions .ci/magician/utility/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ func makeHTTPRequest(url, method, credentials string, body any) (*http.Response,
req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", credentials))
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Accept", "application/json")

fmt.Println("")
fmt.Println("request url: ", url)
fmt.Println("request body: ", string(jsonBody))
Expand All @@ -79,6 +80,7 @@ func makeHTTPRequest(url, method, credentials string, body any) (*http.Response,
}

fmt.Println("response status-code: ", resp.StatusCode)
fmt.Println("response body: ", string(respBodyBytes))
fmt.Println("")

return resp, respBodyBytes, nil
Expand Down
Loading