From 953a462bfb473702df86f608fb007a5845cb5d58 Mon Sep 17 00:00:00 2001 From: Emma Sax Date: Fri, 9 Feb 2024 15:36:06 -0600 Subject: [PATCH 1/4] Remove support for Contexts on RequiredStatusChecks --- github/repos.go | 10 +- github/repos_test.go | 224 +++---------------------------------------- 2 files changed, 15 insertions(+), 219 deletions(-) diff --git a/github/repos.go b/github/repos.go index b492e55b46..298008df50 100644 --- a/github/repos.go +++ b/github/repos.go @@ -1191,12 +1191,8 @@ type RequiredStatusChecks struct { // Require branches to be up to date before merging. (Required.) Strict bool `json:"strict"` // The list of status checks to require in order to merge into this - // branch. (Deprecated. Note: only one of Contexts/Checks can be populated, - // but at least one must be populated). - Contexts []string `json:"contexts,omitempty"` - // The list of status checks to require in order to merge into this - // branch. - Checks []*RequiredStatusCheck `json:"checks,omitempty"` + // branch. An empty slice is valid. (Required.) + Checks []*RequiredStatusCheck `json:"checks"` ContextsURL *string `json:"contexts_url,omitempty"` URL *string `json:"url,omitempty"` } @@ -1204,7 +1200,7 @@ type RequiredStatusChecks struct { // RequiredStatusChecksRequest represents a request to edit a protected branch's status checks. type RequiredStatusChecksRequest struct { Strict *bool `json:"strict,omitempty"` - // Note: if both Contexts and Checks are populated, + // Deprecated. Note: if both Contexts and Checks are populated, // the GitHub API will only use Checks. Contexts []string `json:"contexts,omitempty"` Checks []*RequiredStatusCheck `json:"checks,omitempty"` diff --git a/github/repos_test.go b/github/repos_test.go index 2e61aeb1b1..a0aa7ec89c 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1146,7 +1146,6 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, - "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1206,8 +1205,7 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "continuous-integration", @@ -1301,7 +1299,6 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, - "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1333,8 +1330,7 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "continuous-integration", @@ -1404,194 +1400,6 @@ func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T } } -func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) { - tests := []struct { - branch string - urlPath string - }{ - {branch: "b", urlPath: "/repos/o/r/branches/b/protection"}, - {branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"}, - } - - for _, test := range tests { - t.Run(test.branch, func(t *testing.T) { - client, mux, _, teardown := setup() - defer teardown() - - input := &ProtectionRequest{ - RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, - }, - RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ - DismissStaleReviews: true, - DismissalRestrictionsRequest: &DismissalRestrictionsRequest{ - Users: &[]string{"uu"}, - Teams: &[]string{"tt"}, - Apps: &[]string{"aa"}, - }, - BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{ - Users: []string{"uuu"}, - Teams: []string{"ttt"}, - Apps: []string{"aaa"}, - }, - }, - Restrictions: &BranchRestrictionsRequest{ - Users: []string{"u"}, - Teams: []string{"t"}, - Apps: []string{"a"}, - }, - BlockCreations: Bool(true), - LockBranch: Bool(true), - AllowForkSyncing: Bool(true), - } - - mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { - v := new(ProtectionRequest) - assertNilError(t, json.NewDecoder(r.Body).Decode(v)) - - testMethod(t, r, "PUT") - if !cmp.Equal(v, input) { - t.Errorf("Request body = %+v, want %+v", v, input) - } - - // TODO: remove custom Accept header when this API fully launches - testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview) - fmt.Fprintf(w, `{ - "required_status_checks":{ - "strict":true, - "contexts":["continuous-integration"], - "checks": [ - { - "context": "continuous-integration", - "app_id": null - } - ] - }, - "required_pull_request_reviews":{ - "dismissal_restrictions":{ - "users":[{ - "id":3, - "login":"uu" - }], - "teams":[{ - "id":4, - "slug":"tt" - }], - "apps":[{ - "id":5, - "slug":"aa" - }] - }, - "dismiss_stale_reviews":true, - "require_code_owner_reviews":true, - "bypass_pull_request_allowances": { - "users":[{"id":10,"login":"uuu"}], - "teams":[{"id":20,"slug":"ttt"}], - "apps":[{"id":30,"slug":"aaa"}] - } - }, - "restrictions":{ - "users":[{"id":1,"login":"u"}], - "teams":[{"id":2,"slug":"t"}], - "apps":[{"id":3,"slug":"a"}] - }, - "block_creations": { - "enabled": true - }, - "lock_branch": { - "enabled": true - }, - "allow_fork_syncing": { - "enabled": true - } - }`) - }) - - ctx := context.Background() - protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) - if err != nil { - t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err) - } - - want := &Protection{ - RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, - Checks: []*RequiredStatusCheck{ - { - Context: "continuous-integration", - }, - }, - }, - RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ - DismissStaleReviews: true, - DismissalRestrictions: &DismissalRestrictions{ - Users: []*User{ - {Login: String("uu"), ID: Int64(3)}, - }, - Teams: []*Team{ - {Slug: String("tt"), ID: Int64(4)}, - }, - Apps: []*App{ - {Slug: String("aa"), ID: Int64(5)}, - }, - }, - RequireCodeOwnerReviews: true, - BypassPullRequestAllowances: &BypassPullRequestAllowances{ - Users: []*User{ - {Login: String("uuu"), ID: Int64(10)}, - }, - Teams: []*Team{ - {Slug: String("ttt"), ID: Int64(20)}, - }, - Apps: []*App{ - {Slug: String("aaa"), ID: Int64(30)}, - }, - }, - }, - Restrictions: &BranchRestrictions{ - Users: []*User{ - {Login: String("u"), ID: Int64(1)}, - }, - Teams: []*Team{ - {Slug: String("t"), ID: Int64(2)}, - }, - Apps: []*App{ - {Slug: String("a"), ID: Int64(3)}, - }, - }, - BlockCreations: &BlockCreations{ - Enabled: Bool(true), - }, - LockBranch: &LockBranch{ - Enabled: Bool(true), - }, - AllowForkSyncing: &AllowForkSyncing{ - Enabled: Bool(true), - }, - } - if !cmp.Equal(protection, want) { - t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want) - } - - const methodName = "UpdateBranchProtection" - testBadOptions(t, methodName, func() (err error) { - _, _, err = client.Repositories.UpdateBranchProtection(ctx, "\n", "\n", "\n", input) - return err - }) - - testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { - got, resp, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) - if got != nil { - t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) - } - return resp, err - }) - }) - } -} - func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { tests := []struct { branch string @@ -1649,7 +1457,6 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, - "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1696,8 +1503,7 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "continuous-integration", @@ -1749,7 +1555,7 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { } } -func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) { +func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { tests := []struct { branch string urlPath string @@ -1765,6 +1571,7 @@ func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) input := &ProtectionRequest{ RequiredStatusChecks: &RequiredStatusChecks{ + Checks: []*RequiredStatusCheck{}, Strict: true, }, RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ @@ -1801,7 +1608,7 @@ func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, - "contexts":[] + "checks":[] }, "required_pull_request_reviews":{ "dismissal_restrictions":{ @@ -1843,8 +1650,8 @@ func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Contexts: []string{}, + Checks: []*RequiredStatusCheck{}, + Strict: true, }, RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ DismissStaleReviews: true, @@ -2056,7 +1863,6 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) { testMethod(t, r, "GET") fmt.Fprint(w, `{ "strict": true, - "contexts": ["x","y","z"], "checks": [ { "context": "x", @@ -2081,8 +1887,7 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"x", "y", "z"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "x", @@ -2169,8 +1974,7 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { defer teardown() input := &RequiredStatusChecksRequest{ - Strict: Bool(true), - Contexts: []string{"continuous-integration"}, + Strict: Bool(true), } mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { @@ -2184,7 +1988,6 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { testHeader(t, r, "Accept", mediaTypeV3) fmt.Fprintf(w, `{ "strict":true, - "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -2201,8 +2004,7 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "continuous-integration", @@ -2274,7 +2076,6 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) { testHeader(t, r, "Accept", mediaTypeV3) fmt.Fprintf(w, `{ "strict":true, - "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -2299,8 +2100,7 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Contexts: []string{"continuous-integration"}, + Strict: true, Checks: []*RequiredStatusCheck{ { Context: "continuous-integration", From 95845c93a55dcdf2104354f56b1eabc8143e001b Mon Sep 17 00:00:00 2001 From: Emma Sax Date: Fri, 9 Feb 2024 16:23:13 -0600 Subject: [PATCH 2/4] Switch to pointers --- github/repos.go | 8 +- github/repos_test.go | 557 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 545 insertions(+), 20 deletions(-) diff --git a/github/repos.go b/github/repos.go index 298008df50..1f63d4f82e 100644 --- a/github/repos.go +++ b/github/repos.go @@ -1191,8 +1191,12 @@ type RequiredStatusChecks struct { // Require branches to be up to date before merging. (Required.) Strict bool `json:"strict"` // The list of status checks to require in order to merge into this - // branch. An empty slice is valid. (Required.) - Checks []*RequiredStatusCheck `json:"checks"` + // branch. An empty slice is valid. (Deprecated. Note: only one of + // Contexts/Checks can be populated, but at least one must be populated). + Contexts *[]string `json:"contexts,omitempty"` + // The list of status checks to require in order to merge into this + // branch. An empty slice is valid. + Checks *[]*RequiredStatusCheck `json:"checks,omitempty"` ContextsURL *string `json:"contexts_url,omitempty"` URL *string `json:"url,omitempty"` } diff --git a/github/repos_test.go b/github/repos_test.go index a0aa7ec89c..2f7f6897ef 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -1146,6 +1146,7 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, + "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1205,8 +1206,9 @@ func TestRepositoriesService_GetBranchProtection(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, @@ -1299,6 +1301,7 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, + "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1330,8 +1333,9 @@ func TestRepositoriesService_GetBranchProtection_noDismissalRestrictions(t *test want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, @@ -1400,6 +1404,372 @@ func TestRepositoriesService_GetBranchProtection_branchNotProtected(t *testing.T } } +func TestRepositoriesService_UpdateBranchProtection_Contexts(t *testing.T) { + tests := []struct { + branch string + urlPath string + }{ + {branch: "b", urlPath: "/repos/o/r/branches/b/protection"}, + {branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"}, + } + + for _, test := range tests { + t.Run(test.branch, func(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + input := &ProtectionRequest{ + RequiredStatusChecks: &RequiredStatusChecks{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ + DismissStaleReviews: true, + DismissalRestrictionsRequest: &DismissalRestrictionsRequest{ + Users: &[]string{"uu"}, + Teams: &[]string{"tt"}, + Apps: &[]string{"aa"}, + }, + BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{ + Users: []string{"uuu"}, + Teams: []string{"ttt"}, + Apps: []string{"aaa"}, + }, + }, + Restrictions: &BranchRestrictionsRequest{ + Users: []string{"u"}, + Teams: []string{"t"}, + Apps: []string{"a"}, + }, + BlockCreations: Bool(true), + LockBranch: Bool(true), + AllowForkSyncing: Bool(true), + } + + mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { + v := new(ProtectionRequest) + assertNilError(t, json.NewDecoder(r.Body).Decode(v)) + + testMethod(t, r, "PUT") + if !cmp.Equal(v, input) { + t.Errorf("Request body = %+v, want %+v", v, input) + } + + // TODO: remove custom Accept header when this API fully launches + testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview) + fmt.Fprintf(w, `{ + "required_status_checks":{ + "strict":true, + "contexts":["continuous-integration"], + "checks": [ + { + "context": "continuous-integration", + "app_id": null + } + ] + }, + "required_pull_request_reviews":{ + "dismissal_restrictions":{ + "users":[{ + "id":3, + "login":"uu" + }], + "teams":[{ + "id":4, + "slug":"tt" + }], + "apps":[{ + "id":5, + "slug":"aa" + }] + }, + "dismiss_stale_reviews":true, + "require_code_owner_reviews":true, + "bypass_pull_request_allowances": { + "users":[{"id":10,"login":"uuu"}], + "teams":[{"id":20,"slug":"ttt"}], + "apps":[{"id":30,"slug":"aaa"}] + } + }, + "restrictions":{ + "users":[{"id":1,"login":"u"}], + "teams":[{"id":2,"slug":"t"}], + "apps":[{"id":3,"slug":"a"}] + }, + "block_creations": { + "enabled": true + }, + "lock_branch": { + "enabled": true + }, + "allow_fork_syncing": { + "enabled": true + } + }`) + }) + + ctx := context.Background() + protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) + if err != nil { + t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err) + } + + want := &Protection{ + RequiredStatusChecks: &RequiredStatusChecks{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ + { + Context: "continuous-integration", + }, + }, + }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ + DismissStaleReviews: true, + DismissalRestrictions: &DismissalRestrictions{ + Users: []*User{ + {Login: String("uu"), ID: Int64(3)}, + }, + Teams: []*Team{ + {Slug: String("tt"), ID: Int64(4)}, + }, + Apps: []*App{ + {Slug: String("aa"), ID: Int64(5)}, + }, + }, + RequireCodeOwnerReviews: true, + BypassPullRequestAllowances: &BypassPullRequestAllowances{ + Users: []*User{ + {Login: String("uuu"), ID: Int64(10)}, + }, + Teams: []*Team{ + {Slug: String("ttt"), ID: Int64(20)}, + }, + Apps: []*App{ + {Slug: String("aaa"), ID: Int64(30)}, + }, + }, + }, + Restrictions: &BranchRestrictions{ + Users: []*User{ + {Login: String("u"), ID: Int64(1)}, + }, + Teams: []*Team{ + {Slug: String("t"), ID: Int64(2)}, + }, + Apps: []*App{ + {Slug: String("a"), ID: Int64(3)}, + }, + }, + BlockCreations: &BlockCreations{ + Enabled: Bool(true), + }, + LockBranch: &LockBranch{ + Enabled: Bool(true), + }, + AllowForkSyncing: &AllowForkSyncing{ + Enabled: Bool(true), + }, + } + if !cmp.Equal(protection, want) { + t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want) + } + + const methodName = "UpdateBranchProtection" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.UpdateBranchProtection(ctx, "\n", "\n", "\n", input) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) + }) + } +} + +func TestRepositoriesService_UpdateBranchProtection_EmptyContexts(t *testing.T) { + tests := []struct { + branch string + urlPath string + }{ + {branch: "b", urlPath: "/repos/o/r/branches/b/protection"}, + {branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"}, + } + + for _, test := range tests { + t.Run(test.branch, func(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + input := &ProtectionRequest{ + RequiredStatusChecks: &RequiredStatusChecks{ + Strict: true, + Contexts: &[]string{}, + }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ + DismissStaleReviews: true, + DismissalRestrictionsRequest: &DismissalRestrictionsRequest{ + Users: &[]string{"uu"}, + Teams: &[]string{"tt"}, + Apps: &[]string{"aa"}, + }, + BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{ + Users: []string{"uuu"}, + Teams: []string{"ttt"}, + Apps: []string{"aaa"}, + }, + }, + Restrictions: &BranchRestrictionsRequest{ + Users: []string{"u"}, + Teams: []string{"t"}, + Apps: []string{"a"}, + }, + BlockCreations: Bool(true), + LockBranch: Bool(true), + AllowForkSyncing: Bool(true), + } + + mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { + v := new(ProtectionRequest) + assertNilError(t, json.NewDecoder(r.Body).Decode(v)) + + testMethod(t, r, "PUT") + if !cmp.Equal(v, input) { + t.Errorf("Request body = %+v, want %+v", v, input) + } + + // TODO: remove custom Accept header when this API fully launches + testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview) + fmt.Fprintf(w, `{ + "required_status_checks":{ + "strict":true, + "contexts":[], + "checks": null + }, + "required_pull_request_reviews":{ + "dismissal_restrictions":{ + "users":[{ + "id":3, + "login":"uu" + }], + "teams":[{ + "id":4, + "slug":"tt" + }], + "apps":[{ + "id":5, + "slug":"aa" + }] + }, + "dismiss_stale_reviews":true, + "require_code_owner_reviews":true, + "bypass_pull_request_allowances": { + "users":[{"id":10,"login":"uuu"}], + "teams":[{"id":20,"slug":"ttt"}], + "apps":[{"id":30,"slug":"aaa"}] + } + }, + "restrictions":{ + "users":[{"id":1,"login":"u"}], + "teams":[{"id":2,"slug":"t"}], + "apps":[{"id":3,"slug":"a"}] + }, + "block_creations": { + "enabled": true + }, + "lock_branch": { + "enabled": true + }, + "allow_fork_syncing": { + "enabled": true + } + }`) + }) + + ctx := context.Background() + protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) + if err != nil { + t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err) + } + + want := &Protection{ + RequiredStatusChecks: &RequiredStatusChecks{ + Strict: true, + Contexts: &[]string{}, + }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ + DismissStaleReviews: true, + DismissalRestrictions: &DismissalRestrictions{ + Users: []*User{ + {Login: String("uu"), ID: Int64(3)}, + }, + Teams: []*Team{ + {Slug: String("tt"), ID: Int64(4)}, + }, + Apps: []*App{ + {Slug: String("aa"), ID: Int64(5)}, + }, + }, + RequireCodeOwnerReviews: true, + BypassPullRequestAllowances: &BypassPullRequestAllowances{ + Users: []*User{ + {Login: String("uuu"), ID: Int64(10)}, + }, + Teams: []*Team{ + {Slug: String("ttt"), ID: Int64(20)}, + }, + Apps: []*App{ + {Slug: String("aaa"), ID: Int64(30)}, + }, + }, + }, + Restrictions: &BranchRestrictions{ + Users: []*User{ + {Login: String("u"), ID: Int64(1)}, + }, + Teams: []*Team{ + {Slug: String("t"), ID: Int64(2)}, + }, + Apps: []*App{ + {Slug: String("a"), ID: Int64(3)}, + }, + }, + BlockCreations: &BlockCreations{ + Enabled: Bool(true), + }, + LockBranch: &LockBranch{ + Enabled: Bool(true), + }, + AllowForkSyncing: &AllowForkSyncing{ + Enabled: Bool(true), + }, + } + if !cmp.Equal(protection, want) { + t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want) + } + + const methodName = "UpdateBranchProtection" + testBadOptions(t, methodName, func() (err error) { + _, _, err = client.Repositories.UpdateBranchProtection(ctx, "\n", "\n", "\n", input) + return err + }) + + testNewRequestAndDoFailure(t, methodName, client, func() (*Response, error) { + got, resp, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) + if got != nil { + t.Errorf("testNewRequestAndDoFailure %v = %#v, want nil", methodName, got) + } + return resp, err + }) + }) + } +} + func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { tests := []struct { branch string @@ -1417,7 +1787,7 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { input := &ProtectionRequest{ RequiredStatusChecks: &RequiredStatusChecks{ Strict: true, - Checks: []*RequiredStatusCheck{ + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, @@ -1457,6 +1827,7 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, + "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -1503,8 +1874,9 @@ func TestRepositoriesService_UpdateBranchProtection_Checks(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, @@ -1571,8 +1943,8 @@ func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { input := &ProtectionRequest{ RequiredStatusChecks: &RequiredStatusChecks{ - Checks: []*RequiredStatusCheck{}, Strict: true, + Checks: &[]*RequiredStatusCheck{}, }, RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ DismissStaleReviews: true, @@ -1608,7 +1980,8 @@ func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { fmt.Fprintf(w, `{ "required_status_checks":{ "strict":true, - "checks":[] + "contexts":null, + "checks": [] }, "required_pull_request_reviews":{ "dismissal_restrictions":{ @@ -1627,7 +2000,6 @@ func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { }, "dismiss_stale_reviews":true, "require_code_owner_reviews":true, - "require_last_push_approval":false, "bypass_pull_request_allowances": { "users":[{"id":10,"login":"uuu"}], "teams":[{"id":20,"slug":"ttt"}], @@ -1650,9 +2022,151 @@ func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Checks: []*RequiredStatusCheck{}, + Strict: true, + Checks: &[]*RequiredStatusCheck{}, + }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ + DismissStaleReviews: true, + DismissalRestrictions: &DismissalRestrictions{ + Users: []*User{ + {Login: String("uu"), ID: Int64(3)}, + }, + Teams: []*Team{ + {Slug: String("tt"), ID: Int64(4)}, + }, + Apps: []*App{ + {Slug: String("aa"), ID: Int64(5)}, + }, + }, + RequireCodeOwnerReviews: true, + BypassPullRequestAllowances: &BypassPullRequestAllowances{ + Users: []*User{ + {Login: String("uuu"), ID: Int64(10)}, + }, + Teams: []*Team{ + {Slug: String("ttt"), ID: Int64(20)}, + }, + Apps: []*App{ + {Slug: String("aaa"), ID: Int64(30)}, + }, + }, + }, + Restrictions: &BranchRestrictions{ + Users: []*User{ + {Login: String("u"), ID: Int64(1)}, + }, + Teams: []*Team{ + {Slug: String("t"), ID: Int64(2)}, + }, + Apps: []*App{ + {Slug: String("a"), ID: Int64(3)}, + }, + }, + } + if !cmp.Equal(protection, want) { + t.Errorf("Repositories.UpdateBranchProtection returned %+v, want %+v", protection, want) + } + }) + } +} + +func TestRepositoriesService_UpdateBranchProtection_StrictNoChecks(t *testing.T) { + tests := []struct { + branch string + urlPath string + }{ + {branch: "b", urlPath: "/repos/o/r/branches/b/protection"}, + {branch: "feat/branch-50%", urlPath: "/repos/o/r/branches/feat/branch-50%/protection"}, + } + + for _, test := range tests { + t.Run(test.branch, func(t *testing.T) { + client, mux, _, teardown := setup() + defer teardown() + + input := &ProtectionRequest{ + RequiredStatusChecks: &RequiredStatusChecks{ Strict: true, }, + RequiredPullRequestReviews: &PullRequestReviewsEnforcementRequest{ + DismissStaleReviews: true, + DismissalRestrictionsRequest: &DismissalRestrictionsRequest{ + Users: &[]string{"uu"}, + Teams: &[]string{"tt"}, + Apps: &[]string{"aa"}, + }, + BypassPullRequestAllowancesRequest: &BypassPullRequestAllowancesRequest{ + Users: []string{"uuu"}, + Teams: []string{"ttt"}, + Apps: []string{"aaa"}, + }, + }, + Restrictions: &BranchRestrictionsRequest{ + Users: []string{"u"}, + Teams: []string{"t"}, + Apps: []string{"a"}, + }, + } + + mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { + v := new(ProtectionRequest) + assertNilError(t, json.NewDecoder(r.Body).Decode(v)) + + testMethod(t, r, "PUT") + if !cmp.Equal(v, input) { + t.Errorf("Request body = %+v, want %+v", v, input) + } + + // TODO: remove custom Accept header when this API fully launches + testHeader(t, r, "Accept", mediaTypeRequiredApprovingReviewsPreview) + fmt.Fprintf(w, `{ + "required_status_checks":{ + "strict":true, + "contexts":[] + }, + "required_pull_request_reviews":{ + "dismissal_restrictions":{ + "users":[{ + "id":3, + "login":"uu" + }], + "teams":[{ + "id":4, + "slug":"tt" + }], + "apps":[{ + "id":5, + "slug":"aa" + }] + }, + "dismiss_stale_reviews":true, + "require_code_owner_reviews":true, + "require_last_push_approval":false, + "bypass_pull_request_allowances": { + "users":[{"id":10,"login":"uuu"}], + "teams":[{"id":20,"slug":"ttt"}], + "apps":[{"id":30,"slug":"aaa"}] + } + }, + "restrictions":{ + "users":[{"id":1,"login":"u"}], + "teams":[{"id":2,"slug":"t"}], + "apps":[{"id":3,"slug":"a"}] + } + }`) + }) + + ctx := context.Background() + protection, _, err := client.Repositories.UpdateBranchProtection(ctx, "o", "r", test.branch, input) + if err != nil { + t.Errorf("Repositories.UpdateBranchProtection returned error: %v", err) + } + + want := &Protection{ + RequiredStatusChecks: &RequiredStatusChecks{ + Strict: true, + Contexts: &[]string{}, + }, RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ DismissStaleReviews: true, DismissalRestrictions: &DismissalRestrictions{ @@ -1863,6 +2377,7 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) { testMethod(t, r, "GET") fmt.Fprint(w, `{ "strict": true, + "contexts": ["x","y","z"], "checks": [ { "context": "x", @@ -1887,8 +2402,9 @@ func TestRepositoriesService_GetRequiredStatusChecks(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"x", "y", "z"}, + Checks: &[]*RequiredStatusCheck{ { Context: "x", }, @@ -1974,7 +2490,8 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { defer teardown() input := &RequiredStatusChecksRequest{ - Strict: Bool(true), + Strict: Bool(true), + Contexts: []string{"continuous-integration"}, } mux.HandleFunc(test.urlPath, func(w http.ResponseWriter, r *http.Request) { @@ -1988,6 +2505,7 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { testHeader(t, r, "Accept", mediaTypeV3) fmt.Fprintf(w, `{ "strict":true, + "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -2004,8 +2522,9 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Contexts(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, @@ -2076,6 +2595,7 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) { testHeader(t, r, "Accept", mediaTypeV3) fmt.Fprintf(w, `{ "strict":true, + "contexts":["continuous-integration"], "checks": [ { "context": "continuous-integration", @@ -2100,8 +2620,9 @@ func TestRepositoriesService_UpdateRequiredStatusChecks_Checks(t *testing.T) { } want := &RequiredStatusChecks{ - Strict: true, - Checks: []*RequiredStatusCheck{ + Strict: true, + Contexts: &[]string{"continuous-integration"}, + Checks: &[]*RequiredStatusCheck{ { Context: "continuous-integration", }, From 36606c66aa6515f7b99762c6a3be3b47ccadbf16 Mon Sep 17 00:00:00 2001 From: Emma Sax Date: Fri, 9 Feb 2024 16:23:48 -0600 Subject: [PATCH 3/4] Respace --- github/repos.go | 6 +++--- github/repos_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/github/repos.go b/github/repos.go index 1f63d4f82e..2fb4c6f190 100644 --- a/github/repos.go +++ b/github/repos.go @@ -1193,12 +1193,12 @@ type RequiredStatusChecks struct { // The list of status checks to require in order to merge into this // branch. An empty slice is valid. (Deprecated. Note: only one of // Contexts/Checks can be populated, but at least one must be populated). - Contexts *[]string `json:"contexts,omitempty"` + Contexts *[]string `json:"contexts,omitempty"` // The list of status checks to require in order to merge into this // branch. An empty slice is valid. Checks *[]*RequiredStatusCheck `json:"checks,omitempty"` - ContextsURL *string `json:"contexts_url,omitempty"` - URL *string `json:"url,omitempty"` + ContextsURL *string `json:"contexts_url,omitempty"` + URL *string `json:"url,omitempty"` } // RequiredStatusChecksRequest represents a request to edit a protected branch's status checks. diff --git a/github/repos_test.go b/github/repos_test.go index 2f7f6897ef..0ea381ebb5 100644 --- a/github/repos_test.go +++ b/github/repos_test.go @@ -2022,7 +2022,7 @@ func TestRepositoriesService_UpdateBranchProtection_EmptyChecks(t *testing.T) { want := &Protection{ RequiredStatusChecks: &RequiredStatusChecks{ - Strict: true, + Strict: true, Checks: &[]*RequiredStatusCheck{}, }, RequiredPullRequestReviews: &PullRequestReviewsEnforcement{ From 4741b40ec82d01d9b1165ab7a2b1691778ef840f Mon Sep 17 00:00:00 2001 From: Emma Sax Date: Fri, 9 Feb 2024 22:33:52 -0600 Subject: [PATCH 4/4] Run script files to ensure linting and tests succeed --- github/github-accessors.go | 16 ++++++++++++++++ github/github-accessors_test.go | 20 ++++++++++++++++++++ test/integration/repos_test.go | 4 ++-- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/github/github-accessors.go b/github/github-accessors.go index e28fd27568..b645390e83 100644 --- a/github/github-accessors.go +++ b/github/github-accessors.go @@ -20246,6 +20246,22 @@ func (r *RequiredStatusCheck) GetAppID() int64 { return *r.AppID } +// GetChecks returns the Checks field if it's non-nil, zero value otherwise. +func (r *RequiredStatusChecks) GetChecks() []*RequiredStatusCheck { + if r == nil || r.Checks == nil { + return nil + } + return *r.Checks +} + +// GetContexts returns the Contexts field if it's non-nil, zero value otherwise. +func (r *RequiredStatusChecks) GetContexts() []string { + if r == nil || r.Contexts == nil { + return nil + } + return *r.Contexts +} + // GetContextsURL returns the ContextsURL field if it's non-nil, zero value otherwise. func (r *RequiredStatusChecks) GetContextsURL() string { if r == nil || r.ContextsURL == nil { diff --git a/github/github-accessors_test.go b/github/github-accessors_test.go index 4c349a05ae..8d5b438054 100644 --- a/github/github-accessors_test.go +++ b/github/github-accessors_test.go @@ -23517,6 +23517,26 @@ func TestRequiredStatusCheck_GetAppID(tt *testing.T) { r.GetAppID() } +func TestRequiredStatusChecks_GetChecks(tt *testing.T) { + var zeroValue []*RequiredStatusCheck + r := &RequiredStatusChecks{Checks: &zeroValue} + r.GetChecks() + r = &RequiredStatusChecks{} + r.GetChecks() + r = nil + r.GetChecks() +} + +func TestRequiredStatusChecks_GetContexts(tt *testing.T) { + var zeroValue []string + r := &RequiredStatusChecks{Contexts: &zeroValue} + r.GetContexts() + r = &RequiredStatusChecks{} + r.GetContexts() + r = nil + r.GetContexts() +} + func TestRequiredStatusChecks_GetContextsURL(tt *testing.T) { var zeroValue string r := &RequiredStatusChecks{ContextsURL: &zeroValue} diff --git a/test/integration/repos_test.go b/test/integration/repos_test.go index 79e8cdfe9f..9b32cc4a31 100644 --- a/test/integration/repos_test.go +++ b/test/integration/repos_test.go @@ -103,7 +103,7 @@ func TestRepositories_EditBranches(t *testing.T) { protectionRequest := &github.ProtectionRequest{ RequiredStatusChecks: &github.RequiredStatusChecks{ Strict: true, - Contexts: []string{"continuous-integration"}, + Contexts: &[]string{"continuous-integration"}, }, RequiredPullRequestReviews: &github.PullRequestReviewsEnforcementRequest{ DismissStaleReviews: true, @@ -126,7 +126,7 @@ func TestRepositories_EditBranches(t *testing.T) { want := &github.Protection{ RequiredStatusChecks: &github.RequiredStatusChecks{ Strict: true, - Contexts: []string{"continuous-integration"}, + Contexts: &[]string{"continuous-integration"}, }, RequiredPullRequestReviews: &github.PullRequestReviewsEnforcement{ DismissStaleReviews: true,