Merge branch 'trunk' into gh-attestation-tuf-client-retry

This commit is contained in:
Meredith Lancaster 2025-05-07 08:50:51 -06:00
commit c5a2defec8
63 changed files with 2475 additions and 476 deletions

View file

@ -309,7 +309,7 @@ jobs:
rpmsign --addsign dist/*.rpm
- name: Attest release artifacts
if: inputs.environment == 'production'
uses: actions/attest-build-provenance@bd77c077858b8d561b7a36cbe48ef4cc642ca39d # v2.2.2
uses: actions/attest-build-provenance@db473fddc028af60658334401dc6fa3ffd8669fd # v2.3.0
with:
subject-path: "dist/gh_*"
- name: Run createrepo

View file

@ -0,0 +1,50 @@
skip 'it creates a fork owned by the user running the test'
skip 'this never worked, but could be fixed if we fixed show-refs'
# Setup environment variables used for testscript
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
env FORK=${REPO}-fork
# Use gh as a credential helper
exec gh auth setup-git
# Get the current username for the fork owner
exec gh api user --jq .login
stdout2env USER
# Create a repository to act as upstream with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private
# Defer repo cleanup of upstream
defer gh repo delete --yes ${ORG}/${REPO}
# Create a user fork of repository. This will be owned by USER.
exec gh repo fork ${ORG}/${REPO} --fork-name ${FORK}
sleep 5
# Defer repo cleanup of fork
defer gh repo delete --yes ${USER}/${FORK}
# Retrieve fork repository information
exec gh repo view ${USER}/${FORK} --json id --jq '.id'
stdout2env FORK_ID
# Clone the repo
exec gh repo clone ${USER}/${FORK}
cd ${FORK}
# Configure push.default so that it should use the merge ref
exec git config push.default upstream
# But prepare a branch that doesn't have a tracking merge ref
exec git checkout -b feature-branch
exec git commit --allow-empty -m 'Empty Commit'
exec git push origin feature-branch
# Create the PR
exec gh pr create --title 'Feature Title' --body 'Feature Body'
stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1
# Assert that the PR was created with the correct head repository and refs
exec gh pr view --json headRefName,headRepository,baseRefName,isCrossRepository
stdout {"baseRefName":"main","headRefName":"feature-branch","headRepository":{"id":"${FORK_ID}","name":"${FORK}"},"isCrossRepository":true}

View file

@ -0,0 +1,33 @@
skip 'it creates a fork owned by the user running the test'
# Setup environment variables used for testscript
env REPO=${SCRIPT_NAME}-${RANDOM_STRING}
# Use gh as a credential helper
exec gh auth setup-git
# Get the current username for the fork owner
exec gh api user --jq .login
stdout2env USER
# Create a repository to act as upstream with a file so it has a default branch
exec gh repo create ${ORG}/${REPO} --add-readme --private
# Defer repo cleanup of upstream
defer gh repo delete --yes ${ORG}/${REPO}
# Clone the repo
exec gh repo clone ${ORG}/${REPO}
cd ${REPO}
# Configure push.default so that it should use the merge ref
exec git config push.default upstream
# But prepare a branch that doesn't have a tracking merge ref
exec git checkout -b feature-branch
exec git commit --allow-empty -m 'Empty Commit'
exec git push origin feature-branch
# Create the PR
exec gh pr create --title 'Feature Title' --body 'Feature Body'
stdout https://${GH_HOST}/${ORG}/${REPO}/pull/1

View file

@ -7,7 +7,7 @@ defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING
# Ensure that no default is set
cd $SCRIPT_NAME-$RANDOM_STRING
exec gh repo set-default --view
stderr 'no default repository has been set; use `gh repo set-default` to select one'
stderr 'No default remote repository has been set. To learn more about the default repository, run: gh repo set-default --help'
# Set the default
exec gh repo set-default $ORG/$SCRIPT_NAME-$RANDOM_STRING

View file

@ -28,6 +28,24 @@ func (issue *Issue) ExportData(fields []string) map[string]interface{} {
})
}
data[f] = items
case "closedByPullRequestsReferences":
items := make([]map[string]interface{}, 0, len(issue.ClosedByPullRequestsReferences.Nodes))
for _, n := range issue.ClosedByPullRequestsReferences.Nodes {
items = append(items, map[string]interface{}{
"id": n.ID,
"number": n.Number,
"url": n.URL,
"repository": map[string]interface{}{
"id": n.Repository.ID,
"name": n.Repository.Name,
"owner": map[string]interface{}{
"id": n.Repository.Owner.ID,
"login": n.Repository.Owner.Login,
},
},
})
}
data[f] = items
default:
sf := fieldByName(v, f)
data[f] = sf.Interface()
@ -139,6 +157,24 @@ func (pr *PullRequest) ExportData(fields []string) map[string]interface{} {
}
}
data[f] = &requests
case "closingIssuesReferences":
items := make([]map[string]interface{}, 0, len(pr.ClosingIssuesReferences.Nodes))
for _, n := range pr.ClosingIssuesReferences.Nodes {
items = append(items, map[string]interface{}{
"id": n.ID,
"number": n.Number,
"url": n.URL,
"repository": map[string]interface{}{
"id": n.Repository.ID,
"name": n.Repository.Name,
"owner": map[string]interface{}{
"id": n.Repository.Owner.ID,
"login": n.Repository.Owner.Login,
},
},
})
}
data[f] = items
default:
sf := fieldByName(v, f)
data[f] = sf.Interface()

View file

@ -107,6 +107,70 @@ func TestIssue_ExportData(t *testing.T) {
}
`),
},
{
name: "linked pull requests",
fields: []string{"closedByPullRequestsReferences"},
inputJSON: heredoc.Doc(`
{ "closedByPullRequestsReferences": { "nodes": [
{
"id": "I_123",
"number": 123,
"url": "https://github.com/cli/cli/pull/123",
"repository": {
"id": "R_123",
"name": "cli",
"owner": {
"id": "O_123",
"login": "cli"
}
}
},
{
"id": "I_456",
"number": 456,
"url": "https://github.com/cli/cli/pull/456",
"repository": {
"id": "R_456",
"name": "cli",
"owner": {
"id": "O_456",
"login": "cli"
}
}
}
] } }
`),
outputJSON: heredoc.Doc(`
{ "closedByPullRequestsReferences": [
{
"id": "I_123",
"number": 123,
"repository": {
"id": "R_123",
"name": "cli",
"owner": {
"id": "O_123",
"login": "cli"
}
},
"url": "https://github.com/cli/cli/pull/123"
},
{
"id": "I_456",
"number": 456,
"repository": {
"id": "R_456",
"name": "cli",
"owner": {
"id": "O_456",
"login": "cli"
}
},
"url": "https://github.com/cli/cli/pull/456"
}
] }
`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -120,7 +184,14 @@ func TestIssue_ExportData(t *testing.T) {
enc := json.NewEncoder(&buf)
enc.SetIndent("", "\t")
require.NoError(t, enc.Encode(exported))
assert.Equal(t, tt.outputJSON, buf.String())
var gotData interface{}
dec = json.NewDecoder(&buf)
require.NoError(t, dec.Decode(&gotData))
var expectData interface{}
require.NoError(t, json.Unmarshal([]byte(tt.outputJSON), &expectData))
assert.Equal(t, expectData, gotData)
})
}
}
@ -245,6 +316,70 @@ func TestPullRequest_ExportData(t *testing.T) {
}
`),
},
{
name: "linked issues",
fields: []string{"closingIssuesReferences"},
inputJSON: heredoc.Doc(`
{ "closingIssuesReferences": { "nodes": [
{
"id": "I_123",
"number": 123,
"url": "https://github.com/cli/cli/issues/123",
"repository": {
"id": "R_123",
"name": "cli",
"owner": {
"id": "O_123",
"login": "cli"
}
}
},
{
"id": "I_456",
"number": 456,
"url": "https://github.com/cli/cli/issues/456",
"repository": {
"id": "R_456",
"name": "cli",
"owner": {
"id": "O_456",
"login": "cli"
}
}
}
] } }
`),
outputJSON: heredoc.Doc(`
{ "closingIssuesReferences": [
{
"id": "I_123",
"number": 123,
"repository": {
"id": "R_123",
"name": "cli",
"owner": {
"id": "O_123",
"login": "cli"
}
},
"url": "https://github.com/cli/cli/issues/123"
},
{
"id": "I_456",
"number": 456,
"repository": {
"id": "R_456",
"name": "cli",
"owner": {
"id": "O_456",
"login": "cli"
}
},
"url": "https://github.com/cli/cli/issues/456"
}
] }
`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

View file

@ -44,6 +44,10 @@ type CommentCreateInput struct {
SubjectId string
}
type CommentDeleteInput struct {
CommentId string
}
type CommentUpdateInput struct {
Body string
CommentId string
@ -99,6 +103,27 @@ func CommentUpdate(client *Client, repoHost string, params CommentUpdateInput) (
return mutation.UpdateIssueComment.IssueComment.URL, nil
}
func CommentDelete(client *Client, repoHost string, params CommentDeleteInput) error {
var mutation struct {
DeleteIssueComment struct {
ClientMutationID string
} `graphql:"deleteIssueComment(input: $input)"`
}
variables := map[string]interface{}{
"input": githubv4.DeleteIssueCommentInput{
ID: githubv4.ID(params.CommentId),
},
}
err := client.Mutate(repoHost, "CommentDelete", &mutation, variables)
if err != nil {
return err
}
return nil
}
func (c Comment) Identifier() string {
return c.ID
}

View file

@ -44,6 +44,28 @@ type Issue struct {
Milestone *Milestone
ReactionGroups ReactionGroups
IsPinned bool
ClosedByPullRequestsReferences ClosedByPullRequestsReferences
}
type ClosedByPullRequestsReferences struct {
Nodes []struct {
ID string
Number int
URL string
Repository struct {
ID string
Name string
Owner struct {
ID string
Login string
}
}
}
PageInfo struct {
HasNextPage bool
EndCursor string
}
}
// return values for Issue.Typename

View file

@ -93,6 +93,8 @@ type PullRequest struct {
Reviews PullRequestReviews
LatestReviews PullRequestReviews
ReviewRequests ReviewRequests
ClosingIssuesReferences ClosingIssuesReferences
}
type StatusCheckRollupNode struct {
@ -107,6 +109,26 @@ type CommitStatusCheckRollup struct {
Contexts CheckContexts
}
type ClosingIssuesReferences struct {
Nodes []struct {
ID string
Number int
URL string
Repository struct {
ID string
Name string
Owner struct {
ID string
Login string
}
}
}
PageInfo struct {
HasNextPage bool
EndCursor string
}
}
// https://docs.github.com/en/graphql/reference/enums#checkrunstate
type CheckRunState string

View file

@ -738,34 +738,37 @@ func (m *RepoMetadataResult) LabelsToIDs(names []string) ([]string, error) {
return ids, nil
}
// ProjectsToIDs returns two arrays:
// ProjectsTitlesToIDs returns two arrays:
// - the first contains IDs of projects V1
// - the second contains IDs of projects V2
// - if neither project V1 or project V2 can be found with a given name, then an error is returned
func (m *RepoMetadataResult) ProjectsToIDs(names []string) ([]string, []string, error) {
func (m *RepoMetadataResult) ProjectsTitlesToIDs(titles []string) ([]string, []string, error) {
var ids []string
var idsV2 []string
for _, projectName := range names {
id, found := m.projectNameToID(projectName)
for _, title := range titles {
id, found := m.v1ProjectNameToID(title)
if found {
ids = append(ids, id)
continue
}
idV2, found := m.projectV2TitleToID(projectName)
idV2, found := m.v2ProjectTitleToID(title)
if found {
idsV2 = append(idsV2, idV2)
continue
}
return nil, nil, fmt.Errorf("'%s' not found", projectName)
return nil, nil, fmt.Errorf("'%s' not found", title)
}
return ids, idsV2, nil
}
func (m *RepoMetadataResult) projectNameToID(projectName string) (string, bool) {
// We use the word "titles" when referring to v1 and v2 projects.
// In reality, v1 projects really have "names", so there is a bit of a
// mismatch we just need to gloss over.
func (m *RepoMetadataResult) v1ProjectNameToID(name string) (string, bool) {
for _, p := range m.Projects {
if strings.EqualFold(projectName, p.Name) {
if strings.EqualFold(name, p.Name) {
return p.ID, true
}
}
@ -773,9 +776,9 @@ func (m *RepoMetadataResult) projectNameToID(projectName string) (string, bool)
return "", false
}
func (m *RepoMetadataResult) projectV2TitleToID(projectTitle string) (string, bool) {
func (m *RepoMetadataResult) v2ProjectTitleToID(title string) (string, bool) {
for _, p := range m.ProjectsV2 {
if strings.EqualFold(projectTitle, p.Title) {
if strings.EqualFold(title, p.Title) {
return p.ID, true
}
}
@ -783,8 +786,8 @@ func (m *RepoMetadataResult) projectV2TitleToID(projectTitle string) (string, bo
return "", false
}
func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []string, projectsV1Support gh.ProjectsV1Support) ([]string, error) {
paths := make([]string, 0, len(projectNames))
func ProjectTitlesToPaths(client *Client, repo ghrepo.Interface, titles []string, projectsV1Support gh.ProjectsV1Support) ([]string, error) {
paths := make([]string, 0, len(titles))
matchedPaths := map[string]struct{}{}
// TODO: ProjectsV1Cleanup
@ -796,9 +799,9 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
return nil, err
}
for _, projectName := range projectNames {
for _, title := range titles {
for _, p := range v1Projects {
if strings.EqualFold(projectName, p.Name) {
if strings.EqualFold(title, p.Name) {
pathParts := strings.Split(p.ResourcePath, "/")
var path string
if pathParts[1] == "orgs" || pathParts[1] == "users" {
@ -807,7 +810,7 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
path = fmt.Sprintf("%s/%s/%s", pathParts[1], pathParts[2], pathParts[4])
}
paths = append(paths, path)
matchedPaths[projectName] = struct{}{}
matchedPaths[title] = struct{}{}
break
}
}
@ -820,15 +823,15 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
return nil, err
}
for _, projectName := range projectNames {
for _, title := range titles {
// If we already found a v1 project with this name, skip it
if _, ok := matchedPaths[projectName]; ok {
if _, ok := matchedPaths[title]; ok {
continue
}
found := false
for _, p := range v2Projects {
if strings.EqualFold(projectName, p.Title) {
if strings.EqualFold(title, p.Title) {
pathParts := strings.Split(p.ResourcePath, "/")
var path string
if pathParts[1] == "orgs" || pathParts[1] == "users" {
@ -843,7 +846,7 @@ func ProjectNamesToPaths(client *Client, repo ghrepo.Interface, projectNames []s
}
if !found {
return nil, fmt.Errorf("'%s' not found", projectName)
return nil, fmt.Errorf("'%s' not found", title)
}
}

View file

@ -187,7 +187,7 @@ func Test_RepoMetadata(t *testing.T) {
expectedProjectIDs := []string{"TRIAGEID", "ROADMAPID"}
expectedProjectV2IDs := []string{"TRIAGEV2ID", "ROADMAPV2ID", "MONALISAV2ID"}
projectIDs, projectV2IDs, err := result.ProjectsToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2", "monalisav2"})
projectIDs, projectV2IDs, err := result.ProjectsTitlesToIDs([]string{"triage", "roadmap", "triagev2", "roadmapv2", "monalisav2"})
if err != nil {
t.Errorf("error resolving projects: %v", err)
}
@ -273,7 +273,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
} } } }
`))
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Supported)
projectPaths, err := ProjectTitlesToPaths(client, repo, []string{"Triage", "Roadmap", "TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Supported)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@ -331,7 +331,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
} } } }
`))
projectPaths, err := ProjectNamesToPaths(client, repo, []string{"TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Unsupported)
projectPaths, err := ProjectTitlesToPaths(client, repo, []string{"TriageV2", "RoadmapV2", "MonalisaV2"}, gh.ProjectsV1Unsupported)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
@ -374,7 +374,7 @@ func Test_ProjectNamesToPaths(t *testing.T) {
} } } }
`))
_, err := ProjectNamesToPaths(client, repo, []string{"TriageV2"}, gh.ProjectsV1Unsupported)
_, err := ProjectTitlesToPaths(client, repo, []string{"TriageV2"}, gh.ProjectsV1Unsupported)
require.Equal(t, err, fmt.Errorf("'TriageV2' not found"))
})
}

View file

@ -56,6 +56,25 @@ var issueCommentLast = shortenQuery(`
}
`)
var issueClosedByPullRequestsReferences = shortenQuery(`
closedByPullRequestsReferences(first: 100) {
nodes {
id,
number,
url,
repository {
id,
name,
owner {
id,
login
}
}
}
pageInfo{hasNextPage,endCursor}
}
`)
var prReviewRequests = shortenQuery(`
reviewRequests(first: 100) {
nodes {
@ -132,6 +151,25 @@ var prCommits = shortenQuery(`
}
`)
var prClosingIssuesReferences = shortenQuery(`
closingIssuesReferences(first: 100) {
nodes {
id,
number,
url,
repository {
id,
name,
owner {
id,
login
}
}
}
pageInfo{hasNextPage,endCursor}
}
`)
var autoMergeRequest = shortenQuery(`
autoMergeRequest {
authorEmail,
@ -277,6 +315,7 @@ var sharedIssuePRFields = []string{
var issueOnlyFields = []string{
"isPinned",
"stateReason",
"closedByPullRequestsReferences",
}
var IssueFields = append(sharedIssuePRFields, issueOnlyFields...)
@ -287,6 +326,7 @@ var PullRequestFields = append(sharedIssuePRFields,
"baseRefName",
"baseRefOid",
"changedFiles",
"closingIssuesReferences",
"commits",
"deletions",
"files",
@ -366,6 +406,10 @@ func IssueGraphQL(fields []string) string {
q = append(q, StatusCheckRollupGraphQLWithoutCountByState(""))
case "statusCheckRollupWithCountByState": // pseudo-field
q = append(q, StatusCheckRollupGraphQLWithCountByState())
case "closingIssuesReferences":
q = append(q, prClosingIssuesReferences)
case "closedByPullRequestsReferences":
q = append(q, issueClosedByPullRequestsReferences)
default:
q = append(q, field)
}

4
go.mod
View file

@ -11,13 +11,13 @@ require (
github.com/briandowns/spinner v1.18.1
github.com/cenkalti/backoff/v4 v4.3.0
github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3
github.com/charmbracelet/huh v0.6.1-0.20250409210615-c5906631cbb5
github.com/charmbracelet/huh v0.7.0
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc
github.com/cli/go-gh/v2 v2.12.0
github.com/cli/go-internal v0.0.0-20241025142207-6c48bcd5ce24
github.com/cli/oauth v1.1.1
github.com/cli/safeexec v1.0.1
github.com/cpuguy83/go-md2man/v2 v2.0.6
github.com/cpuguy83/go-md2man/v2 v2.0.7
github.com/creack/pty v1.1.24
github.com/digitorus/timestamp v0.0.0-20231217203849-220c5c2851b7
github.com/distribution/reference v0.6.0

15
go.sum
View file

@ -110,20 +110,28 @@ github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc h1:4p
github.com/charmbracelet/colorprofile v0.2.3-0.20250311203215-f60798e515dc/go.mod h1:X4/0JoqgTIPSFcRA/P6INZzIuyqdFY5rm8tb41s9okk=
github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3 h1:hx6E25SvI2WiZdt/gxINcYBnHD7PE2Vr9auqwg5B05g=
github.com/charmbracelet/glamour v0.9.2-0.20250319212134-549f544650e3/go.mod h1:ihVqv4/YOY5Fweu1cxajuQrwJFh3zU4Ukb4mHVNjq3s=
github.com/charmbracelet/huh v0.6.1-0.20250409210615-c5906631cbb5 h1:uOnMxWghHfEYm2DPMeIHHAEirV/TduBVC9ZRXGcX9Q8=
github.com/charmbracelet/huh v0.6.1-0.20250409210615-c5906631cbb5/go.mod h1:xl27E/xNaX3WwdkqpvBwjJcGWhupkU52CWLC5hReBTw=
github.com/charmbracelet/huh v0.7.0 h1:W8S1uyGETgj9Tuda3/JdVkc3x7DBLZYPZc4c+/rnRdc=
github.com/charmbracelet/huh v0.7.0/go.mod h1:UGC3DZHlgOKHvHC07a5vHag41zzhpPFj34U92sOmyuk=
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc h1:nFRtCfZu/zkltd2lsLUPlVNv3ej/Atod9hcdbRZtlys=
github.com/charmbracelet/lipgloss v1.1.1-0.20250319133953-166f707985bc/go.mod h1:aKC/t2arECF6rNOnaKaVU6y4t4ZeHQzqfxedE/VkVhA=
github.com/charmbracelet/x/ansi v0.8.0 h1:9GTq3xq9caJW8ZrBTe0LIe2fvfLR/bYXKTx2llXn7xE=
github.com/charmbracelet/x/ansi v0.8.0/go.mod h1:wdYl/ONOLHLIVmQaxbIYEC/cRKOQyjTkowiI4blgS9Q=
github.com/charmbracelet/x/cellbuf v0.0.13 h1:/KBBKHuVRbq1lYx5BzEHBAFBP8VcQzJejZ/IA3iR28k=
github.com/charmbracelet/x/cellbuf v0.0.13/go.mod h1:xe0nKWGd3eJgtqZRaN9RjMtK7xUYchjzPr7q6kcvCCs=
github.com/charmbracelet/x/conpty v0.1.0 h1:4zc8KaIcbiL4mghEON8D72agYtSeIgq8FSThSPQIb+U=
github.com/charmbracelet/x/conpty v0.1.0/go.mod h1:rMFsDJoDwVmiYM10aD4bH2XiRgwI7NYJtQgl5yskjEQ=
github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86 h1:JSt3B+U9iqk37QUU2Rvb6DSBYRLtWqFqfxf8l5hOZUA=
github.com/charmbracelet/x/errors v0.0.0-20240508181413-e8d8b6e2de86/go.mod h1:2P0UgXMEa6TsToMSuFqKFQR+fZTO9CNGUNokkPatT/0=
github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91 h1:payRxjMjKgx2PaCWLZ4p3ro9y97+TVLZNaRZgJwSVDQ=
github.com/charmbracelet/x/exp/golden v0.0.0-20241011142426-46044092ad91/go.mod h1:wDlXFlCrmJ8J+swcL/MnGUuYnqgQdW9rhSD61oNMb6U=
github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0 h1:qko3AQ4gK1MTS/de7F5hPGx6/k1u0w4TeYmBFwzYVP4=
github.com/charmbracelet/x/exp/strings v0.0.0-20240722160745-212f7b056ed0/go.mod h1:pBhA0ybfXv6hDjQUZ7hk1lVxBiUbupdw5R31yPUViVQ=
github.com/charmbracelet/x/term v0.2.1 h1:AQeHeLZ1OqSXhrAWpYUtZyX1T3zVxfpZuEQMIQaGIAQ=
github.com/charmbracelet/x/term v0.2.1/go.mod h1:oQ4enTYFV7QN4m0i9mzHrViD7TQKvNEEkHUMCmsxdUg=
github.com/charmbracelet/x/termios v0.1.1 h1:o3Q2bT8eqzGnGPOYheoYS8eEleT5ZVNYNy8JawjaNZY=
github.com/charmbracelet/x/termios v0.1.1/go.mod h1:rB7fnv1TgOPOyyKRJ9o+AsTU/vK5WHJ2ivHeut/Pcwo=
github.com/charmbracelet/x/xpty v0.1.2 h1:Pqmu4TEJ8KeA9uSkISKMU3f+C1F6OGBn8ABuGlqCbtI=
github.com/charmbracelet/x/xpty v0.1.2/go.mod h1:XK2Z0id5rtLWcpeNiMYBccNNBrP2IJnzHI0Lq13Xzq4=
github.com/cli/browser v1.0.0/go.mod h1:IEWkHYbLjkhtjwwWlwTHW2lGxeS5gezEQBMLTwDHf5Q=
github.com/cli/browser v1.3.0 h1:LejqCrpWr+1pRqmEPDGnTZOjsMe7sehifLynZJuqJpo=
github.com/cli/browser v1.3.0/go.mod h1:HH8s+fOAxjhQoBUAsKuPCbqUuxZDhQ2/aD+SzsEfBTk=
@ -142,8 +150,9 @@ github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUo
github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4=
github.com/containerd/stargz-snapshotter/estargz v0.16.3 h1:7evrXtoh1mSbGj/pfRccTampEyKpjpOnS3CyiV1Ebr8=
github.com/containerd/stargz-snapshotter/estargz v0.16.3/go.mod h1:uyr4BfYfOj3G9WBVE8cOlQmXAbPN9VEQpBBeJIuOipU=
github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/cpuguy83/go-md2man/v2 v2.0.7 h1:zbFlGlXEAKlwXpmvle3d8Oe3YnkKIK4xSRTd3sHPnBo=
github.com/cpuguy83/go-md2man/v2 v2.0.7/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/creack/pty v1.1.17/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=

View file

@ -32,13 +32,16 @@ import (
// are sufficient to ensure that the accessible prompter behaves roughly as expected
// but doesn't mandate that prompts always look exactly the same.
func TestAccessiblePrompter(t *testing.T) {
beforePasswordSendTimeout := 20 * time.Microsecond
t.Run("Select", func(t *testing.T) {
console := newTestVirtualTerminal(t)
p := newTestAccessiblePrompter(t, console)
go func() {
// Wait for prompt to appear
_, err := console.ExpectString("Choose:")
_, err := console.ExpectString("Input a number between 1 and 3:")
require.NoError(t, err)
// Select option 1
@ -57,7 +60,7 @@ func TestAccessiblePrompter(t *testing.T) {
go func() {
// Wait for prompt to appear
_, err := console.ExpectString("Select a number")
_, err := console.ExpectString("Input a number between 0 and 3:")
require.NoError(t, err)
// Select options 1 and 2
@ -76,6 +79,27 @@ func TestAccessiblePrompter(t *testing.T) {
assert.Equal(t, []int{0, 1}, multiSelectValue)
})
t.Run("MultiSelect - default values are respected by being pre-selected", func(t *testing.T) {
console := newTestVirtualTerminal(t)
p := newTestAccessiblePrompter(t, console)
go func() {
// Wait for prompt to appear
_, err := console.ExpectString("Select a number")
require.NoError(t, err)
// Don't select anything because the default should be selected.
// This confirms selections
_, err = console.SendLine("0")
require.NoError(t, err)
}()
multiSelectValue, err := p.MultiSelect("Select a number", []string{"2"}, []string{"1", "2", "3"})
require.NoError(t, err)
assert.Equal(t, []int{1}, multiSelectValue)
})
t.Run("Input", func(t *testing.T) {
console := newTestVirtualTerminal(t)
p := newTestAccessiblePrompter(t, console)
@ -126,6 +150,9 @@ func TestAccessiblePrompter(t *testing.T) {
_, err := console.ExpectString("Enter password")
require.NoError(t, err)
// Wait to ensure huh has time to set the echo mode
time.Sleep(beforePasswordSendTimeout)
// Enter a number
_, err = console.SendLine(dummyPassword)
require.NoError(t, err)
@ -134,6 +161,11 @@ func TestAccessiblePrompter(t *testing.T) {
passwordValue, err := p.Password("Enter password")
require.NoError(t, err)
require.Equal(t, dummyPassword, passwordValue)
// Ensure the dummy password is not printed to the screen,
// asserting that echo mode is disabled.
_, err = console.ExpectString(" \r\n\r\n")
require.NoError(t, err)
})
t.Run("Confirm", func(t *testing.T) {
@ -184,6 +216,9 @@ func TestAccessiblePrompter(t *testing.T) {
_, err := console.ExpectString("Paste your authentication token:")
require.NoError(t, err)
// Wait to ensure huh has time to set the echo mode
time.Sleep(beforePasswordSendTimeout)
// Enter some dummy auth token
_, err = console.SendLine(dummyAuthToken)
require.NoError(t, err)
@ -192,6 +227,11 @@ func TestAccessiblePrompter(t *testing.T) {
authValue, err := p.AuthToken()
require.NoError(t, err)
require.Equal(t, dummyAuthToken, authValue)
// Ensure the dummy password is not printed to the screen,
// asserting that echo mode is disabled.
_, err = console.ExpectString(" \r\n\r\n")
require.NoError(t, err)
})
t.Run("AuthToken - blank input returns error", func(t *testing.T) {
@ -212,6 +252,9 @@ func TestAccessiblePrompter(t *testing.T) {
_, err = console.ExpectString("token is required")
require.NoError(t, err)
// Wait to ensure huh has time to set the echo mode
time.Sleep(beforePasswordSendTimeout)
// Now enter some dummy auth token to return control back to the test
_, err = console.SendLine(dummyAuthTokenForAfterFailure)
require.NoError(t, err)
@ -220,6 +263,11 @@ func TestAccessiblePrompter(t *testing.T) {
authValue, err := p.AuthToken()
require.NoError(t, err)
require.Equal(t, dummyAuthTokenForAfterFailure, authValue)
// Ensure the dummy password is not printed to the screen,
// asserting that echo mode is disabled.
_, err = console.ExpectString(" \r\n\r\n")
require.NoError(t, err)
})
t.Run("ConfirmDeletion", func(t *testing.T) {
@ -325,7 +373,7 @@ func TestAccessiblePrompter(t *testing.T) {
require.NoError(t, err)
// Expect a notice to enter something valid since blank is disallowed.
_, err = console.ExpectString("invalid input. please try again")
_, err = console.ExpectString("Invalid: must be between 1 and 1")
require.NoError(t, err)
// Send a 1 to select to open the editor. This will immediately exit
@ -352,7 +400,7 @@ func TestAccessiblePrompter(t *testing.T) {
require.NoError(t, err)
// Expect a notice to enter something valid since blank is disallowed.
_, err = console.ExpectString("invalid input. please try again")
_, err = console.ExpectString("Invalid: must be between 1 and 1")
require.NoError(t, err)
// Send a 1 to select to open the editor since skip is invalid and

View file

@ -2,6 +2,7 @@ package prompter
import (
"fmt"
"slices"
"strings"
"github.com/AlecAivazis/survey/v2"
@ -100,6 +101,14 @@ func (p *accessiblePrompter) MultiSelect(prompt string, defaults []string, optio
var result []int
formOptions := make([]huh.Option[int], len(options))
for i, o := range options {
// If this option is in the defaults slice,
// let's add its index to the result slice and huh
// will treat it as a default selection.
// TODO: does an invalid default value constitute a panic?
if slices.Contains(defaults, o) {
result = append(result, i)
}
formOptions[i] = huh.NewOption(o, i)
}
@ -137,10 +146,12 @@ func (p *accessiblePrompter) Input(prompt, defaultValue string) (string, error)
func (p *accessiblePrompter) Password(prompt string) (string, error) {
var result string
// EchoMode(huh.EchoModePassword) doesn't have any effect in accessible mode.
// EchoModePassword is not used as password masking is unsupported in huh.
// EchoModeNone and EchoModePassword have the same effect of hiding user input.
form := p.newForm(
huh.NewGroup(
huh.NewInput().
EchoMode(huh.EchoModeNone).
Title(prompt).
Value(&result),
),
@ -171,9 +182,12 @@ func (p *accessiblePrompter) Confirm(prompt string, defaultValue bool) (bool, er
func (p *accessiblePrompter) AuthToken() (string, error) {
var result string
// EchoModeNone and EchoModePassword both result in disabling echo mode
// as password masking is outside of VT100 spec.
form := p.newForm(
huh.NewGroup(
huh.NewInput().
EchoMode(huh.EchoModeNone).
Title("Paste your authentication token:").
// Note: if this validation fails, the prompt loops.
Validate(func(input string) error {
@ -183,8 +197,6 @@ func (p *accessiblePrompter) AuthToken() (string, error) {
return nil
}).
Value(&result),
// This doesn't have any effect in accessible mode.
// EchoMode(huh.EchoModePassword),
),
)

View file

@ -141,6 +141,18 @@ func IndexFor(options []string, answer string) (int, error) {
return -1, NoSuchAnswerErr(answer, options)
}
func IndexesFor(options []string, answers ...string) ([]int, error) {
indexes := make([]int, len(answers))
for i, answer := range answers {
index, err := IndexFor(options, answer)
if err != nil {
return nil, err
}
indexes[i] = index
}
return indexes, nil
}
func NoSuchPromptErr(prompt string) error {
return fmt.Errorf("no such prompt '%s'", prompt)
}

View file

@ -0,0 +1,142 @@
package accessibility
import (
"fmt"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
const (
webURL = "https://accessibility.github.com/conformance/cli/"
)
type AccessibilityOptions struct {
IO *iostreams.IOStreams
Browser browser.Browser
Web bool
}
func NewCmdAccessibility(f *cmdutil.Factory) *cobra.Command {
opts := AccessibilityOptions{
IO: f.IOStreams,
Browser: f.Browser,
}
cmd := &cobra.Command{
Use: "accessibility",
Aliases: []string{"a11y"},
Short: "Learn about GitHub CLI's accessibility experiences",
Long: longDescription(opts.IO),
Hidden: true,
RunE: func(cmd *cobra.Command, args []string) error {
if opts.Web {
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", text.DisplayURL(webURL))
}
return opts.Browser.Browse(webURL)
}
return cmd.Help()
},
Example: heredoc.Doc(`
# Open the GitHub Accessibility site in your browser
$ gh accessibility --web
# Display color using customizable, 4-bit accessible colors
$ gh config set accessible_colors enabled
# Use input prompts without redrawing the screen
$ gh config set accessible_prompter enabled
# Disable motion-based spinners for progress indicators in favor of text
$ gh config set spinner disabled
`),
}
cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "Open the GitHub Accessibility site in your browser")
cmdutil.DisableAuthCheck(cmd)
return cmd
}
func longDescription(io *iostreams.IOStreams) string {
cs := io.ColorScheme()
title := cs.Bold("Learn about GitHub CLI's accessibility experiences")
color := cs.Bold("Customizable and contrasting colors")
prompter := cs.Bold("Non-interactive user input prompting")
spinner := cs.Bold("Text-based spinners")
feedback := cs.Bold("Join the conversation")
return heredoc.Docf(`
%[2]s
As the home for all developers, we want every developer to feel welcome in our
community and be empowered to contribute to the future of global software
development with everything GitHub has to offer including the GitHub CLI.
%[3]s
Text interfaces often use color for various purposes, but insufficient contrast
or customizability can leave some users unable to benefit.
For a more accessible experience, the GitHub CLI can use color palettes
based on terminal background appearance and limit colors to 4-bit ANSI color
palettes, which users can customize within terminal preferences.
With this new experience, the GitHub CLI provides multiple options to address
color usage:
1. The GitHub CLI will use 4-bit color palette for increased color contrast based
on dark and light backgrounds including rendering Markdown based on the
GitHub Primer design system.
To enable this experience, use one of the following methods:
- Run %[1]sgh config set accessible_colors enabled%[1]s
- Set %[1]sGH_ACCESSIBLE_COLORS=enabled%[1]s environment variable
2. The GitHub CLI will display issue and pull request labels' custom RGB colors
in terminals with true color support.
To enable this experience, use one of the following methods:
- Run %[1]sgh config set color_labels enabled%[1]s
- Set %[1]sGH_COLOR_LABELS=enabled%[1]s environment variable
%[4]s
Interactive text user interfaces manipulate the terminal cursor to redraw parts
of the screen, which can be difficult for speech synthesizers or braille displays
to accurately detect and read.
For a more accessible experience, the GitHub CLI can provide a similar experience using
non-interactive prompts for user input.
To enable this experience, use one of the following methods:
- Run %[1]sgh config set accessible_prompter enabled%[1]s
- Set %[1]sGH_ACCESSIBLE_PROMPTER=enabled%[1]s environment variable
%[5]s
Motion-based spinners communicate in-progress activity by manipulating the
terminal cursor to create a spinning effect, which may cause discomfort to users
with motion sensitivity or miscommunicate information to speech synthesizers.
For a more accessible experience, this interactivity can be disabled in favor
of text-based progress indicators.
To enable this experience, use one of the following methods:
- Run %[1]sgh config set spinner disabled%[1]s
- Set %[1]sGH_SPINNER_DISABLED=yes%[1]s environment variable
%[6]s
We invite you to join us in improving GitHub CLI accessibility by sharing your
feedback and ideas through GitHub Accessibility feedback channels:
%[7]s
`, "`", title, color, prompter, spinner, feedback, webURL)
}

View file

@ -1,7 +1,10 @@
package api
import (
"encoding/json"
"errors"
"fmt"
"github.com/sigstore/sigstore-go/pkg/bundle"
)
@ -20,3 +23,35 @@ type Attestation struct {
type AttestationsResponse struct {
Attestations []*Attestation `json:"attestations"`
}
type IntotoStatement struct {
PredicateType string `json:"predicateType"`
}
func FilterAttestations(predicateType string, attestations []*Attestation) ([]*Attestation, error) {
filteredAttestations := []*Attestation{}
for _, each := range attestations {
dsseEnvelope := each.Bundle.GetDsseEnvelope()
if dsseEnvelope != nil {
if dsseEnvelope.PayloadType != "application/vnd.in-toto+json" {
// Don't fail just because an entry isn't intoto
continue
}
var intotoStatement IntotoStatement
if err := json.Unmarshal([]byte(dsseEnvelope.Payload), &intotoStatement); err != nil {
// Don't fail just because a single entry can't be unmarshalled
continue
}
if intotoStatement.PredicateType == predicateType {
filteredAttestations = append(filteredAttestations, each)
}
}
}
if len(filteredAttestations) == 0 {
return nil, fmt.Errorf("no attestations found with predicate type: %s", predicateType)
}
return filteredAttestations, nil
}

View file

@ -27,6 +27,28 @@ const (
// Allow injecting backoff interval in tests.
var getAttestationRetryInterval = time.Millisecond * 200
// FetchParams are the parameters for fetching attestations from the GitHub API
type FetchParams struct {
Digest string
Limit int
Owner string
PredicateType string
Repo string
}
func (p *FetchParams) Validate() error {
if p.Digest == "" {
return fmt.Errorf("digest must be provided")
}
if p.Limit <= 0 || p.Limit > maxLimitForFlag {
return fmt.Errorf("limit must be greater than 0 and less than or equal to %d", maxLimitForFlag)
}
if p.Repo == "" && p.Owner == "" {
return fmt.Errorf("owner or repo must be provided")
}
return nil
}
// githubApiClient makes REST calls to the GitHub API
type githubApiClient interface {
REST(hostname, method, p string, body io.Reader, data interface{}) error
@ -39,8 +61,7 @@ type httpClient interface {
}
type Client interface {
GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error)
GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error)
GetByDigest(params FetchParams) ([]*Attestation, error)
GetTrustDomain() (string, error)
}
@ -60,22 +81,11 @@ func NewLiveClient(hc *http.Client, host string, l *ioconfig.Handler) *LiveClien
}
}
// GetByRepoAndDigest fetches the attestation by repo and digest
func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
c.logger.VerbosePrintf("Fetching attestations for artifact digest %s\n\n", digest)
url := fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, repo, digest)
return c.getByURL(url, limit)
}
// GetByOwnerAndDigest fetches attestation by owner and digest
func (c *LiveClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error) {
c.logger.VerbosePrintf("Fetching attestations for artifact digest %s\n\n", digest)
url := fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, owner, digest)
return c.getByURL(url, limit)
}
func (c *LiveClient) getByURL(url string, limit int) ([]*Attestation, error) {
attestations, err := c.getAttestations(url, limit)
// GetByDigest fetches the attestation by digest and either owner or repo
// depending on which is provided
func (c *LiveClient) GetByDigest(params FetchParams) ([]*Attestation, error) {
c.logger.VerbosePrintf("Fetching attestations for artifact digest %s\n\n", params.Digest)
attestations, err := c.getAttestations(params)
if err != nil {
return nil, err
}
@ -88,40 +98,52 @@ func (c *LiveClient) getByURL(url string, limit int) ([]*Attestation, error) {
return bundles, nil
}
// GetTrustDomain returns the current trust domain. If the default is used
// the empty string is returned
func (c *LiveClient) GetTrustDomain() (string, error) {
return c.getTrustDomain(MetaPath)
}
func (c *LiveClient) getAttestations(url string, limit int) ([]*Attestation, error) {
perPage := limit
if perPage <= 0 || perPage > maxLimitForFlag {
return nil, fmt.Errorf("limit must be greater than 0 and less than or equal to %d", maxLimitForFlag)
func (c *LiveClient) buildRequestURL(params FetchParams) (string, error) {
if err := params.Validate(); err != nil {
return "", err
}
var url string
if params.Repo != "" {
// check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo.
// If Repo is not set, the field will remain empty. It will not be populated using the value of Owner.
url = fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, params.Repo, params.Digest)
} else {
url = fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, params.Owner, params.Digest)
}
perPage := params.Limit
if perPage > maxLimitForFetch {
perPage = maxLimitForFetch
}
// ref: https://github.com/cli/go-gh/blob/d32c104a9a25c9de3d7c7b07a43ae0091441c858/example_gh_test.go#L96
url = fmt.Sprintf("%s?per_page=%d", url, perPage)
if params.PredicateType != "" {
url = fmt.Sprintf("%s&predicate_type=%s", url, params.PredicateType)
}
return url, nil
}
func (c *LiveClient) getAttestations(params FetchParams) ([]*Attestation, error) {
url, err := c.buildRequestURL(params)
if err != nil {
return nil, err
}
var attestations []*Attestation
var resp AttestationsResponse
bo := backoff.NewConstantBackOff(getAttestationRetryInterval)
// if no attestation or less than limit, then keep fetching
for url != "" && len(attestations) < limit {
for url != "" && len(attestations) < params.Limit {
err := backoff.Retry(func() error {
newURL, restErr := c.githubAPI.RESTWithNext(c.host, http.MethodGet, url, nil, &resp)
if restErr != nil {
if shouldRetry(restErr) {
return restErr
} else {
return backoff.Permanent(restErr)
}
return backoff.Permanent(restErr)
}
url = newURL
@ -140,8 +162,8 @@ func (c *LiveClient) getAttestations(url string, limit int) ([]*Attestation, err
return nil, ErrNoAttestationsFound
}
if len(attestations) > limit {
return attestations[:limit], nil
if len(attestations) > params.Limit {
return attestations[:params.Limit], nil
}
return attestations, nil
@ -241,6 +263,12 @@ func shouldRetry(err error) bool {
return false
}
// GetTrustDomain returns the current trust domain. If the default is used
// the empty string is returned
func (c *LiveClient) GetTrustDomain() (string, error) {
return c.getTrustDomain(MetaPath)
}
func (c *LiveClient) getTrustDomain(url string) (string, error) {
var resp MetaResponse

View file

@ -42,78 +42,75 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
}
}
var testFetchParamsWithOwner = FetchParams{
Digest: testDigest,
Limit: DefaultLimit,
Owner: testOwner,
PredicateType: "https://slsa.dev/provenance/v1",
}
var testFetchParamsWithRepo = FetchParams{
Digest: testDigest,
Limit: DefaultLimit,
Repo: testRepo,
PredicateType: "https://slsa.dev/provenance/v1",
}
type getByTestCase struct {
name string
params FetchParams
limit int
expectedAttestations int
hasNextPage bool
}
var getByTestCases = []getByTestCase{
{
name: "get by digest with owner",
params: testFetchParamsWithOwner,
expectedAttestations: 5,
},
{
name: "get by digest with repo",
params: testFetchParamsWithRepo,
expectedAttestations: 5,
},
{
name: "get by digest with attestations greater than limit",
params: testFetchParamsWithRepo,
limit: 3,
expectedAttestations: 3,
},
{
name: "get by digest with next page",
params: testFetchParamsWithRepo,
expectedAttestations: 10,
hasNextPage: true,
},
{
name: "greater than limit with next page",
params: testFetchParamsWithRepo,
limit: 7,
expectedAttestations: 7,
hasNextPage: true,
},
}
func TestGetByDigest(t *testing.T) {
c := NewClientWithMockGHClient(false)
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
require.NoError(t, err)
for _, tc := range getByTestCases {
t.Run(tc.name, func(t *testing.T) {
c := NewClientWithMockGHClient(tc.hasNextPage)
require.Equal(t, 5, len(attestations))
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
if tc.limit > 0 {
tc.params.Limit = tc.limit
}
attestations, err := c.GetByDigest(tc.params)
require.NoError(t, err)
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
require.NoError(t, err)
require.Equal(t, 5, len(attestations))
bundle = (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
}
func TestGetByDigestGreaterThanLimit(t *testing.T) {
c := NewClientWithMockGHClient(false)
limit := 3
// The method should return five results when the limit is not set
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, limit)
require.NoError(t, err)
require.Equal(t, 3, len(attestations))
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, limit)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
bundle = (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
}
func TestGetByDigestWithNextPage(t *testing.T) {
c := NewClientWithMockGHClient(true)
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
require.NoError(t, err)
require.Equal(t, len(attestations), 10)
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
require.NoError(t, err)
require.Equal(t, len(attestations), 10)
bundle = (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
}
func TestGetByDigestGreaterThanLimitWithNextPage(t *testing.T) {
c := NewClientWithMockGHClient(true)
limit := 7
// The method should return five results when the limit is not set
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, limit)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, limit)
require.NoError(t, err)
require.Equal(t, len(attestations), limit)
bundle = (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
require.Equal(t, tc.expectedAttestations, len(attestations))
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
})
}
}
func TestGetByDigest_NoAttestationsFound(t *testing.T) {
@ -130,12 +127,7 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
logger: io.NewTestHandler(),
}
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
require.Error(t, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
attestations, err := c.GetByDigest(testFetchParamsWithRepo)
require.Error(t, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
@ -153,11 +145,7 @@ func TestGetByDigest_Error(t *testing.T) {
logger: io.NewTestHandler(),
}
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
require.Error(t, err)
require.Nil(t, attestations)
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
attestations, err := c.GetByDigest(testFetchParamsWithRepo)
require.Error(t, err)
require.Nil(t, attestations)
}
@ -362,7 +350,8 @@ func TestGetAttestationsRetries(t *testing.T) {
logger: io.NewTestHandler(),
}
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
testFetchParamsWithRepo.Limit = 30
attestations, err := c.GetByDigest(testFetchParamsWithRepo)
require.NoError(t, err)
// assert the error path was executed; because this is a paged
@ -373,17 +362,6 @@ func TestGetAttestationsRetries(t *testing.T) {
require.Equal(t, len(attestations), 10)
bundle := (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
// same test as above, but for GetByOwnerAndDigest:
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
require.NoError(t, err)
// because we haven't reset the mock, we have added 2 more failed requests
fetcher.AssertNumberOfCalls(t, "FlakyOnRESTSuccessWithNextPage:error", 4)
require.Equal(t, len(attestations), 10)
bundle = (attestations)[0].Bundle
require.Equal(t, bundle.GetMediaType(), "application/vnd.dev.sigstore.bundle.v0.3+json")
}
// test total retries
@ -401,7 +379,7 @@ func TestGetAttestationsMaxRetries(t *testing.T) {
logger: io.NewTestHandler(),
}
_, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
_, err := c.GetByDigest(testFetchParamsWithRepo)
require.Error(t, err)
fetcher.AssertNumberOfCalls(t, "OnREST500Error", 4)

View file

@ -6,58 +6,49 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
)
func makeTestAttestation() Attestation {
return Attestation{Bundle: data.SigstoreBundle(nil), BundleURL: "https://example.com"}
}
type MockClient struct {
OnGetByRepoAndDigest func(repo, digest string, limit int) ([]*Attestation, error)
OnGetByOwnerAndDigest func(owner, digest string, limit int) ([]*Attestation, error)
OnGetTrustDomain func() (string, error)
OnGetByDigest func(params FetchParams) ([]*Attestation, error)
OnGetTrustDomain func() (string, error)
}
func (m MockClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
return m.OnGetByRepoAndDigest(repo, digest, limit)
}
func (m MockClient) GetByOwnerAndDigest(owner, digest string, limit int) ([]*Attestation, error) {
return m.OnGetByOwnerAndDigest(owner, digest, limit)
func (m MockClient) GetByDigest(params FetchParams) ([]*Attestation, error) {
return m.OnGetByDigest(params)
}
func (m MockClient) GetTrustDomain() (string, error) {
return m.OnGetTrustDomain()
}
func makeTestAttestation() Attestation {
return Attestation{Bundle: data.SigstoreBundle(nil), BundleURL: "https://example.com"}
}
func OnGetByRepoAndDigestSuccess(repo, digest string, limit int) ([]*Attestation, error) {
func OnGetByDigestSuccess(params FetchParams) ([]*Attestation, error) {
att1 := makeTestAttestation()
att2 := makeTestAttestation()
return []*Attestation{&att1, &att2}, nil
attestations := []*Attestation{&att1, &att2}
if params.PredicateType != "" {
return FilterAttestations(params.PredicateType, attestations)
}
return attestations, nil
}
func OnGetByRepoAndDigestFailure(repo, digest string, limit int) ([]*Attestation, error) {
return nil, fmt.Errorf("failed to fetch by repo and digest")
}
func OnGetByOwnerAndDigestSuccess(owner, digest string, limit int) ([]*Attestation, error) {
att1 := makeTestAttestation()
att2 := makeTestAttestation()
return []*Attestation{&att1, &att2}, nil
}
func OnGetByOwnerAndDigestFailure(owner, digest string, limit int) ([]*Attestation, error) {
return nil, fmt.Errorf("failed to fetch by owner and digest")
func OnGetByDigestFailure(params FetchParams) ([]*Attestation, error) {
if params.Repo != "" {
return nil, fmt.Errorf("failed to fetch attestations from %s", params.Repo)
}
return nil, fmt.Errorf("failed to fetch attestations from %s", params.Owner)
}
func NewTestClient() *MockClient {
return &MockClient{
OnGetByRepoAndDigest: OnGetByRepoAndDigestSuccess,
OnGetByOwnerAndDigest: OnGetByOwnerAndDigestSuccess,
OnGetByDigest: OnGetByDigestSuccess,
}
}
func NewFailTestClient() *MockClient {
return &MockClient{
OnGetByRepoAndDigest: OnGetByRepoAndDigestFailure,
OnGetByOwnerAndDigest: OnGetByOwnerAndDigestFailure,
OnGetByDigest: OnGetByDigestFailure,
}
}

View file

@ -9,7 +9,6 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
"github.com/cli/cli/v2/pkg/cmd/attestation/auth"
"github.com/cli/cli/v2/pkg/cmd/attestation/io"
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
"github.com/cli/cli/v2/pkg/cmdutil"
ghauth "github.com/cli/go-gh/v2/pkg/auth"
@ -127,13 +126,16 @@ func runDownload(opts *Options) error {
opts.Logger.VerbosePrintf("Downloading trusted metadata for artifact %s\n\n", opts.ArtifactPath)
params := verification.FetchRemoteAttestationsParams{
if opts.APIClient == nil {
return fmt.Errorf("no APIClient provided")
}
params := api.FetchParams{
Digest: artifact.DigestWithAlg(),
Limit: opts.Limit,
Owner: opts.Owner,
Repo: opts.Repo,
}
attestations, err := verification.GetRemoteAttestations(opts.APIClient, params)
attestations, err := opts.APIClient.GetByDigest(params)
if err != nil {
if errors.Is(err, api.ErrNoAttestationsFound) {
fmt.Fprintf(opts.Logger.IO.Out, "No attestations found for %s\n", opts.ArtifactPath)
@ -144,10 +146,9 @@ func runDownload(opts *Options) error {
// Apply predicate type filter to returned attestations
if opts.PredicateType != "" {
filteredAttestations := verification.FilterAttestations(opts.PredicateType, attestations)
if len(filteredAttestations) == 0 {
return fmt.Errorf("no attestations found with predicate type: %s", opts.PredicateType)
filteredAttestations, err := api.FilterAttestations(opts.PredicateType, attestations)
if err != nil {
return fmt.Errorf("failed to filter attestations: %v", err)
}
attestations = filteredAttestations

View file

@ -275,7 +275,7 @@ func TestRunDownload(t *testing.T) {
t.Run("no attestations found", func(t *testing.T) {
opts := baseOpts
opts.APIClient = api.MockClient{
OnGetByOwnerAndDigest: func(repo, digest string, limit int) ([]*api.Attestation, error) {
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
return nil, api.ErrNoAttestationsFound
},
}
@ -291,7 +291,7 @@ func TestRunDownload(t *testing.T) {
t.Run("failed to fetch attestations", func(t *testing.T) {
opts := baseOpts
opts.APIClient = api.MockClient{
OnGetByOwnerAndDigest: func(repo, digest string, limit int) ([]*api.Attestation, error) {
OnGetByDigest: func(params api.FetchParams) ([]*api.Attestation, error) {
return nil, fmt.Errorf("failed to fetch attestations")
},
}

View file

@ -20,13 +20,6 @@ const SLSAPredicateV1 = "https://slsa.dev/provenance/v1"
var ErrUnrecognisedBundleExtension = errors.New("bundle file extension not supported, must be json or jsonl")
var ErrEmptyBundleFile = errors.New("provided bundle file is empty")
type FetchRemoteAttestationsParams struct {
Digest string
Limit int
Owner string
Repo string
}
// GetLocalAttestations returns a slice of attestations read from a local bundle file.
func GetLocalAttestations(path string) ([]*api.Attestation, error) {
var attestations []*api.Attestation
@ -89,28 +82,6 @@ func loadBundlesFromJSONLinesFile(path string) ([]*api.Attestation, error) {
return attestations, nil
}
func GetRemoteAttestations(client api.Client, params FetchRemoteAttestationsParams) ([]*api.Attestation, error) {
if client == nil {
return nil, fmt.Errorf("api client must be provided")
}
// check if Repo is set first because if Repo has been set, Owner will be set using the value of Repo.
// If Repo is not set, the field will remain empty. It will not be populated using the value of Owner.
if params.Repo != "" {
attestations, err := client.GetByRepoAndDigest(params.Repo, params.Digest, params.Limit)
if err != nil {
return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Repo, err)
}
return attestations, nil
} else if params.Owner != "" {
attestations, err := client.GetByOwnerAndDigest(params.Owner, params.Digest, params.Limit)
if err != nil {
return nil, fmt.Errorf("failed to fetch attestations from %s: %w", params.Owner, err)
}
return attestations, nil
}
return nil, fmt.Errorf("owner or repo must be provided")
}
func GetOCIAttestations(client oci.Client, artifact artifact.DigestedArtifact) ([]*api.Attestation, error) {
attestations, err := client.GetAttestations(artifact.NameRef(), artifact.DigestWithAlg())
if err != nil {
@ -121,31 +92,3 @@ func GetOCIAttestations(client oci.Client, artifact artifact.DigestedArtifact) (
}
return attestations, nil
}
type IntotoStatement struct {
PredicateType string `json:"predicateType"`
}
func FilterAttestations(predicateType string, attestations []*api.Attestation) []*api.Attestation {
filteredAttestations := []*api.Attestation{}
for _, each := range attestations {
dsseEnvelope := each.Bundle.GetDsseEnvelope()
if dsseEnvelope != nil {
if dsseEnvelope.PayloadType != "application/vnd.in-toto+json" {
// Don't fail just because an entry isn't intoto
continue
}
var intotoStatement IntotoStatement
if err := json.Unmarshal([]byte(dsseEnvelope.Payload), &intotoStatement); err != nil {
// Don't fail just because a single entry can't be unmarshalled
continue
}
if intotoStatement.PredicateType == predicateType {
filteredAttestations = append(filteredAttestations, each)
}
}
}
return filteredAttestations
}

View file

@ -157,10 +157,11 @@ func TestFilterAttestations(t *testing.T) {
},
}
filtered := FilterAttestations("https://slsa.dev/provenance/v1", attestations)
filtered, err := api.FilterAttestations("https://slsa.dev/provenance/v1", attestations)
require.Len(t, filtered, 1)
require.NoError(t, err)
filtered = FilterAttestations("NonExistentPredicate", attestations)
require.Len(t, filtered, 0)
filtered, err = api.FilterAttestations("NonExistentPredicate", attestations)
require.Nil(t, filtered)
require.Error(t, err)
}

View file

@ -1,6 +1,7 @@
package verify
import (
"errors"
"fmt"
"github.com/cli/cli/v2/internal/text"
@ -10,43 +11,63 @@ import (
)
func getAttestations(o *Options, a artifact.DigestedArtifact) ([]*api.Attestation, string, error) {
// Fetch attestations from GitHub API within this if block since predicate type
// filter is done when the API is called
if o.FetchAttestationsFromGitHubAPI() {
if o.APIClient == nil {
errMsg := "✗ No APIClient provided"
return nil, errMsg, errors.New(errMsg)
}
params := api.FetchParams{
Digest: a.DigestWithAlg(),
Limit: o.Limit,
Owner: o.Owner,
PredicateType: o.PredicateType,
Repo: o.Repo,
}
attestations, err := o.APIClient.GetByDigest(params)
if err != nil {
msg := "✗ Loading attestations from GitHub API failed"
return nil, msg, err
}
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg := fmt.Sprintf("Loaded %s from GitHub API", pluralAttestation)
return attestations, msg, nil
}
// Fetch attestations from local bundle or OCI registry
// Predicate type filtering is done after the attestations are fetched
var attestations []*api.Attestation
var err error
var msg string
if o.BundlePath != "" {
attestations, err := verification.GetLocalAttestations(o.BundlePath)
attestations, err = verification.GetLocalAttestations(o.BundlePath)
if err != nil {
msg := fmt.Sprintf("✗ Loading attestations from %s failed", a.URL)
return nil, msg, err
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg = fmt.Sprintf("Loaded %s from %s", pluralAttestation, o.BundlePath)
} else {
msg = fmt.Sprintf("Loaded %d attestations from %s", len(attestations), o.BundlePath)
}
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg := fmt.Sprintf("Loaded %s from %s", pluralAttestation, o.BundlePath)
return attestations, msg, nil
}
if o.UseBundleFromRegistry {
attestations, err := verification.GetOCIAttestations(o.OCIClient, a)
} else if o.UseBundleFromRegistry {
attestations, err = verification.GetOCIAttestations(o.OCIClient, a)
if err != nil {
msg := "✗ Loading attestations from OCI registry failed"
return nil, msg, err
msg = "✗ Loading attestations from OCI registry failed"
} else {
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg = fmt.Sprintf("Loaded %s from OCI registry", pluralAttestation)
}
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg := fmt.Sprintf("Loaded %s from %s", pluralAttestation, o.ArtifactPath)
return attestations, msg, nil
}
params := verification.FetchRemoteAttestationsParams{
Digest: a.DigestWithAlg(),
Limit: o.Limit,
Owner: o.Owner,
Repo: o.Repo,
}
attestations, err := verification.GetRemoteAttestations(o.APIClient, params)
if err != nil {
msg := "✗ Loading attestations from GitHub API failed"
return nil, msg, err
}
pluralAttestation := text.Pluralize(len(attestations), "attestation")
msg := fmt.Sprintf("Loaded %s from GitHub API", pluralAttestation)
return attestations, msg, nil
filtered, err := api.FilterAttestations(o.PredicateType, attestations)
if err != nil {
return nil, err.Error(), err
}
return filtered, msg, nil
}
func verifyAttestations(art artifact.DigestedArtifact, att []*api.Attestation, sgVerifier verification.SigstoreVerifier, ec verification.EnforcementCriteria) ([]*verification.AttestationProcessingResult, string, error) {

View file

@ -0,0 +1,71 @@
package verify
import (
"testing"
"github.com/cli/cli/v2/pkg/cmd/attestation/api"
"github.com/cli/cli/v2/pkg/cmd/attestation/artifact"
"github.com/cli/cli/v2/pkg/cmd/attestation/artifact/oci"
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
"github.com/stretchr/testify/require"
)
func TestGetAttestations_OCIRegistry_PredicateTypeFiltering(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(nil, "../test/data/gh_2.60.1_windows_arm64.zip", "sha256")
require.NoError(t, err)
o := &Options{
OCIClient: oci.MockClient{},
PredicateType: verification.SLSAPredicateV1,
Repo: "cli/cli",
UseBundleFromRegistry: true,
}
attestations, msg, err := getAttestations(o, *artifact)
require.NoError(t, err)
require.Contains(t, msg, "Loaded 2 attestations from OCI registry")
require.Len(t, attestations, 2)
o.PredicateType = "custom predicate type"
attestations, msg, err = getAttestations(o, *artifact)
require.Error(t, err)
require.Contains(t, msg, "no attestations found with predicate type")
require.Nil(t, attestations)
}
func TestGetAttestations_LocalBundle_PredicateTypeFiltering(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(nil, "../test/data/gh_2.60.1_windows_arm64.zip", "sha256")
require.NoError(t, err)
o := &Options{
BundlePath: "../test/data/sigstore-js-2.1.0-bundle.json",
PredicateType: verification.SLSAPredicateV1,
Repo: "sigstore/sigstore-js",
}
attestations, _, err := getAttestations(o, *artifact)
require.NoError(t, err)
require.Len(t, attestations, 1)
o.PredicateType = "custom predicate type"
attestations, _, err = getAttestations(o, *artifact)
require.Error(t, err)
require.Nil(t, attestations)
}
func TestGetAttestations_GhAPI_NoAttestationsFound(t *testing.T) {
artifact, err := artifact.NewDigestedArtifact(nil, "../test/data/gh_2.60.1_windows_arm64.zip", "sha256")
require.NoError(t, err)
o := &Options{
APIClient: api.NewTestClient(),
PredicateType: verification.SLSAPredicateV1,
Repo: "sigstore/sigstore-js",
}
attestations, _, err := getAttestations(o, *artifact)
require.NoError(t, err)
require.Len(t, attestations, 2)
o.PredicateType = "custom predicate type"
attestations, _, err = getAttestations(o, *artifact)
require.Error(t, err)
require.Nil(t, attestations)
}

View file

@ -53,6 +53,12 @@ func (opts *Options) Clean() {
}
}
// FetchAttestationsFromGitHubAPI returns true if the command should fetch attestations from the GitHub API
// It checks that a bundle path is not provided and that the "use bundle from registry" flag is not set
func (opts *Options) FetchAttestationsFromGitHubAPI() bool {
return opts.BundlePath == "" && !opts.UseBundleFromRegistry
}
// AreFlagsValid checks that the provided flag combination is valid
// and returns an error otherwise
func (opts *Options) AreFlagsValid() error {

View file

@ -288,14 +288,6 @@ func runVerify(opts *Options) error {
// Print the message signifying success fetching attestations
opts.Logger.Println(logMsg)
// Apply predicate type filter to returned attestations
filteredAttestations := verification.FilterAttestations(ec.PredicateType, attestations)
if len(filteredAttestations) == 0 {
opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found with predicate type: %s\n"), opts.PredicateType)
return fmt.Errorf("no matching predicate found")
}
attestations = filteredAttestations
// print information about the policy that will be enforced against attestations
opts.Logger.Println("\nThe following policy criteria will be enforced:")
opts.Logger.Println(ec.BuildPolicyInformation())

View file

@ -510,7 +510,7 @@ func TestRunVerify(t *testing.T) {
err := runVerify(&customOpts)
require.Error(t, err)
require.ErrorContains(t, err, "no matching predicate found")
require.ErrorContains(t, err, "no attestations found with predicate type")
})
t.Run("with valid OCI artifact with UseBundleFromRegistry flag but no bundle return from registry", func(t *testing.T) {

View file

@ -18,6 +18,7 @@ import (
ghAPI "github.com/cli/go-gh/v2/pkg/api"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDelete(t *testing.T) {
@ -327,11 +328,12 @@ func Test_deleteRun(t *testing.T) {
func Test_gistDelete(t *testing.T) {
tests := []struct {
name string
httpStubs func(*httpmock.Registry)
hostname string
gistID string
wantErr error
name string
httpStubs func(*httpmock.Registry)
hostname string
gistID string
wantErr error
wantErrString string
}{
{
name: "successful delete",
@ -343,36 +345,34 @@ func Test_gistDelete(t *testing.T) {
},
hostname: "github.com",
gistID: "1234",
wantErr: nil,
},
{
name: "when an gist is not found, it returns a NotFoundError",
name: "when a gist is not found, it returns a NotFoundError",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("DELETE", "gists/1234"),
httpmock.StatusStringResponse(404, "{}"),
)
},
hostname: "github.com",
gistID: "1234",
wantErr: shared.NotFoundErr,
hostname: "github.com",
gistID: "1234",
wantErr: shared.NotFoundErr, // To make sure we return the pre-defined error instance.
wantErrString: "not found",
},
{
name: "when there is a non-404 error deleting the gist, that error is returned",
httpStubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("DELETE", "gists/1234"),
httpmock.StatusJSONResponse(500, `{"message": "arbitrary error"}`),
httpmock.JSONErrorResponse(500, ghAPI.HTTPError{
StatusCode: 500,
Message: "arbitrary error",
}),
)
},
hostname: "github.com",
gistID: "1234",
wantErr: api.HTTPError{
HTTPError: &ghAPI.HTTPError{
StatusCode: 500,
Message: "arbitrary error",
},
},
hostname: "github.com",
gistID: "1234",
wantErrString: "HTTP 500: arbitrary error (https://api.github.com/gists/1234)",
},
}
@ -383,12 +383,16 @@ func Test_gistDelete(t *testing.T) {
client := api.NewClientFromHTTP(&http.Client{Transport: reg})
err := deleteGist(client, tt.hostname, tt.gistID)
if tt.wantErr != nil {
assert.ErrorAs(t, err, &tt.wantErr)
if tt.wantErrString == "" && tt.wantErr == nil {
require.NoError(t, err)
} else {
assert.NoError(t, err)
if tt.wantErrString != "" {
require.EqualError(t, err, tt.wantErrString)
}
if tt.wantErr != nil {
require.ErrorIs(t, err, tt.wantErr)
}
}
})
}
}

View file

@ -177,7 +177,7 @@ func Test_deleteRun(t *testing.T) {
opts: DeleteOptions{KeyID: "ABC123", Confirmed: true},
httpStubs: func(reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "user/gpg_keys"), httpmock.StatusStringResponse(200, keysResp))
reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.StatusJSONResponse(404, api.HTTPError{
reg.Register(httpmock.REST("DELETE", "user/gpg_keys/123"), httpmock.JSONErrorResponse(404, api.HTTPError{
StatusCode: 404,
Message: "GPG key 123 not found",
}))

View file

@ -18,6 +18,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter),
ConfirmCreateIfNoneSurvey: prShared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter),
ConfirmDeleteLastComment: prShared.CommentableConfirmDeleteLastComment(f.Prompter),
OpenInBrowser: f.Browser.Browse,
}
@ -63,7 +64,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
}
fields := []string{"id", "url"}
if opts.EditLast {
if opts.EditLast || opts.DeleteLast {
fields = append(fields, "comments")
}
@ -96,7 +97,9 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)")
cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in")
cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment")
cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author")
cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the current user")
cmd.Flags().BoolVar(&opts.DeleteLast, "delete-last", false, "Delete the last comment of the current user")
cmd.Flags().BoolVar(&opts.DeleteLastConfirmed, "yes", false, "Skip the delete confirmation prompt when --delete-last is provided")
cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last")
return cmd

View file

@ -2,6 +2,7 @@ package comment
import (
"bytes"
"errors"
"fmt"
"net/http"
"os"
@ -31,11 +32,13 @@ func TestNewCmdComment(t *testing.T) {
stdin string
output shared.CommentableOptions
wantsErr bool
isTTY bool
}{
{
name: "no arguments",
input: "",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
@ -46,6 +49,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -56,6 +60,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -66,6 +71,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "test",
},
isTTY: true,
wantsErr: false,
},
{
@ -77,6 +83,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "this is on standard input",
},
isTTY: true,
wantsErr: false,
},
{
@ -87,6 +94,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "a body from file",
},
isTTY: true,
wantsErr: false,
},
{
@ -97,6 +105,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeEditor,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -107,6 +116,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeWeb,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -118,6 +128,7 @@ func TestNewCmdComment(t *testing.T) {
Body: "",
EditLast: true,
},
isTTY: true,
wantsErr: false,
},
{
@ -130,42 +141,110 @@ func TestNewCmdComment(t *testing.T) {
EditLast: true,
CreateIfNone: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag non-interactive",
input: "1 --delete-last",
isTTY: false,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation non-interactive",
input: "1 --delete-last --yes",
output: shared.CommentableOptions{
DeleteLast: true,
DeleteLastConfirmed: true,
},
isTTY: false,
wantsErr: false,
},
{
name: "delete last flag interactive",
input: "1 --delete-last",
output: shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag and pre-confirmation interactive",
input: "1 --delete-last --yes",
output: shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
DeleteLastConfirmed: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag and pre-confirmation with web flag",
input: "1 --delete-last --yes --web",
isTTY: true,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation with editor flag",
input: "1 --delete-last --yes --editor",
isTTY: true,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation with body flag",
input: "1 --delete-last --yes --body",
isTTY: true,
wantsErr: true,
},
{
name: "delete pre-confirmation without delete last flag",
input: "1 --yes",
isTTY: true,
wantsErr: true,
},
{
name: "body and body-file flags",
input: "1 --body 'test' --body-file 'test-file.txt'",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor and web flags",
input: "1 --editor --web",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor and body flags",
input: "1 --editor --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "web and body flags",
input: "1 --web --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor, web, and body flags",
input: "1 --editor --web --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "create-if-none flag without edit-last",
input: "1 --create-if-none",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
}
@ -173,9 +252,10 @@ func TestNewCmdComment(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, stdin, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
isTTY := tt.isTTY
ios.SetStdoutTTY(isTTY)
ios.SetStdinTTY(isTTY)
ios.SetStderrTTY(isTTY)
if tt.stdin != "" {
_, _ = stdin.WriteString(tt.stdin)
@ -211,6 +291,8 @@ func TestNewCmdComment(t *testing.T) {
assert.Equal(t, tt.output.Interactive, gotOpts.Interactive)
assert.Equal(t, tt.output.InputType, gotOpts.InputType)
assert.Equal(t, tt.output.Body, gotOpts.Body)
assert.Equal(t, tt.output.DeleteLast, gotOpts.DeleteLast)
assert.Equal(t, tt.output.DeleteLastConfirmed, gotOpts.DeleteLastConfirmed)
})
}
}
@ -220,6 +302,7 @@ func Test_commentRun(t *testing.T) {
name string
input *shared.CommentableOptions
emptyComments bool
comments api.Comments
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
@ -255,6 +338,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "updating last comment with interactive editor succeeds if there are comments",
@ -331,6 +415,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "creating new comment with non-interactive editor succeeds",
@ -358,6 +443,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "updating last comment with non-interactive editor succeeds if there are comments",
@ -433,6 +519,117 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n",
},
{
name: "deleting last comment non-interactively without any comment",
input: &shared.CommentableOptions{
Interactive: false,
DeleteLast: true,
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "deleting last comment interactively without any comment",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "deleting last comment non-interactively and pre-confirmed",
input: &shared.CommentableOptions{
Interactive: false,
DeleteLast: true,
DeleteLastConfirmed: true,
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and pre-confirmed",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
DeleteLastConfirmed: true,
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and confirmed",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "comment body" {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stdout: "! Deleted comments cannot be recovered.\n",
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and confirmation declined",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "comment body" {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
wantsErr: true,
stdout: "deletion not confirmed",
},
{
name: "deleting last comment interactively and confirmed with long comment body",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "Lorem ipsum dolor sit amet, consectet lo..." {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "Lorem ipsum dolor sit amet, consectet lorem ipsum again"},
}},
wantsErr: false,
stdout: "! Deleted comments cannot be recovered.\n",
stderr: "Comment deleted\n",
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
@ -458,6 +655,8 @@ func Test_commentRun(t *testing.T) {
if tt.emptyComments {
comments.Nodes = []api.Comment{}
} else if len(tt.comments.Nodes) > 0 {
comments = tt.comments
}
tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) {
@ -472,6 +671,7 @@ func Test_commentRun(t *testing.T) {
err := shared.CommentableRun(tt.input)
if tt.wantsErr {
assert.Error(t, err)
assert.Equal(t, tt.stderr, stderr.String())
return
}
assert.NoError(t, err)
@ -508,3 +708,15 @@ func mockCommentUpdate(t *testing.T, reg *httpmock.Registry) {
}),
)
}
func mockCommentDelete(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`mutation CommentDelete\b`),
httpmock.GraphQLMutation(`
{ "data": { "deleteIssueComment": {} } }`,
func(inputs map[string]interface{}) {
assert.Equal(t, "id1", inputs["id"])
},
),
)
}

View file

@ -53,3 +53,42 @@ func preloadIssueComments(client *http.Client, repo ghrepo.Interface, issue *api
issue.Comments.PageInfo.HasNextPage = false
return nil
}
func preloadClosedByPullRequestsReferences(client *http.Client, repo ghrepo.Interface, issue *api.Issue) error {
if !issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage {
return nil
}
type response struct {
Node struct {
Issue struct {
ClosedByPullRequestsReferences api.ClosedByPullRequestsReferences `graphql:"closedByPullRequestsReferences(first: 100, after: $endCursor)"`
} `graphql:"...on Issue"`
} `graphql:"node(id: $id)"`
}
variables := map[string]interface{}{
"id": githubv4.ID(issue.ID),
"endCursor": githubv4.String(issue.ClosedByPullRequestsReferences.PageInfo.EndCursor),
}
gql := api.NewClientFromHTTP(client)
for {
var query response
err := gql.Query(repo.RepoHost(), "closedByPullRequestsReferences", &query, variables)
if err != nil {
return err
}
issue.ClosedByPullRequestsReferences.Nodes = append(issue.ClosedByPullRequestsReferences.Nodes, query.Node.Issue.ClosedByPullRequestsReferences.Nodes...)
if !query.Node.Issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Node.Issue.ClosedByPullRequestsReferences.PageInfo.EndCursor)
}
issue.ClosedByPullRequestsReferences.PageInfo.HasNextPage = false
return nil
}

View file

@ -1,7 +1,6 @@
package view
import (
"errors"
"fmt"
"io"
"net/http"
@ -134,6 +133,8 @@ func viewRun(opts *ViewOptions) error {
opts.IO.DetectTerminalTheme()
opts.IO.StartProgressIndicator()
defer opts.IO.StopProgressIndicator()
lookupFields.Add("id")
issue, err := issueShared.FindIssueOrPR(httpClient, baseRepo, opts.IssueNumber, lookupFields.ToSlice())
@ -144,18 +145,21 @@ func viewRun(opts *ViewOptions) error {
if lookupFields.Contains("comments") {
// FIXME: this re-fetches the comments connection even though the initial set of 100 were
// fetched in the previous request.
err = preloadIssueComments(httpClient, baseRepo, issue)
}
opts.IO.StopProgressIndicator()
if err != nil {
var loadErr *issueShared.PartialLoadError
if opts.Exporter == nil && errors.As(err, &loadErr) {
fmt.Fprintf(opts.IO.ErrOut, "warning: %s\n", loadErr.Error())
} else {
err := preloadIssueComments(httpClient, baseRepo, issue)
if err != nil {
return err
}
}
if lookupFields.Contains("closedByPullRequestsReferences") {
err := preloadClosedByPullRequestsReferences(httpClient, baseRepo, issue)
if err != nil {
return err
}
}
opts.IO.StopProgressIndicator()
if opts.WebMode {
openURL := issue.URL
if opts.IO.IsStdoutTTY() {

View file

@ -31,6 +31,7 @@ func TestJSONFields(t *testing.T) {
"body",
"closed",
"comments",
"closedByPullRequestsReferences",
"createdAt",
"closedAt",
"id",

View file

@ -518,7 +518,7 @@ func TestPRCheckout_sameRepo(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder := shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
cs, cmdTeardown := run.Stub()
@ -539,7 +539,7 @@ func TestPRCheckout_existingBranch(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -570,7 +570,7 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO", "hubot/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder := shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
cs, cmdTeardown := run.Stub()
@ -590,7 +590,7 @@ func TestPRCheckout_differentRepo(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder := shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
cs, cmdTeardown := run.Stub()
@ -613,7 +613,7 @@ func TestPRCheckout_differentRepoForce(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder := shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
cs, cmdTeardown := run.Stub()
@ -636,7 +636,7 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -655,7 +655,7 @@ func TestPRCheckout_detachedHead(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -674,7 +674,7 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -693,7 +693,7 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO", "hubot/REPO:-foo")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -711,7 +711,7 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
pr.MaintainerCanModify = true
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -732,7 +732,7 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) {
http := &httpmock.Registry{}
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -753,7 +753,7 @@ func TestPRCheckout_force(t *testing.T) {
http := &httpmock.Registry{}
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -774,7 +774,7 @@ func TestPRCheckout_detach(t *testing.T) {
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
shared.RunCommandFinder("123", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "123", pr, baseRepo)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)

View file

@ -110,7 +110,7 @@ func TestPrClose(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
@ -133,7 +133,7 @@ func TestPrClose_alreadyClosed(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
pr.State = "CLOSED"
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
output, err := runCommand(http, true, "96")
assert.NoError(t, err)
@ -147,7 +147,7 @@ func TestPrClose_deleteBranch_sameRepo(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:blueberries")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
@ -181,7 +181,7 @@ func TestPrClose_deleteBranch_crossRepo(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO", "hubot/REPO:blueberries")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
@ -213,7 +213,7 @@ func TestPrClose_deleteBranch_sameBranch(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO:main", "OWNER/REPO:trunk")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
@ -248,7 +248,7 @@ func TestPrClose_deleteBranch_notInGitRepo(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO:main", "OWNER/REPO:trunk")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation PullRequestClose\b`),
@ -282,7 +282,7 @@ func TestPrClose_withComment(t *testing.T) {
baseRepo, pr := stubPR("OWNER/REPO", "OWNER/REPO:feature")
pr.Title = "The title of the PR"
shared.RunCommandFinder("96", pr, baseRepo)
shared.StubFinderForRunCommandStyleTests(t, "96", pr, baseRepo)
http.Register(
httpmock.GraphQL(`mutation CommentCreate\b`),

View file

@ -16,6 +16,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err
InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter),
ConfirmCreateIfNoneSurvey: shared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter),
ConfirmDeleteLastComment: shared.CommentableConfirmDeleteLastComment(f.Prompter),
OpenInBrowser: f.Browser.Browse,
}
@ -43,7 +44,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err
selector = args[0]
}
fields := []string{"id", "url"}
if opts.EditLast {
if opts.EditLast || opts.DeleteLast {
fields = append(fields, "comments")
}
finder := shared.NewFinder(f)
@ -75,7 +76,9 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err
cmd.Flags().StringVarP(&bodyFile, "body-file", "F", "", "Read body text from `file` (use \"-\" to read from standard input)")
cmd.Flags().BoolP("editor", "e", false, "Skip prompts and open the text editor to write the body in")
cmd.Flags().BoolP("web", "w", false, "Open the web browser to write the comment")
cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the same author")
cmd.Flags().BoolVar(&opts.EditLast, "edit-last", false, "Edit the last comment of the current user")
cmd.Flags().BoolVar(&opts.DeleteLast, "delete-last", false, "Delete the last comment of the current user")
cmd.Flags().BoolVar(&opts.DeleteLastConfirmed, "yes", false, "Skip the delete confirmation prompt when --delete-last is provided")
cmd.Flags().BoolVar(&opts.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last")
return cmd

View file

@ -2,6 +2,7 @@ package comment
import (
"bytes"
"errors"
"fmt"
"net/http"
"os"
@ -31,6 +32,7 @@ func TestNewCmdComment(t *testing.T) {
stdin string
output shared.CommentableOptions
wantsErr bool
isTTY bool
}{
{
name: "no arguments",
@ -40,12 +42,14 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
name: "two arguments",
input: "1 2",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
@ -56,6 +60,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -66,6 +71,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -76,6 +82,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: 0,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -86,6 +93,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "test",
},
isTTY: true,
wantsErr: false,
},
{
@ -97,6 +105,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "this is on standard input",
},
isTTY: true,
wantsErr: false,
},
{
@ -107,6 +116,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeInline,
Body: "a body from file",
},
isTTY: true,
wantsErr: false,
},
{
@ -117,6 +127,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeEditor,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -127,6 +138,7 @@ func TestNewCmdComment(t *testing.T) {
InputType: shared.InputTypeWeb,
Body: "",
},
isTTY: true,
wantsErr: false,
},
{
@ -138,6 +150,7 @@ func TestNewCmdComment(t *testing.T) {
Body: "",
EditLast: true,
},
isTTY: true,
wantsErr: false,
},
{
@ -150,42 +163,110 @@ func TestNewCmdComment(t *testing.T) {
EditLast: true,
CreateIfNone: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag non-interactive",
input: "1 --delete-last",
isTTY: false,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation non-interactive",
input: "1 --delete-last --yes",
output: shared.CommentableOptions{
DeleteLast: true,
DeleteLastConfirmed: true,
},
isTTY: false,
wantsErr: false,
},
{
name: "delete last flag interactive",
input: "1 --delete-last",
output: shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag and pre-confirmation interactive",
input: "1 --delete-last --yes",
output: shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
DeleteLastConfirmed: true,
},
isTTY: true,
wantsErr: false,
},
{
name: "delete last flag and pre-confirmation with web flag",
input: "1 --delete-last --yes --web",
isTTY: true,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation with editor flag",
input: "1 --delete-last --yes --editor",
isTTY: true,
wantsErr: true,
},
{
name: "delete last flag and pre-confirmation with body flag",
input: "1 --delete-last --yes --body",
isTTY: true,
wantsErr: true,
},
{
name: "delete pre-confirmation without delete last flag",
input: "1 --yes",
isTTY: true,
wantsErr: true,
},
{
name: "body and body-file flags",
input: "1 --body 'test' --body-file 'test-file.txt'",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor and web flags",
input: "1 --editor --web",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor and body flags",
input: "1 --editor --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "web and body flags",
input: "1 --web --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "editor, web, and body flags",
input: "1 --editor --web --body test",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
{
name: "create-if-none flag without edit-last",
input: "1 --create-if-none",
output: shared.CommentableOptions{},
isTTY: true,
wantsErr: true,
},
}
@ -193,9 +274,10 @@ func TestNewCmdComment(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, stdin, _, _ := iostreams.Test()
ios.SetStdoutTTY(true)
ios.SetStdinTTY(true)
ios.SetStderrTTY(true)
isTTY := tt.isTTY
ios.SetStdoutTTY(isTTY)
ios.SetStdinTTY(isTTY)
ios.SetStderrTTY(isTTY)
if tt.stdin != "" {
_, _ = stdin.WriteString(tt.stdin)
@ -231,6 +313,8 @@ func TestNewCmdComment(t *testing.T) {
assert.Equal(t, tt.output.Interactive, gotOpts.Interactive)
assert.Equal(t, tt.output.InputType, gotOpts.InputType)
assert.Equal(t, tt.output.Body, gotOpts.Body)
assert.Equal(t, tt.output.DeleteLast, gotOpts.DeleteLast)
assert.Equal(t, tt.output.DeleteLastConfirmed, gotOpts.DeleteLastConfirmed)
})
}
}
@ -240,6 +324,7 @@ func Test_commentRun(t *testing.T) {
name string
input *shared.CommentableOptions
emptyComments bool
comments api.Comments
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
@ -274,6 +359,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "updating last comment with interactive editor succeeds if there are comments",
@ -350,6 +436,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "creating new comment with non-interactive editor succeeds",
@ -377,6 +464,7 @@ func Test_commentRun(t *testing.T) {
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "updating last comment with non-interactive editor succeeds if there are comments",
@ -451,6 +539,117 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-456\n",
},
{
name: "deleting last comment non-interactively without any comment",
input: &shared.CommentableOptions{
Interactive: false,
DeleteLast: true,
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "deleting last comment interactively without any comment",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
},
emptyComments: true,
wantsErr: true,
stdout: "no comments found for current user",
},
{
name: "deleting last comment non-interactively and pre-confirmed",
input: &shared.CommentableOptions{
Interactive: false,
DeleteLast: true,
DeleteLastConfirmed: true,
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and pre-confirmed",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
DeleteLastConfirmed: true,
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and confirmed",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "comment body" {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
stdout: "! Deleted comments cannot be recovered.\n",
stderr: "Comment deleted\n",
},
{
name: "deleting last comment interactively and confirmation declined",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "comment body" {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "comment body"},
}},
wantsErr: true,
stdout: "deletion not confirmed",
},
{
name: "deleting last comment interactively and confirmed with long comment body",
input: &shared.CommentableOptions{
Interactive: true,
DeleteLast: true,
ConfirmDeleteLastComment: func(body string) (bool, error) {
if body != "Lorem ipsum dolor sit amet, consectet lo..." {
return false, errors.New("unexpected comment body")
}
return true, nil
},
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentDelete(t, reg)
},
comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-111", ViewerDidAuthor: true, Body: "Lorem ipsum dolor sit amet, consectet lorem ipsum again"},
}},
wantsErr: false,
stdout: "! Deleted comments cannot be recovered.\n",
stderr: "Comment deleted\n",
},
}
for _, tt := range tests {
ios, _, stdout, stderr := iostreams.Test()
@ -475,6 +674,8 @@ func Test_commentRun(t *testing.T) {
}}
if tt.emptyComments {
comments.Nodes = []api.Comment{}
} else if len(tt.comments.Nodes) > 0 {
comments = tt.comments
}
tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) {
@ -489,6 +690,7 @@ func Test_commentRun(t *testing.T) {
err := shared.CommentableRun(tt.input)
if tt.wantsErr {
assert.Error(t, err)
assert.Equal(t, tt.stderr, stderr.String())
return
}
assert.NoError(t, err)
@ -524,3 +726,15 @@ func mockCommentUpdate(t *testing.T, reg *httpmock.Registry) {
}),
)
}
func mockCommentDelete(t *testing.T, reg *httpmock.Registry) {
reg.Register(
httpmock.GraphQL(`mutation CommentDelete\b`),
httpmock.GraphQLMutation(`
{ "data": { "deleteIssueComment": {} } }`,
func(inputs map[string]interface{}) {
assert.Equal(t, "id1", inputs["id"])
},
),
)
}

View file

@ -18,6 +18,7 @@ import (
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
@ -31,6 +32,7 @@ import (
type CreateOptions struct {
// This struct stores user input and factory functions
Detector fd.Detector
HttpClient func() (*http.Client, error)
GitClient *git.Client
Config func() (gh.Config, error)
@ -363,6 +365,20 @@ func createRun(opts *CreateOptions) error {
return err
}
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
// TODO projectsV1Deprecation
// Remove this section as we should no longer need to detect
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, ctx.PRRefs.BaseRepo().RepoHost())
}
projectsV1Support := opts.Detector.ProjectsV1()
client := ctx.Client
state, err := NewIssueState(*ctx, *opts)
@ -384,7 +400,7 @@ func createRun(opts *CreateOptions) error {
if err != nil {
return err
}
openURL, err = generateCompareURL(*ctx, *state)
openURL, err = generateCompareURL(*ctx, *state, projectsV1Support)
if err != nil {
return err
}
@ -440,8 +456,7 @@ func createRun(opts *CreateOptions) error {
if err != nil {
return err
}
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
return submitPR(*opts, *ctx, *state, projectsV1Support)
}
if opts.RecoverFile != "" {
@ -518,7 +533,7 @@ func createRun(opts *CreateOptions) error {
}
}
openURL, err = generateCompareURL(*ctx, *state)
openURL, err = generateCompareURL(*ctx, *state, projectsV1Support)
if err != nil {
return err
}
@ -537,8 +552,7 @@ func createRun(opts *CreateOptions) error {
Repo: ctx.PRRefs.BaseRepo(),
State: state,
}
// TODO wm: revisit project support
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, gh.ProjectsV1Supported)
err = shared.MetadataSurvey(opts.Prompter, opts.IO, ctx.PRRefs.BaseRepo(), fetcher, state, projectsV1Support)
if err != nil {
return err
}
@ -567,13 +581,11 @@ func createRun(opts *CreateOptions) error {
if action == shared.SubmitDraftAction {
state.Draft = true
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
return submitPR(*opts, *ctx, *state, projectsV1Support)
}
if action == shared.SubmitAction {
// TODO wm: revisit project support
return submitPR(*opts, *ctx, *state, gh.ProjectsV1Supported)
return submitPR(*opts, *ctx, *state, projectsV1Support)
}
err = errors.New("expected to cancel, preview, or submit")
@ -1216,13 +1228,12 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
return pushBranch()
}
func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState) (string, error) {
func generateCompareURL(ctx CreateContext, state shared.IssueMetadataState, projectsV1Support gh.ProjectsV1Support) (string, error) {
u := ghrepo.GenerateRepoURL(
ctx.PRRefs.BaseRepo(),
"compare/%s...%s?expand=1",
url.PathEscape(ctx.PRRefs.BaseRef()), url.PathEscape(ctx.PRRefs.QualifiedHeadRef()))
// TODO wm: revisit project support
url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.PRRefs.BaseRepo(), u, state, gh.ProjectsV1Supported)
url, err := shared.WithPrAndIssueQueryParams(ctx.Client, ctx.PRRefs.BaseRepo(), u, state, projectsV1Support)
if err != nil {
return "", err
}

View file

@ -15,6 +15,7 @@ import (
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/config"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
@ -1618,6 +1619,7 @@ func Test_createRun(t *testing.T) {
}
opts := CreateOptions{}
opts.Detector = &fd.EnabledDetectorMock{}
opts.Prompter = pm
ios, _, stdout, stderr := iostreams.Test()
@ -1850,11 +1852,13 @@ func mustParseQualifiedHeadRef(ref string) shared.QualifiedHeadRef {
func Test_generateCompareURL(t *testing.T) {
tests := []struct {
name string
ctx CreateContext
state shared.IssueMetadataState
want string
wantErr bool
name string
ctx CreateContext
state shared.IssueMetadataState
httpStubs func(*testing.T, *httpmock.Registry)
projectsV1Support gh.ProjectsV1Support
want string
wantErr bool
}{
{
name: "basic",
@ -1938,10 +1942,135 @@ func Test_generateCompareURL(t *testing.T) {
want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1&template=story.md",
wantErr: false,
},
// TODO projectsV1Deprecation
// Clean up these tests, but probably keep one for general project ID resolution.
{
name: "with projects, no v1 support",
ctx: CreateContext{
PRRefs: &skipPushRefs{
qualifiedHeadRef: shared.NewQualifiedHeadRefWithoutOwner("feature"),
baseRefs: baseRefs{
baseRepo: api.InitRepoHostname(&api.Repository{Name: "REPO", Owner: api.RepositoryOwner{Login: "OWNER"}}, "github.com"),
baseBranchName: "main",
},
},
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
// Ensure no v1 projects are requestd
// ( is required to avoid matching projectsV2
reg.Exclude(t, httpmock.GraphQL(`projects\(`))
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [
{ "title": "ProjectTitle", "id": "PROJECTV2ID", "resourcePath": "/OWNER/REPO/projects/3" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
},
state: shared.IssueMetadataState{
ProjectTitles: []string{"ProjectTitle"},
},
projectsV1Support: gh.ProjectsV1Unsupported,
want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1&projects=OWNER%2FREPO%2F3",
wantErr: false,
},
{
name: "with projects, v1 support",
ctx: CreateContext{
PRRefs: &skipPushRefs{
qualifiedHeadRef: shared.NewQualifiedHeadRefWithoutOwner("feature"),
baseRefs: baseRefs{
baseRepo: api.InitRepoHostname(&api.Repository{Name: "REPO", Owner: api.RepositoryOwner{Login: "OWNER"}}, "github.com"),
baseBranchName: "main",
},
},
},
state: shared.IssueMetadataState{
ProjectTitles: []string{"ProjectV1Title"},
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
// v1 project query responses
reg.Register(
httpmock.GraphQL(`query RepositoryProjectList\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projects": {
"nodes": [
{ "name": "ProjectV1Title", "id": "PROJECTV1ID", "resourcePath": "/OWNER/REPO/projects/1" }
],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectList\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projects": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
// v2 project query responses
reg.Register(
httpmock.GraphQL(`query RepositoryProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "repository": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query OrganizationProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "organization": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
reg.Register(
httpmock.GraphQL(`query UserProjectV2List\b`),
httpmock.StringResponse(`
{ "data": { "viewer": { "projectsV2": {
"nodes": [],
"pageInfo": { "hasNextPage": false }
} } } }
`))
},
projectsV1Support: gh.ProjectsV1Supported,
want: "https://github.com/OWNER/REPO/compare/main...feature?body=&expand=1&projects=OWNER%2FREPO%2F1",
wantErr: false,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got, err := generateCompareURL(tt.ctx, tt.state)
// If http stubs are provided, register them and inject the registry into a client
// that is provided to generateCompareURL in the ctx.
if tt.httpStubs != nil {
reg := &httpmock.Registry{}
defer reg.Verify(t)
tt.httpStubs(t, reg)
tt.ctx.Client = api.NewClientFromHTTP(&http.Client{Transport: reg})
}
got, err := generateCompareURL(tt.ctx, tt.state, tt.projectsV1Support)
if (err != nil) != tt.wantErr {
t.Errorf("generateCompareURL() error = %v, wantErr %v", err, tt.wantErr)
return
@ -2008,4 +2137,438 @@ func mockRetrieveProjects(_ *testing.T, reg *httpmock.Registry) {
`))
}
// TODO interactive metadata tests once: 1) we have test utils for Prompter and 2) metadata questions use Prompter
// TODO projectsV1Deprecation
// Remove this test.
func TestProjectsV1Deprecation(t *testing.T) {
t.Run("non-interactive submission", func(t *testing.T) {
t.Run("when projects v1 is supported, queries for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
reg.Register(
// ( is required to avoid matching projectsV2
httpmock.GraphQL(`projects\(`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = createRun(&CreateOptions{
Detector: &fd.EnabledDetectorMock{},
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "upstream",
Resolved: "base",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Finder: shared.NewMockFinder("feature", nil, nil),
HeadBranch: "feature",
TitleProvided: true,
BodyProvided: true,
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request contained projects
reg.Verify(t)
})
t.Run("when projects v1 is not supported, does not query for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
// ( is required to avoid matching projectsV2
reg.Exclude(t, httpmock.GraphQL(`projects\(`))
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
// Ignore the error because we're not really interested in it.
_ = createRun(&CreateOptions{
Detector: &fd.DisabledDetectorMock{},
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "upstream",
Resolved: "base",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Finder: shared.NewMockFinder("feature", nil, nil),
HeadBranch: "feature",
TitleProvided: true,
BodyProvided: true,
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request contained projectCards
reg.Verify(t)
})
})
t.Run("interactive submission", func(t *testing.T) {
t.Run("when projects v1 is supported, queries for it", func(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
cs.Register("git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature", 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "")
// When the command is run
reg := &httpmock.Registry{}
reg.StubRepoResponse("OWNER", "REPO")
reg.Register(
httpmock.GraphQL(`query PullRequestTemplates\b`),
httpmock.StringResponse(`{ "data": { "repository": { "pullRequestTemplates": [] } } }`),
)
reg.Register(
// ( is required to avoid matching projectsV2
httpmock.GraphQL(`projects\(`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
// Register a handler to check for projects V2 just to avoid the registry panicking, even
// though we return a 500 error. This is because the project lookup is done in parallel
// so the previous error doesn't early exit.
reg.Register(
httpmock.GraphQL(`projectsV2`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
ios, _, _, _ := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
pm := &prompter.PrompterMock{}
pm.InputFunc = func(p, _ string) (string, error) {
if p == "Title (required)" {
return "Test Title", nil
} else {
return "", prompter.NoSuchPromptErr(p)
}
}
pm.MarkdownEditorFunc = func(p, _ string, ba bool) (string, error) {
if p == "Body" {
return "Test Body", nil
} else {
return "", prompter.NoSuchPromptErr(p)
}
}
pm.SelectFunc = func(p, _ string, opts []string) (int, error) {
switch p {
case "Choose a template":
return 0, nil
case "What's next?":
return prompter.IndexFor(opts, "Add metadata")
default:
return -1, prompter.NoSuchPromptErr(p)
}
}
pm.MultiSelectFunc = func(p string, _ []string, opts []string) ([]int, error) {
return prompter.IndexesFor(opts, "Projects")
}
opts := CreateOptions{
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
Browser: &browser.Stub{},
IO: ios,
Prompter: pm,
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Finder: shared.NewMockFinder("feature", nil, nil),
Detector: &fd.EnabledDetectorMock{},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Branch: func() (string, error) {
return "feature", nil
},
HeadBranch: "feature",
}
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = createRun(&opts)
// Verify that our request contained projects
reg.Verify(t)
})
t.Run("when projects v1 is not supported, does not query for it", func(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
cs.Register("git -c log.ShowSignature=false log --pretty=format:%H%x00%s%x00%b%x00 --cherry origin/master...feature", 0, "")
cs.Register(`git rev-parse --show-toplevel`, 0, "")
// When the command is run
reg := &httpmock.Registry{}
reg.StubRepoResponse("OWNER", "REPO")
reg.Register(
httpmock.GraphQL(`query PullRequestTemplates\b`),
httpmock.StringResponse(`{ "data": { "repository": { "pullRequestTemplates": [] } } }`),
)
// ( is required to avoid matching projectsV2
reg.Exclude(t, httpmock.GraphQL(`projects\(`))
ios, _, _, _ := iostreams.Test()
ios.SetStdinTTY(true)
ios.SetStdoutTTY(true)
ios.SetStderrTTY(true)
pm := &prompter.PrompterMock{}
pm.InputFunc = func(p, _ string) (string, error) {
if p == "Title (required)" {
return "Test Title", nil
} else {
return "", prompter.NoSuchPromptErr(p)
}
}
pm.MarkdownEditorFunc = func(p, _ string, ba bool) (string, error) {
if p == "Body" {
return "Test Body", nil
} else {
return "", prompter.NoSuchPromptErr(p)
}
}
pm.SelectFunc = func(p, _ string, opts []string) (int, error) {
switch p {
case "Choose a template":
return 0, nil
case "What's next?":
return prompter.IndexFor(opts, "Add metadata")
default:
return -1, prompter.NoSuchPromptErr(p)
}
}
pm.MultiSelectFunc = func(p string, _ []string, opts []string) ([]int, error) {
return prompter.IndexesFor(opts, "Projects")
}
opts := CreateOptions{
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
Browser: &browser.Stub{},
IO: ios,
Prompter: pm,
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Finder: shared.NewMockFinder("feature", nil, nil),
Detector: &fd.DisabledDetectorMock{},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "origin",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Branch: func() (string, error) {
return "feature", nil
},
HeadBranch: "feature",
}
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = createRun(&opts)
// Verify that our request did not contain projectCards
reg.Verify(t)
})
})
t.Run("web mode", func(t *testing.T) {
t.Run("when projects v1 is supported, queries for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
reg.Register(
// ( is required to avoid matching projectsV2
httpmock.GraphQL(`projects\(`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = createRun(&CreateOptions{
Detector: &fd.EnabledDetectorMock{},
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "upstream",
Resolved: "base",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Finder: shared.NewMockFinder("feature", nil, nil),
WebMode: true,
HeadBranch: "feature",
TitleProvided: true,
BodyProvided: true,
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request contained projects
reg.Verify(t)
})
t.Run("when projects v1 is not supported, does not query for it", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.StubRepoInfoResponse("OWNER", "REPO", "main")
// ( is required to avoid matching projectsV2
reg.Exclude(t, httpmock.GraphQL(`projects\(`))
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
// Ignore the error because we're not really interested in it.
_ = createRun(&CreateOptions{
Detector: &fd.DisabledDetectorMock{},
IO: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
GitClient: &git.Client{
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
Remotes: func() (context.Remotes, error) {
return context.Remotes{
{
Remote: &git.Remote{
Name: "upstream",
Resolved: "base",
},
Repo: ghrepo.New("OWNER", "REPO"),
},
}, nil
},
Finder: shared.NewMockFinder("feature", nil, nil),
WebMode: true,
HeadBranch: "feature",
TitleProvided: true,
BodyProvided: true,
Title: "Test Title",
Body: "Test Body",
// Required to force a lookup of projects
Projects: []string{"Project"},
})
// Verify that our request did not contain projectCards
reg.Verify(t)
})
})
}

View file

@ -307,7 +307,7 @@ func TestPrMerge(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -348,7 +348,7 @@ func TestPrMerge_blocked(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -379,7 +379,7 @@ func TestPrMerge_dirty(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -413,7 +413,7 @@ func TestPrMerge_nontty(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -451,7 +451,7 @@ func TestPrMerge_editMessage_nontty(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -490,7 +490,7 @@ func TestPrMerge_withRepoFlag(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -529,7 +529,7 @@ func TestPrMerge_withMatchCommitHeadFlag(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -570,7 +570,7 @@ func TestPrMerge_withAuthorFlag(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -612,7 +612,7 @@ func TestPrMerge_deleteBranch(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -663,7 +663,7 @@ func TestPrMerge_deleteBranch_mergeQueue(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -686,7 +686,7 @@ func TestPrMerge_deleteBranch_nonDefault(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -737,7 +737,7 @@ func TestPrMerge_deleteBranch_onlyLocally(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -785,7 +785,7 @@ func TestPrMerge_deleteBranch_checkoutNewBranch(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -836,7 +836,7 @@ func TestPrMerge_deleteNonCurrentBranch(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"blueberries",
&api.PullRequest{
ID: "PR_10",
@ -893,7 +893,7 @@ func Test_nonDivergingPullRequest(t *testing.T) {
}
stubCommit(pr, "COMMITSHA1")
shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "main"))
shared.StubFinderForRunCommandStyleTests(t, "", pr, baseRepo("OWNER", "REPO", "main"))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
@ -933,7 +933,7 @@ func Test_divergingPullRequestWarning(t *testing.T) {
}
stubCommit(pr, "COMMITSHA1")
shared.RunCommandFinder("", pr, baseRepo("OWNER", "REPO", "main"))
shared.StubFinderForRunCommandStyleTests(t, "", pr, baseRepo("OWNER", "REPO", "main"))
http.Register(
httpmock.GraphQL(`mutation PullRequestMerge\b`),
@ -964,7 +964,7 @@ func Test_pullRequestWithoutCommits(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "PR_10",
@ -1003,7 +1003,7 @@ func TestPrMerge_rebase(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"2",
&api.PullRequest{
ID: "THE-ID",
@ -1044,7 +1044,7 @@ func TestPrMerge_squash(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"3",
&api.PullRequest{
ID: "THE-ID",
@ -1084,7 +1084,7 @@ func TestPrMerge_alreadyMerged(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"4",
&api.PullRequest{
ID: "THE-ID",
@ -1129,7 +1129,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"4",
&api.PullRequest{
ID: "THE-ID",
@ -1159,7 +1159,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_TTY(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"4",
&api.PullRequest{
ID: "THE-ID",
@ -1200,7 +1200,7 @@ func TestPrMerge_alreadyMerged_withMergeStrategy_crossRepo(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"4",
&api.PullRequest{
ID: "THE-ID",
@ -1239,7 +1239,7 @@ func TestPRMergeTTY(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "THE-ID",
@ -1305,7 +1305,7 @@ func TestPRMergeTTY_withDeleteBranch(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{
ID: "THE-ID",
@ -1468,7 +1468,7 @@ func TestPRMergeEmptyStrategyNonTTY(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1495,7 +1495,7 @@ func TestPRTTY_cancelled(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"",
&api.PullRequest{ID: "THE-ID", Number: 123, Title: "title", MergeStateStatus: "CLEAN"},
ghrepo.New("OWNER", "REPO"),
@ -1679,7 +1679,7 @@ func TestPrInMergeQueue(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1710,7 +1710,7 @@ func TestPrAddToMergeQueueWithMergeMethod(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1748,7 +1748,7 @@ func TestPrAddToMergeQueueClean(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1788,7 +1788,7 @@ func TestPrAddToMergeQueueBlocked(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1828,7 +1828,7 @@ func TestPrAddToMergeQueueAdmin(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",
@ -1897,7 +1897,7 @@ func TestPrAddToMergeQueueAdminWithMergeStrategy(t *testing.T) {
http := initFakeHTTP()
defer http.Verify(t)
shared.RunCommandFinder(
shared.StubFinderForRunCommandStyleTests(t,
"1",
&api.PullRequest{
ID: "THE-ID",

View file

@ -124,7 +124,7 @@ func TestPRReady(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "OPEN",
@ -149,7 +149,7 @@ func TestPRReady_alreadyReady(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "OPEN",
@ -166,7 +166,7 @@ func TestPRReadyUndo(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "OPEN",
@ -191,7 +191,7 @@ func TestPRReadyUndo_alreadyDraft(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "OPEN",
@ -208,7 +208,7 @@ func TestPRReady_closed(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "CLOSED",

View file

@ -53,7 +53,7 @@ func TestPRReopen(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "CLOSED",
@ -78,7 +78,7 @@ func TestPRReopen_alreadyOpen(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "OPEN",
@ -95,7 +95,7 @@ func TestPRReopen_alreadyMerged(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "MERGED",
@ -112,7 +112,7 @@ func TestPRReopen_withComment(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("123", &api.PullRequest{
shared.StubFinderForRunCommandStyleTests(t, "123", &api.PullRequest{
ID: "THE-ID",
Number: 123,
State: "CLOSED",

View file

@ -235,7 +235,7 @@ func TestPRReview(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID"}, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "", &api.PullRequest{ID: "THE-ID"}, ghrepo.New("OWNER", "REPO"))
http.Register(
httpmock.GraphQL(`mutation PullRequestReviewAdd\b`),
@ -261,7 +261,7 @@ func TestPRReview_interactive(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
http.Register(
httpmock.GraphQL(`mutation PullRequestReviewAdd\b`),
@ -293,7 +293,7 @@ func TestPRReview_interactive_no_body(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
pm := &prompter.PrompterMock{
SelectFunc: func(_, _ string, _ []string) (int, error) { return 2, nil },
@ -308,7 +308,7 @@ func TestPRReview_interactive_blank_approve(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "", &api.PullRequest{ID: "THE-ID", Number: 123}, ghrepo.New("OWNER", "REPO"))
http.Register(
httpmock.GraphQL(`mutation PullRequestReviewAdd\b`),

View file

@ -18,6 +18,7 @@ import (
)
var errNoUserComments = errors.New("no comments found for current user")
var errDeleteNotConfirmed = errors.New("deletion not confirmed")
type InputType int
@ -41,11 +42,14 @@ type CommentableOptions struct {
InteractiveEditSurvey func(string) (string, error)
ConfirmSubmitSurvey func() (bool, error)
ConfirmCreateIfNoneSurvey func() (bool, error)
ConfirmDeleteLastComment func(string) (bool, error)
OpenInBrowser func(string) error
Interactive bool
InputType InputType
Body string
EditLast bool
DeleteLast bool
DeleteLastConfirmed bool
CreateIfNone bool
Quiet bool
Host string
@ -74,6 +78,21 @@ func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error {
return cmdutil.FlagErrorf("`--create-if-none` can only be used with `--edit-last`")
}
if opts.DeleteLastConfirmed && !opts.DeleteLast {
return cmdutil.FlagErrorf("`--yes` should only be used with `--delete-last`")
}
if opts.DeleteLast {
if inputFlags > 0 {
return cmdutil.FlagErrorf("should not provide comment body when using `--delete-last`")
}
if opts.IO.CanPrompt() || opts.DeleteLastConfirmed {
opts.Interactive = opts.IO.CanPrompt()
return nil
}
return cmdutil.FlagErrorf("should provide `--yes` to confirm deletion in non-interactive mode")
}
if inputFlags == 0 {
if !opts.IO.CanPrompt() {
return cmdutil.FlagErrorf("flags required when not running interactively")
@ -92,6 +111,9 @@ func CommentableRun(opts *CommentableOptions) error {
return err
}
opts.Host = repo.RepoHost()
if opts.DeleteLast {
return deleteComment(commentable, opts)
}
// Create new comment, bail before complexities of updating the last comment
if !opts.EditLast {
@ -236,6 +258,53 @@ func updateComment(commentable Commentable, opts *CommentableOptions) error {
return nil
}
func deleteComment(commentable Commentable, opts *CommentableOptions) error {
comments := commentable.CurrentUserComments()
if len(comments) == 0 {
return errNoUserComments
}
lastComment := comments[len(comments)-1]
cs := opts.IO.ColorScheme()
if opts.Interactive && !opts.DeleteLastConfirmed {
// This is not an ideal way of truncating a random string that may
// contain emojis or other kind of wide chars.
truncated := lastComment.Body
if len(lastComment.Body) > 40 {
truncated = lastComment.Body[:40] + "..."
}
fmt.Fprintf(opts.IO.Out, "%s Deleted comments cannot be recovered.\n", cs.WarningIcon())
ok, err := opts.ConfirmDeleteLastComment(truncated)
if err != nil {
return err
}
if !ok {
return errDeleteNotConfirmed
}
}
httpClient, err := opts.HttpClient()
if err != nil {
return err
}
apiClient := api.NewClientFromHTTP(httpClient)
params := api.CommentDeleteInput{CommentId: lastComment.Identifier()}
deletionErr := api.CommentDelete(apiClient, opts.Host, params)
if deletionErr != nil {
return deletionErr
}
if !opts.Quiet {
fmt.Fprintln(opts.IO.ErrOut, "Comment deleted")
}
return nil
}
func CommentableConfirmSubmitSurvey(p Prompt) func() (bool, error) {
return func() (bool, error) {
return p.Confirm("Submit?", true)
@ -271,6 +340,12 @@ func CommentableEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams
}
}
func CommentableConfirmDeleteLastComment(p Prompt) func(string) (bool, error) {
return func(body string) (bool, error) {
return p.Confirm(fmt.Sprintf("Delete the comment: %q?", body), true)
}
}
func waitForEnter(r io.Reader) error {
scanner := bufio.NewScanner(r)
scanner.Scan()

View file

@ -137,7 +137,7 @@ func (e Editable) ProjectIds() (*[]string, error) {
s.RemoveValues(e.Projects.Remove)
e.Projects.Value = s.ToSlice()
}
p, _, err := e.Metadata.ProjectsToIDs(e.Projects.Value)
p, _, err := e.Metadata.ProjectsTitlesToIDs(e.Projects.Value)
return &p, err
}
@ -171,14 +171,14 @@ func (e Editable) ProjectV2Ids() (*[]string, *[]string, error) {
var err error
if addTitles.Len() > 0 {
_, addIds, err = e.Metadata.ProjectsToIDs(addTitles.ToSlice())
_, addIds, err = e.Metadata.ProjectsTitlesToIDs(addTitles.ToSlice())
if err != nil {
return nil, nil, err
}
}
if removeTitles.Len() > 0 {
_, removeIds, err = e.Metadata.ProjectsToIDs(removeTitles.ToSlice())
_, removeIds, err = e.Metadata.ProjectsTitlesToIDs(removeTitles.ToSlice())
if err != nil {
return nil, nil, err
}

View file

@ -333,12 +333,12 @@ func tryDetermineDefaultPushTarget(gitClient GitConfigClient, localBranchName st
}
// We assume the PR's branch name is the same as whatever was provided, unless the user has specified
// push.default = upstream or tracking, then we use the branch name from the merge ref.
// push.default = upstream or tracking, then we use the branch name from the merge ref if it exists. Otherwise, we fall back to the local branch name
remoteBranch := localBranchName
if pushDefault == git.PushDefaultUpstream || pushDefault == git.PushDefaultTracking {
remoteBranch = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
if remoteBranch == "" {
return defaultPushTarget{}, fmt.Errorf("could not determine remote branch name")
mergeRef := strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
if mergeRef != "" {
remoteBranch = mergeRef
}
}

View file

@ -462,7 +462,7 @@ func TestTryDetermineDefaultPRHead(t *testing.T) {
})
}
t.Run("but if the merge ref is empty, error", func(t *testing.T) {
t.Run("but if the merge ref is empty, use the provided branch name", func(t *testing.T) {
t.Parallel()
repoResolvedFromPushRemoteClient := stubGitConfigClient{
@ -474,12 +474,14 @@ func TestTryDetermineDefaultPRHead(t *testing.T) {
pushDefaultFn: stubPushDefault(git.PushDefaultUpstream, nil),
}
_, err := TryDetermineDefaultPRHead(
defaultPRHead, err := TryDetermineDefaultPRHead(
repoResolvedFromPushRemoteClient,
stubRemoteToRepoResolver(ghContext.Remotes{&baseRemote, &forkRemote}, nil),
"feature-branch",
)
require.Error(t, err)
require.NoError(t, err)
require.Equal(t, "feature-branch", defaultPRHead.BranchName)
})
})

View file

@ -10,12 +10,14 @@ import (
"sort"
"strconv"
"strings"
"testing"
"time"
"github.com/cli/cli/v2/api"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
o "github.com/cli/cli/v2/pkg/option"
@ -54,9 +56,9 @@ type finder struct {
}
func NewFinder(factory *cmdutil.Factory) PRFinder {
if runCommandFinder != nil {
f := runCommandFinder
runCommandFinder = &mockFinder{err: errors.New("you must use a RunCommandFinder to stub PR lookups")}
if finderForRunCommandStyleTests != nil {
f := finderForRunCommandStyleTests
finderForRunCommandStyleTests = &mockFinder{err: errors.New("you must use StubFinderForRunCommandStyleTests to stub PR lookups")}
return f
}
@ -70,12 +72,23 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
}
}
var runCommandFinder PRFinder
var finderForRunCommandStyleTests PRFinder
// RunCommandFinder is the NewMockFinder substitute to be used ONLY in runCommand-style tests.
func RunCommandFinder(selector string, pr *api.PullRequest, repo ghrepo.Interface) *mockFinder {
// StubFinderForRunCommandStyleTests is the NewMockFinder substitute to be used ONLY in runCommand-style tests.
func StubFinderForRunCommandStyleTests(t *testing.T, selector string, pr *api.PullRequest, repo ghrepo.Interface) *mockFinder {
// Create a new mock finder and override the "runCommandFinder" variable so that calls to
// NewFinder() will return this mock. This is a bad pattern, and a result of old style runCommand
// tests that would ideally be replaced. The reason we need to do this is that the runCommand style tests
// construct the cobra command via NewCmd* functions, and then Execute them directly, providing no opportunity
// to inject a test double unless it's on the factory, which finder never is, because only PR commands need it.
finder := NewMockFinder(selector, pr, repo)
runCommandFinder = finder
finderForRunCommandStyleTests = finder
// Ensure that at the end of the test, we reset the "runCommandFinder" variable so that tests are isolated,
// at least if they are run sequentially.
t.Cleanup(func() {
finderForRunCommandStyleTests = nil
})
return finder
}
@ -89,6 +102,8 @@ type FindOptions struct {
BaseBranch string
// States lists the possible PR states to scope the PR-for-branch lookup to.
States []string
Detector fd.Detector
}
func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, error) {
@ -193,9 +208,11 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
fields.AddValues([]string{"id", "number"}) // for additional preload queries below
if fields.Contains("isInMergeQueue") || fields.Contains("isMergeQueueEnabled") {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
detector := fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
prFeatures, err := detector.PullRequestFeatures()
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
}
prFeatures, err := opts.Detector.PullRequestFeatures()
if err != nil {
return nil, nil, err
}
@ -211,8 +228,23 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
fields.Remove("projectItems")
}
// TODO projectsV1Deprecation
// Remove this block
// When removing this, remember to remove `projectCards` from the list of default fields in pr/view.go
if fields.Contains("projectCards") {
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
}
if opts.Detector.ProjectsV1() == gh.ProjectsV1Unsupported {
fields.Remove("projectCards")
}
}
var pr *api.PullRequest
if f.prNumber > 0 {
// If we have a PR number, let's look it up
if numberFieldOnly {
// avoid hitting the API if we already have all the information
return &api.PullRequest{Number: f.prNumber}, f.baseRefRepo, nil
@ -221,11 +253,16 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
if err != nil {
return pr, f.baseRefRepo, err
}
} else {
} else if prRefs.BaseRepo() != nil && f.branchName != "" {
// No PR number, but we have a base repo and branch name.
pr, err = findForRefs(httpClient, prRefs, opts.States, fields.ToSlice())
if err != nil {
return pr, f.baseRefRepo, err
}
} else {
// If we don't have a PR number or a base repo and branch name,
// we can't do anything
return nil, f.baseRefRepo, &NotFoundError{fmt.Errorf("no pull requests found")}
}
g, _ := errgroup.WithContext(context.Background())
@ -239,6 +276,11 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
return preloadPrComments(httpClient, f.baseRefRepo, pr)
})
}
if fields.Contains("closingIssuesReferences") {
g.Go(func() error {
return preloadPrClosingIssuesReferences(httpClient, f.baseRefRepo, pr)
})
}
if fields.Contains("statusCheckRollup") {
g.Go(func() error {
return preloadPrChecks(httpClient, f.baseRefRepo, pr)
@ -452,6 +494,45 @@ func preloadPrComments(client *http.Client, repo ghrepo.Interface, pr *api.PullR
return nil
}
func preloadPrClosingIssuesReferences(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error {
if !pr.ClosingIssuesReferences.PageInfo.HasNextPage {
return nil
}
type response struct {
Node struct {
PullRequest struct {
ClosingIssuesReferences api.ClosingIssuesReferences `graphql:"closingIssuesReferences(first: 100, after: $endCursor)"`
} `graphql:"...on PullRequest"`
} `graphql:"node(id: $id)"`
}
variables := map[string]interface{}{
"id": githubv4.ID(pr.ID),
"endCursor": githubv4.String(pr.ClosingIssuesReferences.PageInfo.EndCursor),
}
gql := api.NewClientFromHTTP(client)
for {
var query response
err := gql.Query(repo.RepoHost(), "closingIssuesReferences", &query, variables)
if err != nil {
return err
}
pr.ClosingIssuesReferences.Nodes = append(pr.ClosingIssuesReferences.Nodes, query.Node.PullRequest.ClosingIssuesReferences.Nodes...)
if !query.Node.PullRequest.ClosingIssuesReferences.PageInfo.HasNextPage {
break
}
variables["endCursor"] = githubv4.String(query.Node.PullRequest.ClosingIssuesReferences.PageInfo.EndCursor)
}
pr.ClosingIssuesReferences.PageInfo.HasNextPage = false
return nil
}
func preloadPrChecks(client *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error {
if len(pr.StatusCheckRollup.Nodes) == 0 {
return nil

View file

@ -165,6 +165,23 @@ func TestFind(t *testing.T) {
wantPR: 13,
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "pr number zero",
args: args{
selector: "0",
fields: []string{"number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
gitConfigClient: stubGitConfigClient{
readBranchConfigFn: stubBranchConfig(git.BranchConfig{}, nil),
pushDefaultFn: stubPushDefault(git.PushDefaultSimple, nil),
remotePushDefaultFn: stubRemotePushDefault("", nil),
},
},
wantErr: true,
},
{
name: "number with hash argument",
args: args{

View file

@ -36,7 +36,7 @@ func WithPrAndIssueQueryParams(client *api.Client, baseRepo ghrepo.Interface, ba
q.Set("labels", strings.Join(state.Labels, ","))
}
if len(state.ProjectTitles) > 0 {
projectPaths, err := api.ProjectNamesToPaths(client, baseRepo, state.ProjectTitles, projectsV1Support)
projectPaths, err := api.ProjectTitlesToPaths(client, baseRepo, state.ProjectTitles, projectsV1Support)
if err != nil {
return "", fmt.Errorf("could not add to project: %w", err)
}
@ -119,7 +119,7 @@ func AddMetadataToIssueParams(client *api.Client, baseRepo ghrepo.Interface, par
}
params["labelIds"] = labelIDs
projectIDs, projectV2IDs, err := tb.MetadataResult.ProjectsToIDs(tb.ProjectTitles)
projectIDs, projectV2IDs, err := tb.MetadataResult.ProjectsTitlesToIDs(tb.ProjectTitles)
if err != nil {
return fmt.Errorf("could not add to project: %w", err)
}

View file

@ -10,6 +10,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
@ -22,6 +23,9 @@ import (
type ViewOptions struct {
IO *iostreams.IOStreams
Browser browser.Browser
// TODO projectsV1Deprecation
// Remove this detector since it is only used for test validation.
Detector fd.Detector
Finder shared.PRFinder
Exporter cmdutil.Exporter
@ -89,6 +93,7 @@ func viewRun(opts *ViewOptions) error {
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: defaultFields,
Detector: opts.Detector,
}
if opts.BrowserMode {
findOptions.Fields = []string{"url"}

View file

@ -12,6 +12,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/browser"
fd "github.com/cli/cli/v2/internal/featuredetection"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/run"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
@ -37,6 +38,7 @@ func TestJSONFields(t *testing.T) {
"changedFiles",
"closed",
"closedAt",
"closingIssuesReferences",
"comments",
"commits",
"createdAt",
@ -175,6 +177,9 @@ func runCommand(rt http.RoundTripper, branch string, isTTY bool, cli string) (*t
factory := &cmdutil.Factory{
IOStreams: ios,
Browser: browser,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: rt}, nil
},
}
cmd := NewCmdView(factory, nil)
@ -403,7 +408,7 @@ func TestPRView_Preview_nontty(t *testing.T) {
pr, err := prFromFixtures(tc.fixtures)
require.NoError(t, err)
shared.RunCommandFinder("12", pr, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "12", pr, ghrepo.New("OWNER", "REPO"))
output, err := runCommand(http, tc.branch, false, tc.args)
if err != nil {
@ -607,7 +612,7 @@ func TestPRView_Preview(t *testing.T) {
pr, err := prFromFixtures(tc.fixtures)
require.NoError(t, err)
shared.RunCommandFinder("12", pr, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "12", pr, ghrepo.New("OWNER", "REPO"))
output, err := runCommand(http, tc.branch, true, tc.args)
if err != nil {
@ -630,7 +635,7 @@ func TestPRView_web_currentBranch(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/10"}, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "", &api.PullRequest{URL: "https://github.com/OWNER/REPO/pull/10"}, ghrepo.New("OWNER", "REPO"))
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -649,7 +654,7 @@ func TestPRView_web_noResultsForBranch(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
shared.RunCommandFinder("", nil, nil)
shared.StubFinderForRunCommandStyleTests(t, "", nil, nil)
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
@ -741,9 +746,9 @@ func TestPRView_tty_Comments(t *testing.T) {
if len(tt.fixtures) > 0 {
pr, err := prFromFixtures(tt.fixtures)
require.NoError(t, err)
shared.RunCommandFinder("123", pr, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "123", pr, ghrepo.New("OWNER", "REPO"))
} else {
shared.RunCommandFinder("123", nil, nil)
shared.StubFinderForRunCommandStyleTests(t, "123", nil, nil)
}
output, err := runCommand(http, tt.branch, true, tt.cli)
@ -852,9 +857,9 @@ func TestPRView_nontty_Comments(t *testing.T) {
if len(tt.fixtures) > 0 {
pr, err := prFromFixtures(tt.fixtures)
require.NoError(t, err)
shared.RunCommandFinder("123", pr, ghrepo.New("OWNER", "REPO"))
shared.StubFinderForRunCommandStyleTests(t, "123", pr, ghrepo.New("OWNER", "REPO"))
} else {
shared.RunCommandFinder("123", nil, nil)
shared.StubFinderForRunCommandStyleTests(t, "123", nil, nil)
}
output, err := runCommand(http, tt.branch, false, tt.cli)
@ -869,3 +874,74 @@ func TestPRView_nontty_Comments(t *testing.T) {
})
}
}
// TODO projectsV1Deprecation
// Remove this test.
func TestProjectsV1Deprecation(t *testing.T) {
t.Run("when projects v1 is supported, is included in query", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.Register(
httpmock.GraphQL(`projectCards`),
// Simulate a GraphQL error to early exit the test.
httpmock.StatusStringResponse(500, ""),
)
f := &cmdutil.Factory{
IOStreams: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
}
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = viewRun(&ViewOptions{
IO: ios,
Finder: shared.NewFinder(f),
Detector: &fd.EnabledDetectorMock{},
SelectorArg: "https://github.com/cli/cli/pull/123",
})
// Verify that our request contained projectCards
reg.Verify(t)
})
t.Run("when projects v1 is not supported, is not included in query", func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
reg := &httpmock.Registry{}
reg.Exclude(
t,
httpmock.GraphQL(`projectCards`),
)
f := &cmdutil.Factory{
IOStreams: ios,
HttpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
}
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
// Ignore the error because we have no way to really stub it without
// fully stubbing a GQL error structure in the request body.
_ = viewRun(&ViewOptions{
IO: ios,
Finder: shared.NewFinder(f),
Detector: &fd.DisabledDetectorMock{},
SelectorArg: "https://github.com/cli/cli/pull/123",
})
// Verify that our request contained projectCards
reg.Verify(t)
})
}

View file

@ -7,6 +7,7 @@ import (
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/cli/go-gh/v2/pkg/api"
"github.com/stretchr/testify/require"
)
@ -14,10 +15,10 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
repo := ghrepo.New("OWNER", "REPO")
tests := []struct {
name string
id string
stubStatus int
stubRespJSON string
name string
id string
stubStatus int
stubResp any
expectErr bool
expectedErrMsg string
@ -31,17 +32,18 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
name: "404 repo or autolink not found",
id: "123",
stubStatus: http.StatusNotFound,
stubRespJSON: `{}`, // API response not used in output
expectErr: true,
expectedErrMsg: "error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
{
name: "500 unexpected error",
id: "123",
stubRespJSON: `{"messsage": "arbitrary error"}`,
name: "500 unexpected error",
id: "123",
stubResp: api.HTTPError{
Message: "arbitrary error",
},
stubStatus: http.StatusInternalServerError,
expectErr: true,
expectedErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
expectedErrMsg: "HTTP 500: arbitrary error (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
}
@ -53,7 +55,7 @@ func TestAutolinkDeleter_Delete(t *testing.T) {
http.MethodDelete,
fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id),
),
httpmock.StatusJSONResponse(tt.stubStatus, tt.stubRespJSON),
httpmock.StatusJSONResponse(tt.stubStatus, tt.stubResp),
)
defer reg.Verify(t)

View file

@ -109,8 +109,6 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
return
}
namePadding := 12
type helpEntry struct {
Title string
Body string
@ -135,6 +133,12 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
helpEntries = append(helpEntries, helpEntry{"ALIASES", strings.Join(BuildAliasList(command, command.Aliases), ", ") + "\n"})
}
// Statically calculated padding for non-extension commands,
// longest is `gh accessibility` with 13 characters + 1 space.
//
// Should consider novel way to calculate this in the future [AF]
namePadding := 14
for _, g := range GroupedCommands(command) {
var names []string
for _, c := range g.Commands {
@ -148,6 +152,9 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
if isRootCmd(command) {
var helpTopics []string
if c := findCommand(command, "accessibility"); c != nil {
helpTopics = append(helpTopics, rpad(c.Name()+":", namePadding)+c.Short)
}
if c := findCommand(command, "actions"); c != nil {
helpTopics = append(helpTopics, rpad(c.Name()+":", namePadding)+c.Short)
}
@ -183,6 +190,7 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, _ []string) {
Use %[1]sgh <command> <subcommand> --help%[1]s for more information about a command.
Read the manual at https://cli.github.com/manual
Learn about exit codes using %[1]sgh help exit-codes%[1]s
Learn about accessibility experiences using %[1]sgh help accessibility%[1]s
`, "`")})
out := f.IOStreams.Out

View file

@ -6,6 +6,7 @@ import (
"strings"
"github.com/MakeNowJust/heredoc"
accessibilityCmd "github.com/cli/cli/v2/pkg/cmd/accessibility"
actionsCmd "github.com/cli/cli/v2/pkg/cmd/actions"
aliasCmd "github.com/cli/cli/v2/pkg/cmd/alias"
"github.com/cli/cli/v2/pkg/cmd/alias/shared"
@ -122,6 +123,7 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) (*cobra.Command,
// Child commands
cmd.AddCommand(versionCmd.NewCmdVersion(f, version, buildDate))
cmd.AddCommand(accessibilityCmd.NewCmdAccessibility(f))
cmd.AddCommand(actionsCmd.NewCmdActions(f))
cmd.AddCommand(aliasCmd.NewCmdAlias(f))
cmd.AddCommand(authCmd.NewCmdAuth(f))

View file

@ -316,7 +316,7 @@ func TestWatchRun(t *testing.T) {
)
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/runs/1234"),
httpmock.StatusJSONResponse(404, api.HTTPError{
httpmock.JSONErrorResponse(404, api.HTTPError{
StatusCode: 404,
Message: "run 1234 not found",
}),

View file

@ -9,6 +9,8 @@ import (
"os"
"regexp"
"strings"
"github.com/cli/go-gh/v2/pkg/api"
)
type Matcher func(req *http.Request) bool
@ -161,6 +163,9 @@ func JSONResponse(body interface{}) Responder {
}
}
// StatusJSONResponse turns the given argument into a JSON response.
//
// The argument is not meant to be a JSON string, unless it's intentional.
func StatusJSONResponse(status int, body interface{}) Responder {
return func(req *http.Request) (*http.Response, error) {
b, _ := json.Marshal(body)
@ -171,6 +176,12 @@ func StatusJSONResponse(status int, body interface{}) Responder {
}
}
// JSONErrorResponse is a type-safe helper to avoid confusion around the
// provided argument.
func JSONErrorResponse(status int, err api.HTTPError) Responder {
return StatusJSONResponse(status, err)
}
func FileResponse(filename string) Responder {
return func(req *http.Request) (*http.Response, error) {
f, err := os.Open(filename)

View file

@ -14,3 +14,9 @@ if ! $ghBuildPath attestation verify "$ghCLIArtifact" --digest-alg=sha256 --owne
echo "Failed to verify"
exit 1
fi
# Try to verify when specifying a predicate type that does not match the attestation
if $ghBuildPath attestation verify "$ghCLIArtifact" --digest-alg=sha256 --owner=cli --predicate-type=my-custom-predicate-type; then
echo "Verification should have failed"
exit 1
fi