From 0e8c9d4305bd392546af05eede84b55bcff98849 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Jan 2022 14:28:43 +0000 Subject: [PATCH 1/2] Attempt to prevent intermittent failure TestGit/xxx/BranchProtectMerge/MergePR One of the repeated intermittent failures we see in testing is a failure due to branches not being ready to merge. Prior to the immediate queue implementation we would attempt to flush all the queues and this would prevent the issue. However, the immediate queue is not flushable so the flushall is not successful at preventing this. This PR proposes an alternative solution - wait some time and try again up to 5 times. If this fails then there is a genuine issue and we should fail. Related #17719 Signed-off-by: Andrew Thornton --- integrations/api_helper_for_declarative_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/integrations/api_helper_for_declarative_test.go b/integrations/api_helper_for_declarative_test.go index 9c6adaf084b71..d1e29ff6554f0 100644 --- a/integrations/api_helper_for_declarative_test.go +++ b/integrations/api_helper_for_declarative_test.go @@ -269,11 +269,15 @@ func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64) resp := ctx.Session.MakeRequest(t, req, NoExpectedStatus) - if resp.Code == http.StatusMethodNotAllowed { + for i := 0; i < 5; i++ { + if resp.Code != http.StatusMethodNotAllowed { + break + } err := api.APIError{} DecodeJSON(t, resp, &err) assert.EqualValues(t, "Please try again later", err.Message) queue.GetManager().FlushAll(context.Background(), 5*time.Second) + <-time.After(1 * time.Second) req = NewRequestWithJSON(t, http.MethodPost, urlStr, &forms.MergePullRequestForm{ MergeMessageField: "doAPIMergePullRequest Merge", Do: string(repo_model.MergeStyleMerge), From 38970c5b3ca5bf6545b851a00a5eb116e7019599 Mon Sep 17 00:00:00 2001 From: Andrew Thornton Date: Sat, 29 Jan 2022 15:09:35 +0000 Subject: [PATCH 2/2] as per review Signed-off-by: Andrew Thornton --- .../api_helper_for_declarative_test.go | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/integrations/api_helper_for_declarative_test.go b/integrations/api_helper_for_declarative_test.go index d1e29ff6554f0..7f2cd787c355f 100644 --- a/integrations/api_helper_for_declarative_test.go +++ b/integrations/api_helper_for_declarative_test.go @@ -8,6 +8,7 @@ import ( "context" "fmt" "net/http" + "net/http/httptest" "net/url" "os" "testing" @@ -262,14 +263,18 @@ func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64) return func(t *testing.T) { urlStr := fmt.Sprintf("/api/v1/repos/%s/%s/pulls/%d/merge?token=%s", owner, repo, index, ctx.Token) - req := NewRequestWithJSON(t, http.MethodPost, urlStr, &forms.MergePullRequestForm{ - MergeMessageField: "doAPIMergePullRequest Merge", - Do: string(repo_model.MergeStyleMerge), - }) - resp := ctx.Session.MakeRequest(t, req, NoExpectedStatus) + var req *http.Request + var resp *httptest.ResponseRecorder + + for i := 0; i < 6; i++ { + req = NewRequestWithJSON(t, http.MethodPost, urlStr, &forms.MergePullRequestForm{ + MergeMessageField: "doAPIMergePullRequest Merge", + Do: string(repo_model.MergeStyleMerge), + }) + + resp = ctx.Session.MakeRequest(t, req, NoExpectedStatus) - for i := 0; i < 5; i++ { if resp.Code != http.StatusMethodNotAllowed { break } @@ -278,11 +283,6 @@ func doAPIMergePullRequest(ctx APITestContext, owner, repo string, index int64) assert.EqualValues(t, "Please try again later", err.Message) queue.GetManager().FlushAll(context.Background(), 5*time.Second) <-time.After(1 * time.Second) - req = NewRequestWithJSON(t, http.MethodPost, urlStr, &forms.MergePullRequestForm{ - MergeMessageField: "doAPIMergePullRequest Merge", - Do: string(repo_model.MergeStyleMerge), - }) - resp = ctx.Session.MakeRequest(t, req, NoExpectedStatus) } expected := ctx.ExpectedCode