STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge branch 'trunk' into attestation-verify-ref-commit-policy-opts

Browse source
This commit is contained in:
Meredith Lancaster 2025-02-24 10:59:58 -07:00 • committed by GitHub
commit ba5cf8e1ce
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
123 changed files with 5086 additions and 1353 deletions
Download patch file Download diff file Expand all files Collapse all files

14
pkg/cmd/actions/actions.go
View file

@ -34,19 +34,19 @@ func actionsExplainer(cs *iostreams.ColorScheme) string {
GitHub CLI integrates with GitHub Actions to help you manage runs and workflows.
%[3]s
gh run list: List recent workflow runs
gh run view: View details for a workflow run or one of its jobs
gh run watch: Watch a workflow run while it executes
gh run rerun: Rerun a failed workflow run
gh run list: List recent workflow runs
gh run view: View details for a workflow run or one of its jobs
gh run watch: Watch a workflow run while it executes
gh run rerun: Rerun a failed workflow run
gh run download: Download artifacts generated by runs
To see more help, run %[1]sgh help run <subcommand>%[1]s
%[4]s
gh workflow list: List workflow files in your repository
gh workflow view: View details for a workflow file
gh workflow enable: Enable a workflow file
gh workflow disable: Disable a workflow file
gh workflow view: View details for a workflow file
gh workflow enable: Enable a workflow file
gh workflow disable: Disable a workflow file
gh workflow run: Trigger a workflow_dispatch run for a workflow file
To see more help, run %[1]sgh help workflow <subcommand>%[1]s

34
pkg/cmd/api/api.go
View file

@ -201,12 +201,12 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command
Annotations: map[string]string{
"help:environment": heredoc.Doc(`
GH_TOKEN, GITHUB_TOKEN (in order of precedence): an authentication token for
github.com API requests.
<github.com> API requests.
GH_ENTERPRISE_TOKEN, GITHUB_ENTERPRISE_TOKEN (in order of precedence): an
authentication token for API requests to GitHub Enterprise.
GH_HOST: make the request to a GitHub host other than github.com.
GH_HOST: make the request to a GitHub host other than <github.com>.
`),
},
Args: cobra.ExactArgs(1),
@ -239,17 +239,19 @@ func NewCmdApi(f *cmdutil.Factory, runF func(*ApiOptions) error) *cobra.Command
return err
}
if opts.Slurp && !opts.Paginate {
return cmdutil.FlagErrorf("`--paginate` required when passing `--slurp`")
}
if opts.Slurp {
if err := cmdutil.MutuallyExclusive(
"the `--slurp` option is not supported with `--jq` or `--template`",
opts.Slurp,
opts.FilterOutput != "",
opts.Template != "",
); err != nil {
return err
}
if err := cmdutil.MutuallyExclusive(
"the `--slurp` option is not supported with `--jq` or `--template`",
opts.Slurp,
opts.FilterOutput != "",
opts.Template != "",
); err != nil {
return err
if !opts.Paginate {
return cmdutil.FlagErrorf("`--paginate` required when passing `--slurp`")
}
}
if err := cmdutil.MutuallyExclusive(
@ -461,9 +463,11 @@ func processResponse(resp *http.Response, opts *ApiOptions, bodyWriter, headersW
var serverError string
if isJSON && (opts.RequestPath == "graphql" || resp.StatusCode >= 400) {
responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode)
if err != nil {
return
if !strings.EqualFold(opts.RequestMethod, "HEAD") {
responseBody, serverError, err = parseErrorResponse(responseBody, resp.StatusCode)
if err != nil {
return
}
}
}

29
pkg/cmd/api/api_test.go
View file

@ -1234,6 +1234,35 @@ func Test_apiRun_DELETE(t *testing.T) {
}
}
func Test_apiRun_HEAD(t *testing.T) {
ios, _, _, _ := iostreams.Test()
err := apiRun(&ApiOptions{
IO: ios,
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
HttpClient: func() (*http.Client, error) {
var tr roundTripper = func(req *http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: 422,
Request: req,
Header: map[string][]string{
"Content-Type": {"application/json"},
}}, nil
}
return &http.Client{Transport: tr}, nil
},
MagicFields: []string(nil),
RawFields: []string(nil),
RequestMethod: "HEAD",
RequestMethodPassed: true,
})
if err != cmdutil.SilentError {
t.Fatalf("got error %v", err)
}
}
func Test_apiRun_inputFile(t *testing.T) {
tests := []struct {
name string

16
pkg/cmd/attestation/api/attestation.go
View file

@ -1,8 +1,7 @@
package api
import (
"fmt"
"errors"
"github.com/sigstore/sigstore-go/pkg/bundle"
)
@ -11,18 +10,7 @@ const (
GetAttestationByOwnerAndSubjectDigestPath = "orgs/%s/attestations/%s"
)
type ErrNoAttestations struct {
name string
digest string
}
func (e ErrNoAttestations) Error() string {
return fmt.Sprintf("no attestations found for digest %s in %s", e.name, e.digest)
}
func newErrNoAttestations(name, digest string) ErrNoAttestations {
return ErrNoAttestations{name, digest}
}
var ErrNoAttestationsFound = errors.New("no attestations found")
type Attestation struct {
Bundle *bundle.Bundle `json:"bundle"`

108
pkg/cmd/attestation/api/client.go
View file

@ -60,44 +60,26 @@ func NewLiveClient(hc *http.Client, host string, l *ioconfig.Handler) *LiveClien
}
}
func (c *LiveClient) BuildRepoAndDigestURL(repo, digest string) string {
repo = strings.Trim(repo, "/")
return fmt.Sprintf(GetAttestationByRepoAndSubjectDigestPath, repo, digest)
}
// GetByRepoAndDigest fetches the attestation by repo and digest
func (c *LiveClient) GetByRepoAndDigest(repo, digest string, limit int) ([]*Attestation, error) {
url := c.BuildRepoAndDigestURL(repo, digest)
attestations, err := c.getAttestations(url, repo, digest, limit)
if err != nil {
return nil, err
}
bundles, err := c.fetchBundleFromAttestations(attestations)
if err != nil {
return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
}
return bundles, nil
}
func (c *LiveClient) BuildOwnerAndDigestURL(owner, digest string) string {
owner = strings.Trim(owner, "/")
return fmt.Sprintf(GetAttestationByOwnerAndSubjectDigestPath, owner, digest)
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) {
url := c.BuildOwnerAndDigestURL(owner, digest)
attestations, err := c.getAttestations(url, owner, digest, limit)
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)
if err != nil {
return nil, err
}
if len(attestations) == 0 {
return nil, newErrNoAttestations(owner, digest)
}
bundles, err := c.fetchBundleFromAttestations(attestations)
if err != nil {
return nil, fmt.Errorf("failed to fetch bundle with URL: %w", err)
@ -112,9 +94,7 @@ func (c *LiveClient) GetTrustDomain() (string, error) {
return c.getTrustDomain(MetaPath)
}
func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*Attestation, error) {
c.logger.VerbosePrintf("Fetching attestations for artifact digest %s\n\n", digest)
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)
@ -157,7 +137,7 @@ func (c *LiveClient) getAttestations(url, name, digest string, limit int) ([]*At
}
if len(attestations) == 0 {
return nil, newErrNoAttestations(name, digest)
return nil, ErrNoAttestationsFound
}
if len(attestations) > limit {
@ -176,7 +156,7 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
return fmt.Errorf("attestation has no bundle or bundle URL")
}
// for now, we fallback to the bundle field if the bundle URL is empty
// for now, we fall back to the bundle field if the bundle URL is empty
if a.BundleURL == "" {
c.logger.VerbosePrintf("Bundle URL is empty. Falling back to bundle field\n\n")
fetched[i] = &Attestation{
@ -186,13 +166,14 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
}
// otherwise fetch the bundle with the provided URL
b, err := c.GetBundle(a.BundleURL)
b, err := c.getBundle(a.BundleURL)
if err != nil {
return fmt.Errorf("failed to fetch bundle with URL: %w", err)
}
fetched[i] = &Attestation{
Bundle: b,
}
return nil
})
}
@ -204,38 +185,49 @@ func (c *LiveClient) fetchBundleFromAttestations(attestations []*Attestation) ([
return fetched, nil
}
func (c *LiveClient) GetBundle(url string) (*bundle.Bundle, error) {
func (c *LiveClient) getBundle(url string) (*bundle.Bundle, error) {
c.logger.VerbosePrintf("Fetching attestation bundle with bundle URL\n\n")
resp, err := c.httpClient.Get(url)
if err != nil {
return nil, err
}
var sgBundle *bundle.Bundle
bo := backoff.NewConstantBackOff(getAttestationRetryInterval)
err := backoff.Retry(func() error {
resp, err := c.httpClient.Get(url)
if err != nil {
return fmt.Errorf("request to fetch bundle from URL failed: %w", err)
}
defer resp.Body.Close()
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("unexpected status code: %d", resp.StatusCode)
}
if resp.StatusCode >= 500 && resp.StatusCode <= 599 {
return fmt.Errorf("attestation bundle with URL %s returned status code %d", url, resp.StatusCode)
}
body, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("failed to read blob storage response body: %w", err)
}
defer resp.Body.Close()
body, err := io.ReadAll(resp.Body)
if err != nil {
return fmt.Errorf("failed to read blob storage response body: %w", err)
}
var out []byte
decompressed, err := snappy.Decode(out, body)
if err != nil {
return nil, fmt.Errorf("failed to decompress with snappy: %w", err)
}
var out []byte
decompressed, err := snappy.Decode(out, body)
if err != nil {
return backoff.Permanent(fmt.Errorf("failed to decompress with snappy: %w", err))
}
var pbBundle v1.Bundle
if err = protojson.Unmarshal(decompressed, &pbBundle); err != nil {
return nil, fmt.Errorf("failed to unmarshal to bundle: %w", err)
}
var pbBundle v1.Bundle
if err = protojson.Unmarshal(decompressed, &pbBundle); err != nil {
return backoff.Permanent(fmt.Errorf("failed to unmarshal to bundle: %w", err))
}
c.logger.VerbosePrintf("Successfully fetched bundle\n\n")
c.logger.VerbosePrintf("Successfully fetched bundle\n\n")
return bundle.NewBundle(&pbBundle)
sgBundle, err = bundle.NewBundle(&pbBundle)
if err != nil {
return backoff.Permanent(fmt.Errorf("failed to create new bundle: %w", err))
}
return nil
}, backoff.WithMaxRetries(bo, 3))
return sgBundle, err
}
func shouldRetry(err error) bool {

127
pkg/cmd/attestation/api/client_test.go
View file

@ -42,24 +42,6 @@ func NewClientWithMockGHClient(hasNextPage bool) Client {
}
}
func TestGetURL(t *testing.T) {
c := LiveClient{}
testData := []struct {
repo string
digest string
expected string
}{
{repo: "/github/example/", digest: "sha256:12313213", expected: "repos/github/example/attestations/sha256:12313213"},
{repo: "/github/example", digest: "sha256:12313213", expected: "repos/github/example/attestations/sha256:12313213"},
}
for _, data := range testData {
s := c.BuildRepoAndDigestURL(data.repo, data.digest)
require.Equal(t, data.expected, s)
}
}
func TestGetByDigest(t *testing.T) {
c := NewClientWithMockGHClient(false)
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
@ -150,12 +132,12 @@ func TestGetByDigest_NoAttestationsFound(t *testing.T) {
attestations, err := c.GetByRepoAndDigest(testRepo, testDigest, DefaultLimit)
require.Error(t, err)
require.IsType(t, ErrNoAttestations{}, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
attestations, err = c.GetByOwnerAndDigest(testOwner, testDigest, DefaultLimit)
require.Error(t, err)
require.IsType(t, ErrNoAttestations{}, err)
require.IsType(t, ErrNoAttestationsFound, err)
require.Nil(t, attestations)
}
@ -180,7 +162,7 @@ func TestGetByDigest_Error(t *testing.T) {
require.Nil(t, attestations)
}
func TestFetchBundleFromAttestations(t *testing.T) {
func TestFetchBundleFromAttestations_BundleURL(t *testing.T) {
httpClient := &mockHttpClient{}
client := LiveClient{
httpClient: httpClient,
@ -193,43 +175,50 @@ func TestFetchBundleFromAttestations(t *testing.T) {
fetched, err := client.fetchBundleFromAttestations(attestations)
require.NoError(t, err)
require.Len(t, fetched, 2)
require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
require.NotNil(t, "application/vnd.dev.sigstore.bundle.v0.3+json", fetched[0].Bundle.GetMediaType())
httpClient.AssertNumberOfCalls(t, "OnGetSuccess", 2)
}
func TestFetchBundleFromAttestations_InvalidAttestation(t *testing.T) {
func TestFetchBundleFromAttestations_MissingBundleAndBundleURLFields(t *testing.T) {
httpClient := &mockHttpClient{}
client := LiveClient{
httpClient: httpClient,
logger: io.NewTestHandler(),
}
// If both the BundleURL and Bundle fields are empty, the function should
// return an error indicating that
att1 := Attestation{}
attestations := []*Attestation{&att1}
fetched, err := client.fetchBundleFromAttestations(attestations)
require.Error(t, err)
require.Nil(t, fetched, 2)
bundles, err := client.fetchBundleFromAttestations(attestations)
require.ErrorContains(t, err, "attestation has no bundle or bundle URL")
require.Nil(t, bundles, 2)
}
func TestFetchBundleFromAttestations_Fail(t *testing.T) {
httpClient := &failAfterOneCallHttpClient{}
func TestFetchBundleFromAttestations_FailOnTheSecondAttestation(t *testing.T) {
mockHTTPClient := &failAfterNCallsHttpClient{
// the initial HTTP request will succeed, which returns a bundle for the first attestation
// all following HTTP requests will fail, which means the function fails to fetch a bundle
// for the second attestation and the function returns an error
FailOnCallN: 2,
FailOnAllSubsequentCalls: true,
}
c := &LiveClient{
httpClient: httpClient,
httpClient: mockHTTPClient,
logger: io.NewTestHandler(),
}
att1 := makeTestAttestation()
att2 := makeTestAttestation()
attestations := []*Attestation{&att1, &att2}
fetched, err := c.fetchBundleFromAttestations(attestations)
bundles, err := c.fetchBundleFromAttestations(attestations)
require.Error(t, err)
require.Nil(t, fetched)
httpClient.AssertNumberOfCalls(t, "OnGetFailAfterOneCall", 2)
require.Nil(t, bundles)
}
func TestFetchBundleFromAttestations_FetchByURLFail(t *testing.T) {
mockHTTPClient := &failHttpClient{}
func TestFetchBundleFromAttestations_FailAfterRetrying(t *testing.T) {
mockHTTPClient := &reqFailHttpClient{}
c := &LiveClient{
httpClient: mockHTTPClient,
@ -241,10 +230,10 @@ func TestFetchBundleFromAttestations_FetchByURLFail(t *testing.T) {
bundle, err := c.fetchBundleFromAttestations(attestations)
require.Error(t, err)
require.Nil(t, bundle)
mockHTTPClient.AssertNumberOfCalls(t, "OnGetFail", 1)
mockHTTPClient.AssertNumberOfCalls(t, "OnGetReqFail", 4)
}
func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
func TestFetchBundleFromAttestations_FallbackToBundleField(t *testing.T) {
mockHTTPClient := &mockHttpClient{}
c := &LiveClient{
@ -252,6 +241,7 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
logger: io.NewTestHandler(),
}
// If the bundle URL is empty, the code will fallback to the bundle field
a := Attestation{Bundle: data.SigstoreBundle(t)}
attestations := []*Attestation{&a}
fetched, err := c.fetchBundleFromAttestations(attestations)
@ -260,6 +250,71 @@ func TestFetchBundleByURL_FallbackToBundleField(t *testing.T) {
mockHTTPClient.AssertNotCalled(t, "OnGetSuccess")
}
// getBundle successfully fetches a bundle on the first HTTP request attempt
func TestGetBundle(t *testing.T) {
mockHTTPClient := &mockHttpClient{}
c := &LiveClient{
httpClient: mockHTTPClient,
logger: io.NewTestHandler(),
}
b, err := c.getBundle("https://mybundleurl.com")
require.NoError(t, err)
require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", b.GetMediaType())
mockHTTPClient.AssertNumberOfCalls(t, "OnGetSuccess", 1)
}
// getBundle retries successfully when the initial HTTP request returns
// a 5XX status code
func TestGetBundle_SuccessfulRetry(t *testing.T) {
mockHTTPClient := &failAfterNCallsHttpClient{
FailOnCallN: 1,
FailOnAllSubsequentCalls: false,
}
c := &LiveClient{
httpClient: mockHTTPClient,
logger: io.NewTestHandler(),
}
b, err := c.getBundle("mybundleurl")
require.NoError(t, err)
require.Equal(t, "application/vnd.dev.sigstore.bundle.v0.3+json", b.GetMediaType())
mockHTTPClient.AssertNumberOfCalls(t, "OnGetFailAfterNCalls", 2)
}
// getBundle does not retry when the function fails with a permanent backoff error condition
func TestGetBundle_PermanentBackoffFail(t *testing.T) {
mockHTTPClient := &invalidBundleClient{}
c := &LiveClient{
httpClient: mockHTTPClient,
logger: io.NewTestHandler(),
}
b, err := c.getBundle("mybundleurl")
// var permanent *backoff.PermanentError
//require.IsType(t, &backoff.PermanentError{}, err)
require.Error(t, err)
require.Nil(t, b)
mockHTTPClient.AssertNumberOfCalls(t, "OnGetInvalidBundle", 1)
}
// getBundle retries when the HTTP request fails
func TestGetBundle_RequestFail(t *testing.T) {
mockHTTPClient := &reqFailHttpClient{}
c := &LiveClient{
httpClient: mockHTTPClient,
logger: io.NewTestHandler(),
}
b, err := c.getBundle("mybundleurl")
require.Error(t, err)
require.Nil(t, b)
mockHTTPClient.AssertNumberOfCalls(t, "OnGetReqFail", 4)
}
func TestGetTrustDomain(t *testing.T) {
fetcher := mockMetaGenerator{
TrustDomain: "foo",

48
pkg/cmd/attestation/api/mock_httpClient_test.go
View file

@ -5,6 +5,7 @@ import (
"fmt"
"io"
"net/http"
"sync"
"github.com/cli/cli/v2/pkg/cmd/attestation/test/data"
"github.com/golang/snappy"
@ -27,34 +28,59 @@ func (m *mockHttpClient) Get(url string) (*http.Response, error) {
}, nil
}
type failHttpClient struct {
type invalidBundleClient struct {
mock.Mock
}
func (m *failHttpClient) Get(url string) (*http.Response, error) {
m.On("OnGetFail").Return()
m.MethodCalled("OnGetFail")
func (m *invalidBundleClient) Get(url string) (*http.Response, error) {
m.On("OnGetInvalidBundle").Return()
m.MethodCalled("OnGetInvalidBundle")
var compressed []byte
compressed = snappy.Encode(compressed, []byte("invalid bundle bytes"))
return &http.Response{
StatusCode: 200,
Body: io.NopCloser(bytes.NewReader(compressed)),
}, nil
}
type reqFailHttpClient struct {
mock.Mock
}
func (m *reqFailHttpClient) Get(url string) (*http.Response, error) {
m.On("OnGetReqFail").Return()
m.MethodCalled("OnGetReqFail")
return &http.Response{
StatusCode: 500,
}, fmt.Errorf("failed to fetch with %s", url)
}
type failAfterOneCallHttpClient struct {
type failAfterNCallsHttpClient struct {
mock.Mock
mu sync.Mutex
FailOnCallN int
FailOnAllSubsequentCalls bool
NumCalls int
}
func (m *failAfterOneCallHttpClient) Get(url string) (*http.Response, error) {
m.On("OnGetFailAfterOneCall").Return()
func (m *failAfterNCallsHttpClient) Get(url string) (*http.Response, error) {
m.mu.Lock()
defer m.mu.Unlock()
if len(m.Calls) >= 1 {
m.MethodCalled("OnGetFailAfterOneCall")
m.On("OnGetFailAfterNCalls").Return()
m.NumCalls++
if m.NumCalls == m.FailOnCallN || (m.NumCalls > m.FailOnCallN && m.FailOnAllSubsequentCalls) {
m.MethodCalled("OnGetFailAfterNCalls")
return &http.Response{
StatusCode: 500,
}, fmt.Errorf("failed to fetch with %s", url)
}, nil
}
m.MethodCalled("OnGetFailAfterOneCall")
m.MethodCalled("OnGetFailAfterNCalls")
var compressed []byte
compressed = snappy.Encode(compressed, data.SigstoreBundleRaw)
return &http.Response{

2
pkg/cmd/attestation/download/download.go
View file

@ -135,7 +135,7 @@ func runDownload(opts *Options) error {
}
attestations, err := verification.GetRemoteAttestations(opts.APIClient, params)
if err != nil {
if errors.Is(err, api.ErrNoAttestations{}) {
if errors.Is(err, api.ErrNoAttestationsFound) {
fmt.Fprintf(opts.Logger.IO.Out, "No attestations found for %s\n", opts.ArtifactPath)
return nil
}

2
pkg/cmd/attestation/download/download_test.go
View file

@ -276,7 +276,7 @@ func TestRunDownload(t *testing.T) {
opts := baseOpts
opts.APIClient = api.MockClient{
OnGetByOwnerAndDigest: func(repo, digest string, limit int) ([]*api.Attestation, error) {
return nil, api.ErrNoAttestations{}
return nil, api.ErrNoAttestationsFound
},
}

2
pkg/cmd/attestation/inspect/inspect.go
View file

@ -178,7 +178,7 @@ func runInspect(opts *Options) error {
}
// summarize cert if present
if leafCert := verificationContent.GetCertificate(); leafCert != nil {
if leafCert := verificationContent.Certificate(); leafCert != nil {
certSummary, err := certificate.SummarizeCertificate(leafCert)
if err != nil {

5
pkg/cmd/attestation/trustedroot/trustedroot.go
View file

@ -11,6 +11,7 @@ import (
"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"
o "github.com/cli/cli/v2/pkg/option"
ghauth "github.com/cli/go-gh/v2/pkg/auth"
"github.com/MakeNowJust/heredoc"
@ -121,7 +122,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error {
var tufOptions []tufConfig
var defaultTR = "trusted_root.json"
tufOpt := verification.DefaultOptionsWithCacheSetting()
tufOpt := verification.DefaultOptionsWithCacheSetting(o.None[string]())
// Disable local caching, so we get up-to-date response from TUF repository
tufOpt.CacheValidity = 0
@ -150,7 +151,7 @@ func getTrustedRoot(makeTUF tufClientInstantiator, opts *Options) error {
targets: []string{defaultTR},
})
tufOpt = verification.GitHubTUFOptions()
tufOpt = verification.GitHubTUFOptions(o.None[string]())
tufOpt.CacheValidity = 0
tufOptions = append(tufOptions, tufConfig{
tufOptions: tufOpt,

41
pkg/cmd/attestation/verification/sigstore.go
View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/api"
"github.com/cli/cli/v2/pkg/cmd/attestation/io"
o "github.com/cli/cli/v2/pkg/option"
"github.com/sigstore/sigstore-go/pkg/bundle"
"github.com/sigstore/sigstore-go/pkg/root"
@ -34,6 +35,8 @@ type SigstoreConfig struct {
NoPublicGood bool
// If tenancy mode is not used, trust domain is empty
TrustDomain string
// TUFMetadataDir
TUFMetadataDir o.Option[string]
}
type SigstoreVerifier interface {
@ -45,7 +48,8 @@ type LiveSigstoreVerifier struct {
Logger *io.Handler
NoPublicGood bool
// If tenancy mode is not used, trust domain is empty
TrustDomain string
TrustDomain string
TUFMetadataDir o.Option[string]
}
var ErrNoAttestationsVerified = errors.New("no attestations were verified")
@ -55,10 +59,11 @@ var ErrNoAttestationsVerified = errors.New("no attestations were verified")
// Public Good, GitHub, or a custom trusted root.
func NewLiveSigstoreVerifier(config SigstoreConfig) *LiveSigstoreVerifier {
return &LiveSigstoreVerifier{
TrustedRoot: config.TrustedRoot,
Logger: config.Logger,
NoPublicGood: config.NoPublicGood,
TrustDomain: config.TrustDomain,
TrustedRoot: config.TrustedRoot,
Logger: config.Logger,
NoPublicGood: config.NoPublicGood,
TrustDomain: config.TrustDomain,
TUFMetadataDir: config.TUFMetadataDir,
}
}
@ -70,7 +75,7 @@ func getBundleIssuer(b *bundle.Bundle) (string, error) {
if err != nil {
return "", fmt.Errorf("failed to get bundle verification content: %v", err)
}
leafCert := verifyContent.GetCertificate()
leafCert := verifyContent.Certificate()
if leafCert == nil {
return "", fmt.Errorf("leaf cert not found")
}
@ -89,9 +94,9 @@ func (v *LiveSigstoreVerifier) chooseVerifier(issuer string) (*verify.SignedEnti
if v.NoPublicGood {
return nil, fmt.Errorf("detected public good instance but requested verification without public good instance")
}
return newPublicGoodVerifier()
return newPublicGoodVerifier(v.TUFMetadataDir)
case GitHubIssuerOrg:
return newGitHubVerifier(v.TrustDomain)
return newGitHubVerifier(v.TrustDomain, v.TUFMetadataDir)
default:
return nil, fmt.Errorf("leaf certificate issuer is not recognized")
}
@ -116,7 +121,11 @@ func (v *LiveSigstoreVerifier) chooseVerifier(issuer string) (*verify.SignedEnti
// Compare bundle leafCert issuer with trusted root cert authority
certAuthorities := trustedRoot.FulcioCertificateAuthorities()
for _, certAuthority := range certAuthorities {
lowestCert, err := getLowestCertInChain(&certAuthority)
fulcioCertAuthority, ok := certAuthority.(*root.FulcioCertificateAuthority)
if !ok {
return nil, fmt.Errorf("trusted root cert authority is not a FulcioCertificateAuthority")
}
lowestCert, err := getLowestCertInChain(fulcioCertAuthority)
if err != nil {
return nil, err
}
@ -149,10 +158,8 @@ func (v *LiveSigstoreVerifier) chooseVerifier(issuer string) (*verify.SignedEnti
return nil, fmt.Errorf("unable to use provided trusted roots")
}
func getLowestCertInChain(ca *root.CertificateAuthority) (*x509.Certificate, error) {
if ca.Leaf != nil {
return ca.Leaf, nil
} else if len(ca.Intermediates) > 0 {
func getLowestCertInChain(ca *root.FulcioCertificateAuthority) (*x509.Certificate, error) {
if len(ca.Intermediates) > 0 {
return ca.Intermediates[0], nil
} else if ca.Root != nil {
return ca.Root, nil
@ -253,10 +260,10 @@ func newCustomVerifier(trustedRoot *root.TrustedRoot) (*verify.SignedEntityVerif
return gv, nil
}
func newGitHubVerifier(trustDomain string) (*verify.SignedEntityVerifier, error) {
func newGitHubVerifier(trustDomain string, tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) {
var tr string
opts := GitHubTUFOptions()
opts := GitHubTUFOptions(tufMetadataDir)
client, err := tuf.New(opts)
if err != nil {
return nil, fmt.Errorf("failed to create TUF client: %v", err)
@ -287,8 +294,8 @@ func newGitHubVerifierWithTrustedRoot(trustedRoot *root.TrustedRoot) (*verify.Si
return gv, nil
}
func newPublicGoodVerifier() (*verify.SignedEntityVerifier, error) {
opts := DefaultOptionsWithCacheSetting()
func newPublicGoodVerifier(tufMetadataDir o.Option[string]) (*verify.SignedEntityVerifier, error) {
opts := DefaultOptionsWithCacheSetting(tufMetadataDir)
client, err := tuf.New(opts)
if err != nil {
return nil, fmt.Errorf("failed to create TUF client: %v", err)

44
pkg/cmd/attestation/verification/sigstore_integration_test.go
View file

@ -9,6 +9,7 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/artifact"
"github.com/cli/cli/v2/pkg/cmd/attestation/io"
"github.com/cli/cli/v2/pkg/cmd/attestation/test"
o "github.com/cli/cli/v2/pkg/option"
"github.com/sigstore/sigstore-go/pkg/verify"
"github.com/stretchr/testify/require"
@ -48,25 +49,29 @@ func TestLiveSigstoreVerifier(t *testing.T) {
}
for _, tc := range testcases {
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
t.Run(tc.name, func(t *testing.T) {
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
TUFMetadataDir: o.Some(t.TempDir()),
})
results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t))
if tc.expectErr {
require.Error(t, err)
require.ErrorContains(t, err, tc.errContains)
require.Nil(t, results)
} else {
require.NoError(t, err)
require.Equal(t, len(tc.attestations), len(results))
}
})
results, err := verifier.Verify(tc.attestations, publicGoodPolicy(t))
if tc.expectErr {
require.Error(t, err, "test case: %s", tc.name)
require.ErrorContains(t, err, tc.errContains, "test case: %s", tc.name)
require.Nil(t, results, "test case: %s", tc.name)
} else {
require.Equal(t, len(tc.attestations), len(results), "test case: %s", tc.name)
require.NoError(t, err, "test case: %s", tc.name)
}
}
t.Run("with 2/3 verified attestations", func(t *testing.T) {
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
Logger: io.NewTestHandler(),
TUFMetadataDir: o.Some(t.TempDir()),
})
invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json")
@ -82,7 +87,8 @@ func TestLiveSigstoreVerifier(t *testing.T) {
t.Run("fail with 0/2 verified attestations", func(t *testing.T) {
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
Logger: io.NewTestHandler(),
TUFMetadataDir: o.Some(t.TempDir()),
})
invalidBundle := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0-bundle-v0.1.json")
@ -105,7 +111,8 @@ func TestLiveSigstoreVerifier(t *testing.T) {
attestations := getAttestationsFor(t, "../test/data/github_provenance_demo-0.0.12-py3-none-any-bundle.jsonl")
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
Logger: io.NewTestHandler(),
TUFMetadataDir: o.Some(t.TempDir()),
})
results, err := verifier.Verify(attestations, githubPolicy)
@ -117,8 +124,9 @@ func TestLiveSigstoreVerifier(t *testing.T) {
attestations := getAttestationsFor(t, "../test/data/sigstore-js-2.1.0_with_2_bundles.jsonl")
verifier := NewLiveSigstoreVerifier(SigstoreConfig{
Logger: io.NewTestHandler(),
TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"),
Logger: io.NewTestHandler(),
TrustedRoot: test.NormalizeRelativePath("../test/data/trusted_root.json"),
TUFMetadataDir: o.Some(t.TempDir()),
})
results, err := verifier.Verify(attestations, publicGoodPolicy(t))

11
pkg/cmd/attestation/verification/tuf.go
View file

@ -5,6 +5,7 @@ import (
"os"
"path/filepath"
o "github.com/cli/cli/v2/pkg/option"
"github.com/cli/go-gh/v2/pkg/config"
"github.com/sigstore/sigstore-go/pkg/tuf"
)
@ -14,7 +15,7 @@ var githubRoot []byte
const GitHubTUFMirror = "https://tuf-repo.github.com"
func DefaultOptionsWithCacheSetting() *tuf.Options {
func DefaultOptionsWithCacheSetting(tufMetadataDir o.Option[string]) *tuf.Options {
opts := tuf.DefaultOptions()
// The CODESPACES environment variable will be set to true in a Codespaces workspace
@ -25,8 +26,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options {
opts.DisableLocalCache = true
}
// Set the cache path to a directory owned by the CLI
opts.CachePath = filepath.Join(config.CacheDir(), ".sigstore", "root")
// Set the cache path to the provided dir, or a directory owned by the CLI
opts.CachePath = tufMetadataDir.UnwrapOr(filepath.Join(config.CacheDir(), ".sigstore", "root"))
// Allow TUF cache for 1 day
opts.CacheValidity = 1
@ -34,8 +35,8 @@ func DefaultOptionsWithCacheSetting() *tuf.Options {
return opts
}
func GitHubTUFOptions() *tuf.Options {
opts := DefaultOptionsWithCacheSetting()
func GitHubTUFOptions(tufMetadataDir o.Option[string]) *tuf.Options {
opts := DefaultOptionsWithCacheSetting(tufMetadataDir)
opts.Root = githubRoot
opts.RepositoryBaseURL = GitHubTUFMirror

10
pkg/cmd/attestation/verification/tuf_test.go
View file

@ -5,16 +5,22 @@ import (
"path/filepath"
"testing"
o "github.com/cli/cli/v2/pkg/option"
"github.com/cli/go-gh/v2/pkg/config"
"github.com/stretchr/testify/require"
)
func TestGitHubTUFOptions(t *testing.T) {
func TestGitHubTUFOptionsNoMetadataDir(t *testing.T) {
os.Setenv("CODESPACES", "true")
opts := GitHubTUFOptions()
opts := GitHubTUFOptions(o.None[string]())
require.Equal(t, GitHubTUFMirror, opts.RepositoryBaseURL)
require.NotNil(t, opts.Root)
require.True(t, opts.DisableLocalCache)
require.Equal(t, filepath.Join(config.CacheDir(), ".sigstore", "root"), opts.CachePath)
}
func TestGitHubTUFOptionsWithMetadataDir(t *testing.T) {
opts := GitHubTUFOptions(o.Some("anything"))
require.Equal(t, "anything", opts.CachePath)
}

4
pkg/cmd/attestation/verify/attestation_integration_test.go
View file

@ -10,6 +10,7 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/io"
"github.com/cli/cli/v2/pkg/cmd/attestation/test"
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
o "github.com/cli/cli/v2/pkg/option"
"github.com/sigstore/sigstore-go/pkg/fulcio/certificate"
"github.com/stretchr/testify/require"
)
@ -25,7 +26,8 @@ func getAttestationsFor(t *testing.T, bundlePath string) []*api.Attestation {
func TestVerifyAttestations(t *testing.T) {
sgVerifier := verification.NewLiveSigstoreVerifier(verification.SigstoreConfig{
Logger: io.NewTestHandler(),
Logger: io.NewTestHandler(),
TUFMetadataDir: o.Some(t.TempDir()),
})
certSummary := certificate.Summary{}

4
pkg/cmd/attestation/verify/verify.go
View file

@ -224,7 +224,7 @@ func runVerify(opts *Options) error {
attestations, logMsg, err := getAttestations(opts, *artifact)
if err != nil {
if ok := errors.Is(err, api.ErrNoAttestations{}); ok {
if ok := errors.Is(err, api.ErrNoAttestationsFound); ok {
opts.Logger.Printf(opts.Logger.ColorScheme.Red("✗ No attestations found for subject %s\n"), artifact.DigestWithAlg())
return err
}
@ -239,7 +239,7 @@ func runVerify(opts *Options) error {
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 err
return fmt.Errorf("no matching predicate found")
}
attestations = filteredAttestations

13
pkg/cmd/attestation/verify/verify_integration_test.go
View file

@ -11,6 +11,7 @@ import (
"github.com/cli/cli/v2/pkg/cmd/attestation/test"
"github.com/cli/cli/v2/pkg/cmd/attestation/verification"
"github.com/cli/cli/v2/pkg/cmd/factory"
o "github.com/cli/cli/v2/pkg/option"
"github.com/cli/go-gh/v2/pkg/auth"
"github.com/stretchr/testify/require"
)
@ -19,7 +20,8 @@ func TestVerifyIntegration(t *testing.T) {
logger := io.NewTestHandler()
sigstoreConfig := verification.SigstoreConfig{
Logger: logger,
Logger: logger,
TUFMetadataDir: o.Some(t.TempDir()),
}
cmdFactory := factory.New("test")
@ -130,7 +132,8 @@ func TestVerifyIntegrationCustomIssuer(t *testing.T) {
logger := io.NewTestHandler()
sigstoreConfig := verification.SigstoreConfig{
Logger: logger,
Logger: logger,
TUFMetadataDir: o.Some(t.TempDir()),
}
cmdFactory := factory.New("test")
@ -200,7 +203,8 @@ func TestVerifyIntegrationReusableWorkflow(t *testing.T) {
logger := io.NewTestHandler()
sigstoreConfig := verification.SigstoreConfig{
Logger: logger,
Logger: logger,
TUFMetadataDir: o.Some(t.TempDir()),
}
cmdFactory := factory.New("test")
@ -289,7 +293,8 @@ func TestVerifyIntegrationReusableWorkflowSignerWorkflow(t *testing.T) {
logger := io.NewTestHandler()
sigstoreConfig := verification.SigstoreConfig{
Logger: logger,
Logger: logger,
TUFMetadataDir: o.Some(t.TempDir()),
}
cmdFactory := factory.New("test")

12
pkg/cmd/attestation/verify/verify_test.go
View file

@ -501,6 +501,18 @@ func TestRunVerify(t *testing.T) {
require.Nil(t, runVerify(&customOpts))
})
t.Run("with valid OCI artifact with UseBundleFromRegistry flag and unknown predicate type", func(t *testing.T) {
customOpts := publicGoodOpts
customOpts.ArtifactPath = "oci://ghcr.io/github/test"
customOpts.BundlePath = ""
customOpts.UseBundleFromRegistry = true
customOpts.PredicateType = "https://predicate.type"
err := runVerify(&customOpts)
require.Error(t, err)
require.ErrorContains(t, err, "no matching predicate found")
})
t.Run("with valid OCI artifact with UseBundleFromRegistry flag but no bundle return from registry", func(t *testing.T) {
customOpts := publicGoodOpts
customOpts.ArtifactPath = "oci://ghcr.io/github/test"

4
pkg/cmd/auth/login/login.go
View file

@ -72,7 +72,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
The minimum required scopes for the token are: %[1]srepo%[1]s, %[1]sread:org%[1]s, and %[1]sgist%[1]s.
Take care when passing a fine-grained personal access token to %[1]s--with-token%[1]s
as the inherent scoping to certain resources may cause confusing behaviour when interacting with other
resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage.
resources. Favour setting %[1]sGH_TOKEN$%[1]s for fine-grained personal access token usage.
Alternatively, gh will use the authentication token found in environment variables.
This method is most suitable for "headless" use of gh such as in automation. See
@ -94,7 +94,7 @@ func NewCmdLogin(f *cmdutil.Factory, runF func(*LoginOptions) error) *cobra.Comm
# Start interactive setup
$ gh auth login
# Authenticate against github.com by reading the token from a file
# Authenticate against <github.com> by reading the token from a file
$ gh auth login --with-token < mytoken.txt
# Authenticate with specific host

2
pkg/cmd/browse/browse.go
View file

@ -69,7 +69,7 @@ func NewCmdBrowse(f *cmdutil.Factory, runF func(*BrowseOptions) error) *cobra.Co
- Repository home page
- Repository settings
`),
Use: "browse [<number> | <path> | <commit-SHA>]",
Use: "browse [<number> | <path> | <commit-sha>]",
Args: cobra.MaximumNArgs(1),
Example: heredoc.Doc(`
$ gh browse

28
pkg/cmd/cache/delete/delete.go vendored
View file

@ -22,8 +22,9 @@ type DeleteOptions struct {
HttpClient func() (*http.Client, error)
IO *iostreams.IOStreams
DeleteAll bool
Identifier string
DeleteAll bool
SucceedOnNoCaches bool
Identifier string
}
func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Command {
@ -33,7 +34,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
}
cmd := &cobra.Command{
Use: "delete [<cache-id>| <cache-key> | --all]",
Use: "delete [<cache-id> | <cache-key> | --all]",
Short: "Delete GitHub Actions caches",
Long: heredoc.Docf(`
Delete GitHub Actions caches.
@ -50,8 +51,11 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
# Delete a cache by id in a specific repo
$ gh cache delete 1234 --repo cli/cli
# Delete all caches
# Delete all caches (exit code 1 on no caches)
$ gh cache delete --all
# Delete all caches (exit code 0 on no caches)
$ gh cache delete --all --succeed-on-no-caches
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
@ -65,6 +69,10 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
return err
}
if !opts.DeleteAll && opts.SucceedOnNoCaches {
return cmdutil.FlagErrorf("--succeed-on-no-caches must be used in conjunction with --all")
}
if !opts.DeleteAll && len(args) == 0 {
return cmdutil.FlagErrorf("must provide either cache id, cache key, or use --all")
}
@ -82,6 +90,7 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
}
cmd.Flags().BoolVarP(&opts.DeleteAll, "all", "a", false, "Delete all caches")
cmd.Flags().BoolVar(&opts.SucceedOnNoCaches, "succeed-on-no-caches", false, "Return exit code 0 if no caches found. Must be used in conjunction with `--all`")
return cmd
}
@ -100,12 +109,21 @@ func deleteRun(opts *DeleteOptions) error {
var toDelete []string
if opts.DeleteAll {
opts.IO.StartProgressIndicator()
caches, err := shared.GetCaches(client, repo, shared.GetCachesOptions{Limit: -1})
opts.IO.StopProgressIndicator()
if err != nil {
return err
}
if len(caches.ActionsCaches) == 0 {
return fmt.Errorf("%s No caches to delete", opts.IO.ColorScheme().FailureIcon())
if opts.SucceedOnNoCaches {
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(opts.IO.Out, "%s No caches to delete\n", opts.IO.ColorScheme().SuccessIcon())
}
return nil
} else {
return fmt.Errorf("%s No caches to delete", opts.IO.ColorScheme().FailureIcon())
}
}
for _, cache := range caches.ActionsCaches {
toDelete = append(toDelete, strconv.Itoa(cache.Id))

77
pkg/cmd/cache/delete/delete_test.go vendored
View file

@ -43,6 +43,21 @@ func TestNewCmdDelete(t *testing.T) {
cli: "--all",
wants: DeleteOptions{DeleteAll: true},
},
{
name: "delete all and succeed-on-no-caches flags",
cli: "--all --succeed-on-no-caches",
wants: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true},
},
{
name: "succeed-on-no-caches flag",
cli: "--succeed-on-no-caches",
wantsErr: "--succeed-on-no-caches must be used in conjunction with --all",
},
{
name: "succeed-on-no-caches flag and id argument",
cli: "--succeed-on-no-caches 123",
wantsErr: "--succeed-on-no-caches must be used in conjunction with --all",
},
{
name: "id argument and delete all flag",
cli: "1 --all",
@ -72,6 +87,7 @@ func TestNewCmdDelete(t *testing.T) {
}
assert.NoError(t, err)
assert.Equal(t, tt.wants.DeleteAll, gotOpts.DeleteAll)
assert.Equal(t, tt.wants.SucceedOnNoCaches, gotOpts.SucceedOnNoCaches)
assert.Equal(t, tt.wants.Identifier, gotOpts.Identifier)
})
}
@ -160,6 +176,19 @@ func TestDeleteRun(t *testing.T) {
tty: true,
wantStdout: "✓ Deleted 2 caches from OWNER/REPO\n",
},
{
name: "attempts to delete all caches but api errors",
opts: DeleteOptions{DeleteAll: true},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"),
httpmock.StatusStringResponse(500, ""),
)
},
tty: true,
wantErr: true,
wantErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/actions/caches?per_page=100)",
},
{
name: "displays delete error",
opts: DeleteOptions{Identifier: "123"},
@ -186,6 +215,54 @@ func TestDeleteRun(t *testing.T) {
tty: true,
wantStdout: "✓ Deleted 1 cache from OWNER/REPO\n",
},
{
name: "no caches to delete when deleting all",
opts: DeleteOptions{DeleteAll: true},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"),
httpmock.JSONResponse(shared.CachePayload{
ActionsCaches: []shared.Cache{},
TotalCount: 0,
}),
)
},
tty: false,
wantErr: true,
wantErrMsg: "X No caches to delete",
},
{
name: "no caches to delete when deleting all but succeed on no cache tty",
opts: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"),
httpmock.JSONResponse(shared.CachePayload{
ActionsCaches: []shared.Cache{},
TotalCount: 0,
}),
)
},
tty: true,
wantErr: false,
wantStdout: "✓ No caches to delete\n",
},
{
name: "no caches to delete when deleting all but succeed on no cache non-tty",
opts: DeleteOptions{DeleteAll: true, SucceedOnNoCaches: true},
stubs: func(reg *httpmock.Registry) {
reg.Register(
httpmock.REST("GET", "repos/OWNER/REPO/actions/caches"),
httpmock.JSONResponse(shared.CachePayload{
ActionsCaches: []shared.Cache{},
TotalCount: 0,
}),
)
},
tty: false,
wantErr: false,
wantStdout: "",
},
}
for _, tt := range tests {

2
pkg/cmd/codespace/rebuild.go
View file

@ -23,7 +23,7 @@ func newRebuildCmd(app *App) *cobra.Command {
Short: "Rebuild a codespace",
Long: heredoc.Doc(`
Rebuilding recreates your codespace.
Your code and any current changes will be preserved. Your codespace will be rebuilt using
your working directory's dev container. A full rebuild also removes cached Docker images.
`),

6
pkg/cmd/codespace/ssh.go
View file

@ -55,7 +55,7 @@ func newSSHCmd(app *App) *cobra.Command {
Long: heredoc.Docf(`
The %[1]sssh%[1]s command is used to SSH into a codespace. In its simplest form, you can
run %[1]sgh cs ssh%[1]s, select a codespace interactively, and connect.
The %[1]sssh%[1]s command will automatically create a public/private ssh key pair in the
%[1]s~/.ssh%[1]s directory if you do not have an existing valid key pair. When selecting the
key pair to use, the preferred order is:
@ -732,8 +732,8 @@ func newCpCmd(app *App) *cobra.Command {
be evaluated on the remote machine, subject to expansion of tildes, braces, globs,
environment variables, and backticks. For security, do not use this flag with arguments
provided by untrusted users; see <https://lwn.net/Articles/835962/> for discussion.
By default, the %[1]scp%[1]s command will create a public/private ssh key pair to authenticate with
By default, the %[1]scp%[1]s command will create a public/private ssh key pair to authenticate with
the codespace inside the %[1]s~/.ssh directory%[1]s.
`, "`"),
Example: heredoc.Doc(`

2
pkg/cmd/codespace/view.go
View file

@ -30,7 +30,7 @@ func newViewCmd(app *App) *cobra.Command {
Short: "View details about a codespace",
Example: heredoc.Doc(`
# select a codespace from a list of all codespaces you own
$ gh cs view
$ gh cs view
# view the details of a specific codespace
$ gh cs view -c codespace-name-12345

6
pkg/cmd/completion/completion.go
View file

@ -33,7 +33,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command {
After, add this to your %[1]s~/.bash_profile%[1]s:
eval "$(gh completion -s bash)"
### zsh
Generate a %[1]s_gh%[1]s completion script and put it somewhere in your %[1]s$fpath%[1]s:
@ -44,7 +44,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command {
autoload -U compinit
compinit -i
Zsh version 5.7 or later is recommended.
### fish
@ -59,7 +59,7 @@ func NewCmdCompletion(io *iostreams.IOStreams) *cobra.Command {
mkdir -Path (Split-Path -Parent $profile) -ErrorAction SilentlyContinue
notepad $profile
Add the line and save the file:
Invoke-Expression -Command $(gh completion -s powershell | Out-String)

2
pkg/cmd/config/config.go
View file

@ -16,7 +16,7 @@ import (
func NewCmdConfig(f *cmdutil.Factory) *cobra.Command {
longDoc := strings.Builder{}
longDoc.WriteString("Display or change configuration settings for gh.\n\n")
longDoc.WriteString("Current respected settings:\n")
longDoc.WriteString("Current respected settings:\n\n")
for _, co := range config.Options {
longDoc.WriteString(fmt.Sprintf("- `%s`: %s", co.Key, co.Description))
if len(co.AllowedValues) > 0 {

13
pkg/cmd/extension/command.go
View file

@ -298,8 +298,17 @@ func NewCmdExtension(f *cmdutil.Factory) *cobra.Command {
Long: heredoc.Docf(`
Install a GitHub CLI extension from a GitHub or local repository.
For GitHub repositories, the repository argument can be specified in %[1]sOWNER/REPO%[1]s format or as a full repository URL.
The URL format is useful when the repository is not hosted on github.com.
For GitHub repositories, the repository argument can be specified in
%[1]sOWNER/REPO%[1]s format or as a full repository URL.
The URL format is useful when the repository is not hosted on <github.com>.
For remote repositories, the GitHub CLI first looks for the release artifacts assuming
that it's a binary extension i.e. prebuilt binaries provided as part of the release.
In the absence of a release, the repository itself is cloned assuming that it's a
script extension i.e. prebuilt executable or script exists on its root.
The %[1]s--pin%[1]s flag may be used to specify a tag or commit for binary and script
extensions respectively, the latest version is used otherwise.
For local repositories, often used while developing extensions, use %[1]s.%[1]s as the
value of the repository argument. Note the following:

2
pkg/cmd/extension/manager.go
View file

@ -268,7 +268,7 @@ func (m *Manager) Install(repo ghrepo.Interface, target string) error {
return err
}
if !hs {
return errors.New("extension is not installable: missing executable")
return fmt.Errorf("extension is not installable: no usable release artifact or script found in %s", repo)
}
return m.installGit(repo, target)

36
pkg/cmd/factory/remote_resolver.go
View file

@ -21,25 +21,24 @@ type remoteResolver struct {
readRemotes func() (git.RemoteSet, error)
getConfig func() (gh.Config, error)
urlTranslator context.Translator
cachedRemotes context.Remotes
remotesError error
}
func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
var cachedRemotes context.Remotes
var remotesError error
return func() (context.Remotes, error) {
if cachedRemotes != nil || remotesError != nil {
return cachedRemotes, remotesError
if rr.cachedRemotes != nil || rr.remotesError != nil {
return rr.cachedRemotes, rr.remotesError
}
gitRemotes, err := rr.readRemotes()
if err != nil {
remotesError = err
rr.remotesError = err
return nil, err
}
if len(gitRemotes) == 0 {
remotesError = errors.New("no git remotes found")
return nil, remotesError
rr.remotesError = errors.New("no git remotes found")
return nil, rr.remotesError
}
sshTranslate := rr.urlTranslator
@ -68,30 +67,31 @@ func (rr *remoteResolver) Resolver() func() (context.Remotes, error) {
// Sort remotes
sort.Sort(resolvedRemotes)
// Filter remotes by hosts
// Note that this is not caching correctly: https://github.com/cli/cli/issues/10103
cachedRemotes := resolvedRemotes.FilterByHosts(hosts)
rr.cachedRemotes = resolvedRemotes.FilterByHosts(hosts)
// Filter again by default host if one is set
// For config file default host fallback to cachedRemotes if none match
// For environment default host (GH_HOST) do not fallback to cachedRemotes if none match
if src != "default" {
filteredRemotes := cachedRemotes.FilterByHosts([]string{defaultHost})
filteredRemotes := rr.cachedRemotes.FilterByHosts([]string{defaultHost})
if isHostEnv(src) || len(filteredRemotes) > 0 {
cachedRemotes = filteredRemotes
rr.cachedRemotes = filteredRemotes
}
}
if len(cachedRemotes) == 0 {
if len(rr.cachedRemotes) == 0 {
if isHostEnv(src) {
return nil, fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable.", src)
rr.remotesError = fmt.Errorf("none of the git remotes configured for this repository correspond to the %s environment variable. Try adding a matching remote or unsetting the variable", src)
return nil, rr.remotesError
} else if cfg.Authentication().HasEnvToken() {
return nil, errors.New("set the GH_HOST environment variable to specify which GitHub host to use")
rr.remotesError = errors.New("set the GH_HOST environment variable to specify which GitHub host to use")
return nil, rr.remotesError
}
return nil, errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`")
rr.remotesError = errors.New("none of the git remotes configured for this repository point to a known GitHub host. To tell gh about a new GitHub host, please use `gh auth login`")
return nil, rr.remotesError
}
return cachedRemotes, nil
return rr.cachedRemotes, nil
}
}

95
pkg/cmd/factory/remote_resolver_test.go
View file

@ -1,14 +1,17 @@
package factory
import (
"errors"
"net/url"
"testing"
"github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
ghmock "github.com/cli/cli/v2/internal/gh/mock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
type identityTranslator struct{}
@ -288,3 +291,95 @@ func Test_remoteResolver(t *testing.T) {
})
}
}
func Test_remoteResolver_Caching(t *testing.T) {
t.Run("cache remotes", func(t *testing.T) {
var readRemotesCalled bool
rr := &remoteResolver{
readRemotes: func() (git.RemoteSet, error) {
if readRemotesCalled {
return git.RemoteSet{}, errors.New("readRemotes should only be called once")
}
readRemotesCalled = true
return git.RemoteSet{
git.NewRemote("origin", "https://github.com/owner/repo.git"),
}, nil
},
getConfig: func() (gh.Config, error) {
cfg := &ghmock.ConfigMock{}
cfg.AuthenticationFunc = func() gh.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"github.com"})
authCfg.SetDefaultHost("github.com", "default")
return authCfg
}
return cfg, nil
},
urlTranslator: identityTranslator{},
}
resolver := rr.Resolver()
expectedRemoteNames := []string{"origin"}
remotes, err := resolver()
require.NoError(t, err)
require.Equal(t, expectedRemoteNames, mapRemotesToNames(remotes))
require.Equal(t, readRemotesCalled, true)
cachedRemotes, err := resolver()
require.NoError(t, err, "expected no error to be cached")
require.Equal(t, expectedRemoteNames, mapRemotesToNames(cachedRemotes), "expected the remotes to be cached")
})
t.Run("cache error", func(t *testing.T) {
var readRemotesCalled bool
rr := &remoteResolver{
readRemotes: func() (git.RemoteSet, error) {
if readRemotesCalled {
return git.RemoteSet{
git.NewRemote("origin", "https://github.com/owner/repo.git"),
}, nil
}
readRemotesCalled = true
return git.RemoteSet{}, errors.New("error to be cached")
},
getConfig: func() (gh.Config, error) {
cfg := &ghmock.ConfigMock{}
cfg.AuthenticationFunc = func() gh.AuthConfig {
authCfg := &config.AuthConfig{}
authCfg.SetHosts([]string{"github.com"})
authCfg.SetDefaultHost("github.com", "default")
return authCfg
}
return cfg, nil
},
urlTranslator: identityTranslator{},
}
resolver := rr.Resolver()
expectedErr := errors.New("error to be cached")
remotes, err := resolver()
require.Equal(t, expectedErr, err)
require.Empty(t, remotes, "should return no remotes")
require.Equal(t, readRemotesCalled, true)
cachedRemotes, err := resolver()
require.Equal(t, expectedErr, err, "expected the error to be cached")
require.Empty(t, cachedRemotes, "should return no remotes")
})
}
func mapRemotesToNames(remotes context.Remotes) []string {
names := make([]string, len(remotes))
for i, r := range remotes {
names[i] = r.Name
}
return names
}

10
pkg/cmd/gist/list/list_test.go
View file

@ -540,14 +540,14 @@ func Test_listRun(t *testing.T) {
wantOut: heredoc.Doc(`
1234 main.txt
octo match in the description
2345 octo.txt
match in the file name
3456 main.txt
match in the file text
octo in the text
`),
},
{
@ -599,14 +599,14 @@ func Test_listRun(t *testing.T) {
wantOut: heredoc.Docf(`
%[1]s[0;34m1234%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m
%[1]s[0;30;43mocto%[1]s[0m%[1]s[0;1;39m match in the description%[1]s[0m
%[1]s[0;34m2345%[1]s[0m %[1]s[0;30;43mocto%[1]s[0m%[1]s[0;32m.txt%[1]s[0m
%[1]s[0;1;39mmatch in the file name%[1]s[0m
%[1]s[0;34m3456%[1]s[0m %[1]s[0;32mmain.txt%[1]s[0m
%[1]s[0;1;39mmatch in the file text%[1]s[0m
%[1]s[0;30;43mocto%[1]s[0m in the text
`, "\x1b"),
},
}

2
pkg/cmd/gist/rename/rename.go
View file

@ -35,7 +35,7 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co
}
cmd := &cobra.Command{
Use: "rename {<id> | <url>} <oldFilename> <newFilename>",
Use: "rename {<id> | <url>} <old-filename> <new-filename>",
Short: "Rename a file in a gist",
Long: heredoc.Doc(`Rename a file in the given gist ID / URL.`),
Args: cobra.ExactArgs(3),

14
pkg/cmd/issue/comment/comment.go
View file

@ -11,12 +11,13 @@ import (
func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) error) *cobra.Command {
opts := &prShared.CommentableOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams),
InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter),
OpenInBrowser: f.Browser.Browse,
IO: f.IOStreams,
HttpClient: f.HttpClient,
EditSurvey: prShared.CommentableEditSurvey(f.Config, f.IOStreams),
InteractiveEditSurvey: prShared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: prShared.CommentableConfirmSubmitSurvey(f.Prompter),
ConfirmCreateIfNoneSurvey: prShared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter),
OpenInBrowser: f.Browser.Browse,
}
var bodyFile string
@ -69,6 +70,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*prShared.CommentableOptions) e
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.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last")
return cmd
}

127
pkg/cmd/issue/comment/comment_test.go
View file

@ -109,6 +109,29 @@ func TestNewCmdComment(t *testing.T) {
},
wantsErr: false,
},
{
name: "edit last flag",
input: "1 --edit-last",
output: shared.CommentableOptions{
Interactive: true,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
},
wantsErr: false,
},
{
name: "edit last flag with create if none",
input: "1 --edit-last --create-if-none",
output: shared.CommentableOptions{
Interactive: true,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
CreateIfNone: true,
},
wantsErr: false,
},
{
name: "body and body-file flags",
input: "1 --body 'test' --body-file 'test-file.txt'",
@ -139,6 +162,12 @@ func TestNewCmdComment(t *testing.T) {
output: shared.CommentableOptions{},
wantsErr: true,
},
{
name: "create-if-none flag without edit-last",
input: "1 --create-if-none",
output: shared.CommentableOptions{},
wantsErr: true,
},
}
for _, tt := range tests {
@ -188,11 +217,12 @@ func TestNewCmdComment(t *testing.T) {
func Test_commentRun(t *testing.T) {
tests := []struct {
name string
input *shared.CommentableOptions
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
name string
input *shared.CommentableOptions
emptyComments bool
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
}{
{
name: "interactive editor",
@ -225,6 +255,24 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n",
},
{
name: "interactive editor with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: true,
InputType: 0,
Body: "",
EditLast: true,
CreateIfNone: true,
InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil },
ConfirmCreateIfNoneSurvey: func() (bool, error) { return true, nil },
ConfirmSubmitSurvey: func() (bool, error) { return true, nil },
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentUpdate(t, reg)
},
stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n",
},
{
name: "non-interactive web",
input: &shared.CommentableOptions{
@ -248,6 +296,39 @@ func Test_commentRun(t *testing.T) {
},
stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n",
},
{
name: "non-interactive web with edit last and create if none for empty comments",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeWeb,
Body: "",
EditLast: true,
CreateIfNone: true,
OpenInBrowser: func(u string) error {
assert.Contains(t, u, "#issuecomment-new")
return nil
},
},
emptyComments: true,
stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n",
},
{
name: "non-interactive web with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeWeb,
Body: "",
EditLast: true,
CreateIfNone: true,
OpenInBrowser: func(u string) error {
assert.Contains(t, u, "#issuecomment-111")
return nil
},
},
stderr: "Opening https://github.com/OWNER/REPO/issues/123 in your browser.\n",
},
{
name: "non-interactive editor",
input: &shared.CommentableOptions{
@ -277,6 +358,23 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-111\n",
},
{
name: "non-interactive editor with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
CreateIfNone: true,
EditSurvey: func(string) (string, error) { return "comment body", nil },
},
emptyComments: true,
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentCreate(t, reg)
},
stdout: "https://github.com/OWNER/REPO/issues/123#issuecomment-456\n",
},
{
name: "non-interactive inline",
input: &shared.CommentableOptions{
@ -319,14 +417,21 @@ func Test_commentRun(t *testing.T) {
tt.input.HttpClient = func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
}
comments := api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true},
{ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"},
}}
if tt.emptyComments {
comments.Nodes = []api.Comment{}
}
tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) {
return &api.Issue{
ID: "ISSUE-ID",
URL: "https://github.com/OWNER/REPO/issues/123",
Comments: api.Comments{Nodes: []api.Comment{
{ID: "id1", Author: api.CommentAuthor{Login: "octocat"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-111", ViewerDidAuthor: true},
{ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/issues/123#issuecomment-222"},
}},
ID: "ISSUE-ID",
URL: "https://github.com/OWNER/REPO/issues/123",
Comments: comments,
}, ghrepo.New("OWNER", "REPO"), nil
}

3
pkg/cmd/issue/list/list.go
View file

@ -59,7 +59,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Use: "list",
Short: "List issues in a repository",
Long: heredoc.Doc(`
List issues in a GitHub repository.
List issues in a GitHub repository. By default, this only lists open issues.
The search query syntax is documented here:
<https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests>
@ -70,6 +70,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
$ gh issue list --assignee "@me"
$ gh issue list --milestone "The big 1.0"
$ gh issue list --search "error no:assignee sort:created-asc"
$ gh issue list --state all
`),
Aliases: []string{"ls"},
Args: cmdutil.NoArgsQuoteReminder,

2
pkg/cmd/label/list_test.go
View file

@ -218,7 +218,7 @@ func TestListRun(t *testing.T) {
wantStdout: heredoc.Doc(`
Showing 2 of 2 labels in OWNER/REPO
NAME DESCRIPTION COLOR
bug This is a bug label #d73a4a
docs This is a docs label #ffa8da

126
pkg/cmd/pr/checkout/checkout.go
View file

@ -6,11 +6,13 @@ import (
"net/http"
"strings"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
cliContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
@ -25,8 +27,12 @@ type CheckoutOptions struct {
Remotes func() (cliContext.Remotes, error)
Branch func() (string, error)
Finder shared.PRFinder
Finder shared.PRFinder
Prompter shared.Prompt
Lister shared.PRLister
Interactive bool
BaseRepo func() (ghrepo.Interface, error)
SelectorArg string
RecurseSubmodules bool
Force bool
@ -42,17 +48,33 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr
Config: f.Config,
Remotes: f.Remotes,
Branch: f.Branch,
Prompter: f.Prompter,
BaseRepo: f.BaseRepo,
}
cmd := &cobra.Command{
Use: "checkout {<number> | <url> | <branch>}",
Use: "checkout [<number> | <url> | <branch>]",
Short: "Check out a pull request in git",
Args: cmdutil.ExactArgs(1, "argument required"),
Example: heredoc.Doc(`
# Interactively select a PR from the 10 most recent to check out
$ gh pr checkout
# Checkout a specific PR
$ gh pr checkout 32
$ gh pr checkout https://github.com/OWNER/REPO/pull/32
$ gh pr checkout feature
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.Finder = shared.NewFinder(f)
opts.Lister = shared.NewLister(f)
if len(args) > 0 {
opts.SelectorArg = args[0]
} else if !opts.IO.CanPrompt() {
return cmdutil.FlagErrorf("pull request number, URL, or branch required when not running interactively")
} else {
opts.Interactive = true
}
if runF != nil {
@ -71,11 +93,12 @@ func NewCmdCheckout(f *cmdutil.Factory, runF func(*CheckoutOptions) error) *cobr
}
func checkoutRun(opts *CheckoutOptions) error {
findOptions := shared.FindOptions{
Selector: opts.SelectorArg,
Fields: []string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"},
baseRepo, err := opts.BaseRepo()
if err != nil {
return err
}
pr, baseRepo, err := opts.Finder.Find(findOptions)
pr, err := resolvePR(baseRepo, opts.Prompter, opts.SelectorArg, opts.Interactive, opts.Finder, opts.Lister, opts.IO)
if err != nil {
return err
}
@ -149,7 +172,7 @@ func cmdsForExistingRemote(remote *cliContext.Remote, pr *api.PullRequest, opts
refSpec += fmt.Sprintf(":refs/remotes/%s", remoteBranch)
}
cmds = append(cmds, []string{"fetch", remote.Name, refSpec})
cmds = append(cmds, []string{"fetch", remote.Name, refSpec, "--no-tags"})
localBranch := pr.HeadRefName
if opts.BranchName != "" {
@ -179,7 +202,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
ref := fmt.Sprintf("refs/pull/%d/head", pr.Number)
if opts.Detach {
cmds = append(cmds, []string{"fetch", baseURLOrName, ref})
cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"})
cmds = append(cmds, []string{"checkout", "--detach", "FETCH_HEAD"})
return cmds
}
@ -195,7 +218,7 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
currentBranch, _ := opts.Branch()
if localBranch == currentBranch {
// PR head matches currently checked out branch
cmds = append(cmds, []string{"fetch", baseURLOrName, ref})
cmds = append(cmds, []string{"fetch", baseURLOrName, ref, "--no-tags"})
if opts.Force {
cmds = append(cmds, []string{"reset", "--hard", "FETCH_HEAD"})
} else {
@ -203,13 +226,12 @@ func cmdsForMissingRemote(pr *api.PullRequest, baseURLOrName, repoHost, defaultB
cmds = append(cmds, []string{"merge", "--ff-only", "FETCH_HEAD"})
}
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
fetchCmd := []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--no-tags"}
if opts.Force {
cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch), "--force"})
} else {
// TODO: check if non-fast-forward and suggest to use `--force`
cmds = append(cmds, []string{"fetch", baseURLOrName, fmt.Sprintf("%s:%s", ref, localBranch)})
fetchCmd = append(fetchCmd, "--force")
}
cmds = append(cmds, fetchCmd)
cmds = append(cmds, []string{"checkout", localBranch})
}
@ -263,3 +285,77 @@ func executeCmds(client *git.Client, credentialPattern git.CredentialPattern, cm
}
return nil
}
func resolvePR(baseRepo ghrepo.Interface, prompter shared.Prompt, pullRequestSelector string, isInteractive bool, pullRequestFinder shared.PRFinder, prLister shared.PRLister, io *iostreams.IOStreams) (*api.PullRequest, error) {
// When non-interactive
if pullRequestSelector != "" {
pr, _, err := pullRequestFinder.Find(shared.FindOptions{
Selector: pullRequestSelector,
Fields: []string{
"number",
"headRefName",
"headRepository",
"headRepositoryOwner",
"isCrossRepository",
"maintainerCanModify",
},
})
if err != nil {
return nil, err
}
return pr, nil
}
if !isInteractive {
return nil, cmdutil.FlagErrorf("pull request number, URL, or branch required when not running interactively")
}
// When interactive
io.StartProgressIndicator()
listResult, err := prLister.List(shared.ListOptions{
State: "open",
Fields: []string{
"number",
"title",
"state",
"isDraft",
"headRefName",
"headRepository",
"headRepositoryOwner",
"isCrossRepository",
"maintainerCanModify",
},
LimitResults: 10})
io.StopProgressIndicator()
if err != nil {
return nil, err
}
if len(listResult.PullRequests) == 0 {
return nil, shared.ListNoResults(ghrepo.FullName(baseRepo), "pull request", false)
}
pr, err := promptForPR(prompter, *listResult)
return pr, err
}
func promptForPR(prompter shared.Prompt, jobs api.PullRequestAndTotalCount) (*api.PullRequest, error) {
candidates := []string{}
for _, pr := range jobs.PullRequests {
candidates = append(candidates, fmt.Sprintf("%d\t%s %s [%s]",
pr.Number,
shared.PrStateWithDraft(&pr),
text.RemoveExcessiveWhitespace(pr.Title),
pr.HeadLabel(),
))
}
selected, err := prompter.Select("Select a pull request", "", candidates)
if err != nil {
return nil, err
}
if selected >= 0 {
return &jobs.PullRequests[selected], nil
}
return nil, nil
}

207
pkg/cmd/pr/checkout/checkout_test.go
View file

@ -14,6 +14,7 @@ import (
"github.com/cli/cli/v2/internal/config"
"github.com/cli/cli/v2/internal/gh"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/internal/run"
"github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
@ -27,6 +28,10 @@ import (
// repo: either "baseOwner/baseRepo" or "baseOwner/baseRepo:defaultBranch"
// prHead: "headOwner/headRepo:headBranch"
func stubPR(repo, prHead string) (ghrepo.Interface, *api.PullRequest) {
return _stubPR(repo, prHead, 123, "PR title", "OPEN", false)
}
func _stubPR(repo, prHead string, number int, title string, state string, isDraft bool) (ghrepo.Interface, *api.PullRequest) {
defaultBranch := ""
if idx := strings.IndexRune(repo, ':'); idx >= 0 {
defaultBranch = repo[idx+1:]
@ -52,25 +57,33 @@ func stubPR(repo, prHead string) (ghrepo.Interface, *api.PullRequest) {
}
return baseRepo, &api.PullRequest{
Number: 123,
Number: number,
HeadRefName: headRefName,
HeadRepositoryOwner: api.Owner{Login: headRepo.RepoOwner()},
HeadRepository: &api.PRRepository{Name: headRepo.RepoName()},
IsCrossRepository: !ghrepo.IsSame(baseRepo, headRepo),
MaintainerCanModify: false,
Title: title,
State: state,
IsDraft: isDraft,
}
}
func Test_checkoutRun(t *testing.T) {
tests := []struct {
name string
opts *CheckoutOptions
httpStubs func(*httpmock.Registry)
runStubs func(*run.CommandStubber)
name string
opts *CheckoutOptions
httpStubs func(*httpmock.Registry)
runStubs func(*run.CommandStubber)
promptStubs func(*prompter.MockPrompter)
remotes map[string]string
wantStdout string
wantStderr string
wantErr bool
errMsg string
}{
{
name: "checkout with ssh remote URL",
@ -81,6 +94,10 @@ func Test_checkoutRun(t *testing.T) {
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
BaseRepo: func() (ghrepo.Interface, error) {
baseRepo, _ := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
return baseRepo, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
@ -93,7 +110,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
},
},
@ -108,6 +125,10 @@ func Test_checkoutRun(t *testing.T) {
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
BaseRepo: func() (ghrepo.Interface, error) {
baseRepo, _ := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
return baseRepo, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
@ -119,7 +140,7 @@ func Test_checkoutRun(t *testing.T) {
"origin": "OWNER/REPO",
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
@ -137,6 +158,10 @@ func Test_checkoutRun(t *testing.T) {
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
BaseRepo: func() (ghrepo.Interface, error) {
baseRepo, _ := stubPR("OWNER/REPO:master", "OWNER/REPO:feature")
return baseRepo, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
@ -149,7 +174,7 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/foobar`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b foobar --track origin/feature`, 0, "")
},
},
@ -164,6 +189,10 @@ func Test_checkoutRun(t *testing.T) {
finder := shared.NewMockFinder("123", pr, baseRepo)
return finder
}(),
BaseRepo: func() (ghrepo.Interface, error) {
baseRepo, _ := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
return baseRepo, nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
@ -176,19 +205,94 @@ func Test_checkoutRun(t *testing.T) {
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git config branch\.foobar\.merge`, 1, "")
cs.Register(`git fetch origin refs/pull/123/head:foobar`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:foobar --no-tags`, 0, "")
cs.Register(`git checkout foobar`, 0, "")
cs.Register(`git config branch\.foobar\.remote https://github.com/hubot/REPO.git`, 0, "")
cs.Register(`git config branch\.foobar\.pushRemote https://github.com/hubot/REPO.git`, 0, "")
cs.Register(`git config branch\.foobar\.merge refs/heads/feature`, 0, "")
},
},
{
name: "with no selected PR args and non tty, return error",
opts: &CheckoutOptions{
SelectorArg: "",
Interactive: false,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
},
remotes: map[string]string{
"origin": "OWNER/REPO",
},
wantErr: true,
errMsg: "pull request number, URL, or branch required when not running interactively",
},
{
name: "with no selected PR args and stdin tty, prompts for choice",
opts: &CheckoutOptions{
SelectorArg: "",
Interactive: true,
Lister: func() shared.PRLister {
_, pr1 := _stubPR("OWNER/REPO:master", "OWNER/REPO:feature", 32, "New feature", "OPEN", false)
_, pr2 := _stubPR("OWNER/REPO:master", "OWNER/REPO:bug-fix", 29, "Fixed bad bug", "OPEN", false)
_, pr3 := _stubPR("OWNER/REPO:master", "OWNER/REPO:docs", 28, "Improve documentation", "OPEN", true)
lister := shared.NewMockLister(&api.PullRequestAndTotalCount{
TotalCount: 3,
PullRequests: []api.PullRequest{
*pr1, *pr2, *pr3,
}, SearchCapped: false}, nil)
lister.ExpectFields([]string{"number", "title", "state", "isDraft", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
return lister
}(),
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Config: func() (gh.Config, error) {
return config.NewBlankConfig(), nil
},
},
promptStubs: func(pm *prompter.MockPrompter) {
pm.RegisterSelect("Select a pull request",
[]string{"32\tOPEN New feature [feature]", "29\tOPEN Fixed bad bug [bug-fix]", "28\tDRAFT Improve documentation [docs]"},
func(_, _ string, opts []string) (int, error) {
return prompter.IndexFor(opts, "32\tOPEN New feature [feature]")
})
},
runStubs: func(cs *run.CommandStubber) {
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
},
remotes: map[string]string{
"origin": "OWNER/REPO",
},
},
{
name: "with no select PR args and no open PR, return error",
opts: &CheckoutOptions{
SelectorArg: "",
Interactive: true,
BaseRepo: func() (ghrepo.Interface, error) {
return ghrepo.New("OWNER", "REPO"), nil
},
Lister: shared.NewMockLister(&api.PullRequestAndTotalCount{
TotalCount: 0,
PullRequests: []api.PullRequest{},
}, nil),
},
remotes: map[string]string{
"origin": "OWNER/REPO",
},
wantErr: true,
errMsg: "no open pull requests in OWNER/REPO",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
opts := tt.opts
ios, _, stdout, stderr := iostreams.Test()
opts.IO = ios
httpReg := &httpmock.Registry{}
defer httpReg.Verify(t)
@ -205,6 +309,12 @@ func Test_checkoutRun(t *testing.T) {
tt.runStubs(cmdStubs)
}
pm := prompter.NewMockPrompter(t)
tt.opts.Prompter = pm
if tt.promptStubs != nil {
tt.promptStubs(pm)
}
opts.Remotes = func() (context.Remotes, error) {
if len(tt.remotes) == 0 {
return nil, errors.New("no remotes")
@ -232,6 +342,9 @@ func Test_checkoutRun(t *testing.T) {
if (err != nil) != tt.wantErr {
t.Errorf("want error: %v, got: %v", tt.wantErr, err)
}
if err != nil {
assert.Equal(t, tt.errMsg, err.Error())
}
assert.Equal(t, tt.wantStdout, stdout.String())
assert.Equal(t, tt.wantStderr, stderr.String())
})
@ -240,7 +353,7 @@ func Test_checkoutRun(t *testing.T) {
/** LEGACY TESTS **/
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string) (*test.CmdOut, error) {
func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cli string, baseRepo ghrepo.Interface) (*test.CmdOut, error) {
ios, _, stdout, stderr := iostreams.Test()
factory := &cmdutil.Factory{
@ -269,6 +382,9 @@ func runCommand(rt http.RoundTripper, remotes context.Remotes, branch string, cl
GhPath: "some/path/gh",
GitPath: "some/path/git",
},
BaseRepo: func() (ghrepo.Interface, error) {
return baseRepo, nil
},
}
cmd := NewCmdCheckout(factory, nil)
@ -301,11 +417,11 @@ func TestPRCheckout_sameRepo(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track origin/feature`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -320,12 +436,12 @@ func TestPRCheckout_existingBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -352,11 +468,11 @@ func TestPRCheckout_differentRepo_remoteExists(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature`, 0, "")
cs.Register(`git fetch robot-fork \+refs/heads/feature:refs/remotes/robot-fork/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 1, "")
cs.Register(`git checkout -b feature --track robot-fork/feature`, 0, "")
output, err := runCommand(http, remotes, "master", `123`)
output, err := runCommand(http, remotes, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -372,14 +488,37 @@ func TestPRCheckout_differentRepo(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.pushRemote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
}
func TestPRCheckout_differentRepoForce(t *testing.T) {
http := &httpmock.Registry{}
defer http.Verify(t)
baseRepo, pr := stubPR("OWNER/REPO:master", "hubot/REPO:feature")
finder := shared.RunCommandFinder("123", pr, baseRepo)
finder.ExpectFields([]string{"number", "headRefName", "headRepository", "headRepositoryOwner", "isCrossRepository", "maintainerCanModify"})
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags --force`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote origin`, 0, "")
cs.Register(`git config branch\.feature\.pushRemote origin`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/pull/123/head`, 0, "")
output, err := runCommand(http, nil, "master", `123 --force`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -394,11 +533,11 @@ func TestPRCheckout_differentRepo_existingBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -413,11 +552,11 @@ func TestPRCheckout_detachedHead(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git checkout feature`, 0, "")
output, err := runCommand(http, nil, "", `123`)
output, err := runCommand(http, nil, "", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -432,11 +571,11 @@ func TestPRCheckout_differentRepo_currentBranch(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 0, "refs/heads/feature\n")
cs.Register(`git merge --ff-only FETCH_HEAD`, 0, "")
output, err := runCommand(http, nil, "feature", `123`)
output, err := runCommand(http, nil, "feature", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -451,7 +590,9 @@ func TestPRCheckout_differentRepo_invalidBranchName(t *testing.T) {
_, cmdTeardown := run.Stub()
defer cmdTeardown(t)
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.EqualError(t, err, `invalid branch name: "-foo"`)
assert.Equal(t, "", output.Stderr())
assert.Equal(t, "", output.Stderr())
@ -467,14 +608,14 @@ func TestPRCheckout_maintainerCanModify(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin refs/pull/123/head:feature`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head:feature --no-tags`, 0, "")
cs.Register(`git config branch\.feature\.merge`, 1, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git config branch\.feature\.remote https://github\.com/hubot/REPO\.git`, 0, "")
cs.Register(`git config branch\.feature\.pushRemote https://github\.com/hubot/REPO\.git`, 0, "")
cs.Register(`git config branch\.feature\.merge refs/heads/feature`, 0, "")
output, err := runCommand(http, nil, "master", `123`)
output, err := runCommand(http, nil, "master", `123`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -488,14 +629,14 @@ func TestPRCheckout_recurseSubmodules(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git merge --ff-only refs/remotes/origin/feature`, 0, "")
cs.Register(`git submodule sync --recursive`, 0, "")
cs.Register(`git submodule update --init --recursive`, 0, "")
output, err := runCommand(http, nil, "master", `123 --recurse-submodules`)
output, err := runCommand(http, nil, "master", `123 --recurse-submodules`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())
@ -509,12 +650,12 @@ func TestPRCheckout_force(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature`, 0, "")
cs.Register(`git fetch origin \+refs/heads/feature:refs/remotes/origin/feature --no-tags`, 0, "")
cs.Register(`git show-ref --verify -- refs/heads/feature`, 0, "")
cs.Register(`git checkout feature`, 0, "")
cs.Register(`git reset --hard refs/remotes/origin/feature`, 0, "")
output, err := runCommand(http, nil, "master", `123 --force`)
output, err := runCommand(http, nil, "master", `123 --force`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
@ -531,9 +672,9 @@ func TestPRCheckout_detach(t *testing.T) {
cs, cmdTeardown := run.Stub()
defer cmdTeardown(t)
cs.Register(`git checkout --detach FETCH_HEAD`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head`, 0, "")
cs.Register(`git fetch origin refs/pull/123/head --no-tags`, 0, "")
output, err := runCommand(http, nil, "", `123 --detach`)
output, err := runCommand(http, nil, "", `123 --detach`, baseRepo)
assert.NoError(t, err)
assert.Equal(t, "", output.String())
assert.Equal(t, "", output.Stderr())

2
pkg/cmd/pr/close/close.go
View file

@ -143,7 +143,7 @@ func closeRun(opts *CloseOptions) error {
branchSwitchString = fmt.Sprintf(" and switched to branch %s", cs.Cyan(branchToSwitchTo))
}
} else {
fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the local branch since current directory is not a git repository \n", cs.WarningIcon())
fmt.Fprintf(opts.IO.ErrOut, "%s Skipped deleting the local branch since current directory is not a git repository\n", cs.WarningIcon())
}
}

2
pkg/cmd/pr/close/close_test.go
View file

@ -271,7 +271,7 @@ func TestPrClose_deleteBranch_notInGitRepo(t *testing.T) {
assert.Equal(t, "", output.String())
assert.Equal(t, heredoc.Doc(`
✓ Closed pull request OWNER/REPO#96 (The title of the PR)
! Skipped deleting the local branch since current directory is not a git repository
! Skipped deleting the local branch since current directory is not a git repository
✓ Deleted branch trunk
`), output.Stderr())
}

14
pkg/cmd/pr/comment/comment.go
View file

@ -10,12 +10,13 @@ import (
func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) error) *cobra.Command {
opts := &shared.CommentableOptions{
IO: f.IOStreams,
HttpClient: f.HttpClient,
EditSurvey: shared.CommentableEditSurvey(f.Config, f.IOStreams),
InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter),
OpenInBrowser: f.Browser.Browse,
IO: f.IOStreams,
HttpClient: f.HttpClient,
EditSurvey: shared.CommentableEditSurvey(f.Config, f.IOStreams),
InteractiveEditSurvey: shared.CommentableInteractiveEditSurvey(f.Config, f.IOStreams),
ConfirmSubmitSurvey: shared.CommentableConfirmSubmitSurvey(f.Prompter),
ConfirmCreateIfNoneSurvey: shared.CommentableInteractiveCreateIfNoneSurvey(f.Prompter),
OpenInBrowser: f.Browser.Browse,
}
var bodyFile string
@ -75,6 +76,7 @@ func NewCmdComment(f *cmdutil.Factory, runF func(*shared.CommentableOptions) err
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.CreateIfNone, "create-if-none", false, "Create a new comment if no comments are found. Can be used only with --edit-last")
return cmd
}

131
pkg/cmd/pr/comment/comment_test.go
View file

@ -129,6 +129,29 @@ func TestNewCmdComment(t *testing.T) {
},
wantsErr: false,
},
{
name: "edit last flag",
input: "1 --edit-last",
output: shared.CommentableOptions{
Interactive: true,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
},
wantsErr: false,
},
{
name: "edit last flag with create if none",
input: "1 --edit-last --create-if-none",
output: shared.CommentableOptions{
Interactive: true,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
CreateIfNone: true,
},
wantsErr: false,
},
{
name: "body and body-file flags",
input: "1 --body 'test' --body-file 'test-file.txt'",
@ -159,6 +182,12 @@ func TestNewCmdComment(t *testing.T) {
output: shared.CommentableOptions{},
wantsErr: true,
},
{
name: "create-if-none flag without edit-last",
input: "1 --create-if-none",
output: shared.CommentableOptions{},
wantsErr: true,
},
}
for _, tt := range tests {
@ -208,11 +237,12 @@ func TestNewCmdComment(t *testing.T) {
func Test_commentRun(t *testing.T) {
tests := []struct {
name string
input *shared.CommentableOptions
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
name string
input *shared.CommentableOptions
emptyComments bool
httpStubs func(*testing.T, *httpmock.Registry)
stdout string
stderr string
}{
{
name: "interactive editor",
@ -245,6 +275,24 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n",
},
{
name: "interactive editor with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: true,
InputType: 0,
Body: "",
EditLast: true,
CreateIfNone: true,
InteractiveEditSurvey: func(string) (string, error) { return "comment body", nil },
ConfirmCreateIfNoneSurvey: func() (bool, error) { return true, nil },
ConfirmSubmitSurvey: func() (bool, error) { return true, nil },
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentUpdate(t, reg)
},
stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n",
},
{
name: "non-interactive web",
input: &shared.CommentableOptions{
@ -264,7 +312,43 @@ func Test_commentRun(t *testing.T) {
Body: "",
EditLast: true,
OpenInBrowser: func(string) error { return nil },
OpenInBrowser: func(u string) error {
assert.Contains(t, u, "#issuecomment-111")
return nil
},
},
stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n",
},
{
name: "non-interactive web with edit last and create if none for empty comments",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeWeb,
Body: "",
EditLast: true,
CreateIfNone: true,
OpenInBrowser: func(u string) error {
assert.Contains(t, u, "#issuecomment-new")
return nil
},
},
emptyComments: true,
stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n",
},
{
name: "non-interactive web with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeWeb,
Body: "",
EditLast: true,
CreateIfNone: true,
OpenInBrowser: func(u string) error {
assert.Contains(t, u, "#issuecomment-111")
return nil
},
},
stderr: "Opening https://github.com/OWNER/REPO/pull/123 in your browser.\n",
},
@ -297,6 +381,23 @@ func Test_commentRun(t *testing.T) {
},
stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-111\n",
},
{
name: "non-interactive editor with edit last and create if none",
input: &shared.CommentableOptions{
Interactive: false,
InputType: shared.InputTypeEditor,
Body: "",
EditLast: true,
CreateIfNone: true,
EditSurvey: func(string) (string, error) { return "comment body", nil },
},
emptyComments: true,
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
mockCommentCreate(t, reg)
},
stdout: "https://github.com/OWNER/REPO/pull/123#issuecomment-456\n",
},
{
name: "non-interactive inline",
input: &shared.CommentableOptions{
@ -339,14 +440,20 @@ func Test_commentRun(t *testing.T) {
tt.input.IO = ios
tt.input.HttpClient = httpClient
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},
{ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"},
}}
if tt.emptyComments {
comments.Nodes = []api.Comment{}
}
tt.input.RetrieveCommentable = func() (shared.Commentable, ghrepo.Interface, error) {
return &api.PullRequest{
Number: 123,
URL: "https://github.com/OWNER/REPO/pull/123",
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},
{ID: "id2", Author: api.CommentAuthor{Login: "monalisa"}, URL: "https://github.com/OWNER/REPO/pull/123#issuecomment-222"},
}},
Number: 123,
URL: "https://github.com/OWNER/REPO/pull/123",
Comments: comments,
}, ghrepo.New("OWNER", "REPO"), nil
}

27
pkg/cmd/pr/create/create.go
View file

@ -518,6 +518,7 @@ func initDefaultTitleBody(ctx CreateContext, state *shared.IssueMetadataState, u
return nil
}
// TODO: Replace with the finder's PullRequestRefs struct
// trackingRef represents a ref for a remote tracking branch.
type trackingRef struct {
remoteName string
@ -685,7 +686,9 @@ func NewCreateContext(opts *CreateOptions) (*CreateContext, error) {
return nil, err
}
if isPushEnabled {
// determine whether the head branch is already pushed to a remote
// TODO: This doesn't respect the @{push} revision resolution or triagular workflows assembled with
// remote.pushDefault, or branch.<branchName>.pushremote config settings. The finder's ParsePRRefs
// may be able to replace this function entirely.
if trackingRef, found := tryDetermineTrackingRef(gitClient, remotes, headBranch, headBranchConfig); found {
isPushEnabled = false
if r, err := remotes.FindByName(trackingRef.remoteName); err == nil {
@ -932,6 +935,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
headRepo := ctx.HeadRepo
headRemote := ctx.HeadRemote
client := ctx.Client
gitClient := ctx.GitClient
var err error
// if a head repository could not be determined so far, automatically create
@ -973,7 +977,8 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
headRepoURL := ghrepo.FormatRemoteURL(headRepo, cloneProtocol)
gitClient := ctx.GitClient
origin, _ := remotes.FindByName("origin")
upstream, _ := remotes.FindByName("upstream")
upstreamName := "upstream"
upstream, _ := remotes.FindByName(upstreamName)
remoteName := "origin"
if origin != nil {
@ -981,7 +986,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
}
if origin != nil && upstream == nil && ghrepo.IsSame(origin, ctx.BaseRepo) {
renameCmd, err := gitClient.Command(context.Background(), "remote", "rename", "origin", "upstream")
renameCmd, err := gitClient.Command(context.Background(), "remote", "rename", "origin", upstreamName)
if err != nil {
return err
}
@ -989,7 +994,7 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
return fmt.Errorf("error renaming origin remote: %w", err)
}
remoteName = "origin"
fmt.Fprintf(opts.IO.ErrOut, "Changed %s remote to %q\n", ghrepo.FullName(ctx.BaseRepo), "upstream")
fmt.Fprintf(opts.IO.ErrOut, "Changed %s remote to %q\n", ghrepo.FullName(ctx.BaseRepo), upstreamName)
}
gitRemote, err := gitClient.AddRemote(context.Background(), remoteName, headRepoURL, []string{})
@ -999,6 +1004,19 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
fmt.Fprintf(opts.IO.ErrOut, "Added %s as remote %q\n", ghrepo.FullName(headRepo), remoteName)
// Only mark `upstream` remote as default if `gh pr create` created the remote.
if didForkRepo {
err := gitClient.SetRemoteResolution(context.Background(), upstreamName, "base")
if err != nil {
return fmt.Errorf("error setting upstream as default: %w", err)
}
if opts.IO.IsStdoutTTY() {
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.ErrOut, "%s Repository %s set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n", cs.WarningIcon(), cs.Bold(ghrepo.FullName(headRepo)))
}
}
headRemote = &ghContext.Remote{
Remote: gitRemote,
Repo: headRepo,
@ -1010,7 +1028,6 @@ func handlePush(opts CreateOptions, ctx CreateContext) error {
pushBranch := func() error {
w := NewRegexpWriter(opts.IO.ErrOut, gitPushRegexp, "")
defer w.Flush()
gitClient := ctx.GitClient
ref := fmt.Sprintf("HEAD:refs/heads/%s", ctx.HeadBranch)
bo := backoff.NewConstantBackOff(2 * time.Second)
ctx := context.Background()

7
pkg/cmd/pr/create/create_test.go
View file

@ -798,6 +798,7 @@ func Test_createRun(t *testing.T) {
cs.Register("git remote rename origin upstream", 0, "")
cs.Register(`git remote add origin https://github.com/monalisa/REPO.git`, 0, "")
cs.Register(`git push --set-upstream origin HEAD:refs/heads/feature`, 0, "")
cs.Register(`git config --add remote.upstream.gh-resolved base`, 0, "")
},
promptStubs: func(pm *prompter.PrompterMock) {
pm.SelectFunc = func(p, _ string, opts []string) (int, error) {
@ -809,7 +810,7 @@ func Test_createRun(t *testing.T) {
}
},
expectedOut: "https://github.com/OWNER/REPO/pull/12\n",
expectedErrOut: "\nCreating pull request for monalisa:feature into master in OWNER/REPO\n\nChanged OWNER/REPO remote to \"upstream\"\nAdded monalisa/REPO as remote \"origin\"\n",
expectedErrOut: "\nCreating pull request for monalisa:feature into master in OWNER/REPO\n\nChanged OWNER/REPO remote to \"upstream\"\nAdded monalisa/REPO as remote \"origin\"\n! Repository monalisa/REPO set as the default repository. To learn more about the default repository, run: gh repo set-default --help\n",
},
{
name: "pushed to non base repo",
@ -1515,7 +1516,7 @@ func Test_createRun(t *testing.T) {
},
customBranchConfig: true,
cmdStubs: func(cs *run.CommandStubber) {
cs.Register(`git config --get-regexp \^branch\\\.task1\\\.\(remote\|merge\|gh-merge-base\)\$`, 0, heredoc.Doc(`
cs.Register(`git config --get-regexp \^branch\\\.task1\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, heredoc.Doc(`
branch.task1.remote origin
branch.task1.merge refs/heads/task1
branch.task1.gh-merge-base feature/feat2`)) // ReadBranchConfig
@ -1549,7 +1550,7 @@ func Test_createRun(t *testing.T) {
defer cmdTeardown(t)
cs.Register(`git status --porcelain`, 0, "")
if !tt.customBranchConfig {
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|gh-merge-base\)\$`, 0, "")
cs.Register(`git config --get-regexp \^branch\\\..+\\\.\(remote\|merge\|pushremote\|gh-merge-base\)\$`, 0, "")
}
if tt.cmdStubs != nil {

118
pkg/cmd/pr/list/http.go
View file

@ -7,6 +7,7 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghrepo"
prShared "github.com/cli/cli/v2/pkg/cmd/pr/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
)
func shouldUseSearch(filters prShared.FilterOptions) bool {
@ -18,113 +19,16 @@ func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters pr
return searchPullRequests(httpClient, repo, filters, limit)
}
type response struct {
Repository struct {
PullRequests struct {
Nodes []api.PullRequest
PageInfo struct {
HasNextPage bool
EndCursor string
}
TotalCount int
}
}
}
fragment := fmt.Sprintf("fragment pr on PullRequest{%s}", api.PullRequestGraphQL(filters.Fields))
query := fragment + `
query PullRequestList(
$owner: String!,
$repo: String!,
$limit: Int!,
$endCursor: String,
$baseBranch: String,
$headBranch: String,
$state: [PullRequestState!] = OPEN
) {
repository(owner: $owner, name: $repo) {
pullRequests(
states: $state,
baseRefName: $baseBranch,
headRefName: $headBranch,
first: $limit,
after: $endCursor,
orderBy: {field: CREATED_AT, direction: DESC}
) {
totalCount
nodes {
...pr
}
pageInfo {
hasNextPage
endCursor
}
}
}
}`
pageLimit := min(limit, 100)
variables := map[string]interface{}{
"owner": repo.RepoOwner(),
"repo": repo.RepoName(),
}
switch filters.State {
case "open":
variables["state"] = []string{"OPEN"}
case "closed":
variables["state"] = []string{"CLOSED", "MERGED"}
case "merged":
variables["state"] = []string{"MERGED"}
case "all":
variables["state"] = []string{"OPEN", "CLOSED", "MERGED"}
default:
return nil, fmt.Errorf("invalid state: %s", filters.State)
}
if filters.BaseBranch != "" {
variables["baseBranch"] = filters.BaseBranch
}
if filters.HeadBranch != "" {
variables["headBranch"] = filters.HeadBranch
}
res := api.PullRequestAndTotalCount{}
var check = make(map[int]struct{})
client := api.NewClientFromHTTP(httpClient)
loop:
for {
variables["limit"] = pageLimit
var data response
err := client.GraphQL(repo.RepoHost(), query, variables, &data)
if err != nil {
return nil, err
}
prData := data.Repository.PullRequests
res.TotalCount = prData.TotalCount
for _, pr := range prData.Nodes {
if _, exists := check[pr.Number]; exists && pr.Number > 0 {
continue
}
check[pr.Number] = struct{}{}
res.PullRequests = append(res.PullRequests, pr)
if len(res.PullRequests) == limit {
break loop
}
}
if prData.PageInfo.HasNextPage {
variables["endCursor"] = prData.PageInfo.EndCursor
pageLimit = min(pageLimit, limit-len(res.PullRequests))
} else {
break
}
}
return &res, nil
return prShared.NewLister(&cmdutil.Factory{
HttpClient: func() (*http.Client, error) { return httpClient, nil },
BaseRepo: func() (ghrepo.Interface, error) { return repo, nil },
}).List(prShared.ListOptions{
LimitResults: limit,
State: filters.State,
BaseBranch: filters.BaseBranch,
HeadBranch: filters.HeadBranch,
Fields: filters.Fields,
})
}
func searchPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters prShared.FilterOptions, limit int) (*api.PullRequestAndTotalCount, error) {

4
pkg/cmd/pr/list/http_test.go
View file

@ -10,7 +10,7 @@ import (
"github.com/cli/cli/v2/pkg/httpmock"
)
func Test_listPullRequests(t *testing.T) {
func Test_ListPullRequests(t *testing.T) {
type args struct {
repo ghrepo.Interface
filters prShared.FilterOptions
@ -155,7 +155,7 @@ func Test_listPullRequests(t *testing.T) {
_, err := listPullRequests(httpClient, tt.args.repo, tt.args.filters, tt.args.limit)
if (err != nil) != tt.wantErr {
t.Errorf("listPullRequests() error = %v, wantErr %v", err, tt.wantErr)
t.Errorf("ListPullRequests() error = %v, wantErr %v", err, tt.wantErr)
return
}
})

12
pkg/cmd/pr/list/list.go
View file

@ -55,7 +55,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Use: "list",
Short: "List pull requests in a repository",
Long: heredoc.Doc(`
List pull requests in a GitHub repository.
List pull requests in a GitHub repository. By default, this only lists open PRs.
The search query syntax is documented here:
<https://docs.github.com/en/search-github/searching-on-github/searching-issues-and-pull-requests>
@ -222,7 +222,7 @@ func listRun(opts *ListOptions) error {
table.AddField(text.RemoveExcessiveWhitespace(pr.Title))
table.AddField(pr.HeadLabel(), tableprinter.WithColor(cs.Cyan))
if !isTTY {
table.AddField(prStateWithDraft(&pr))
table.AddField(shared.PrStateWithDraft(&pr))
}
table.AddTimeField(opts.Now(), pr.CreatedAt, cs.Gray)
table.EndRow()
@ -234,11 +234,3 @@ func listRun(opts *ListOptions) error {
return nil
}
func prStateWithDraft(pr *api.PullRequest) string {
if pr.IsDraft && pr.State == "OPEN" {
return "DRAFT"
}
return pr.State
}

61
pkg/cmd/pr/shared/commentable.go
View file

@ -17,6 +17,8 @@ import (
"github.com/spf13/cobra"
)
var errNoUserComments = errors.New("no comments found for current user")
type InputType int
const (
@ -32,19 +34,21 @@ type Commentable interface {
}
type CommentableOptions struct {
IO *iostreams.IOStreams
HttpClient func() (*http.Client, error)
RetrieveCommentable func() (Commentable, ghrepo.Interface, error)
EditSurvey func(string) (string, error)
InteractiveEditSurvey func(string) (string, error)
ConfirmSubmitSurvey func() (bool, error)
OpenInBrowser func(string) error
Interactive bool
InputType InputType
Body string
EditLast bool
Quiet bool
Host string
IO *iostreams.IOStreams
HttpClient func() (*http.Client, error)
RetrieveCommentable func() (Commentable, ghrepo.Interface, error)
EditSurvey func(string) (string, error)
InteractiveEditSurvey func(string) (string, error)
ConfirmSubmitSurvey func() (bool, error)
ConfirmCreateIfNoneSurvey func() (bool, error)
OpenInBrowser func(string) error
Interactive bool
InputType InputType
Body string
EditLast bool
CreateIfNone bool
Quiet bool
Host string
}
func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error {
@ -66,6 +70,10 @@ func CommentablePreRun(cmd *cobra.Command, opts *CommentableOptions) error {
inputFlags++
}
if opts.CreateIfNone && !opts.EditLast {
return cmdutil.FlagErrorf("`--create-if-none` can only be used with `--edit-last`")
}
if inputFlags == 0 {
if !opts.IO.CanPrompt() {
return cmdutil.FlagErrorf("flags required when not running interactively")
@ -85,7 +93,24 @@ func CommentableRun(opts *CommentableOptions) error {
}
opts.Host = repo.RepoHost()
if opts.EditLast {
return updateComment(commentable, opts)
err := updateComment(commentable, opts)
if !errors.Is(err, errNoUserComments) {
return err
}
if opts.Interactive {
if opts.CreateIfNone {
fmt.Fprintln(opts.IO.ErrOut, "No comments found. Creating a new comment.")
} else {
ok, err := opts.ConfirmCreateIfNoneSurvey()
if err != nil {
return err
}
if !ok {
return errNoUserComments
}
}
}
}
return createComment(commentable, opts)
}
@ -144,7 +169,7 @@ func createComment(commentable Commentable, opts *CommentableOptions) error {
func updateComment(commentable Commentable, opts *CommentableOptions) error {
comments := commentable.CurrentUserComments()
if len(comments) == 0 {
return fmt.Errorf("no comments found for current user")
return errNoUserComments
}
lastComment := &comments[len(comments)-1]
@ -219,6 +244,12 @@ func CommentableInteractiveEditSurvey(cf func() (gh.Config, error), io *iostream
}
}
func CommentableInteractiveCreateIfNoneSurvey(p Prompt) func() (bool, error) {
return func() (bool, error) {
return p.Confirm("No comments found. Create one?", true)
}
}
func CommentableEditSurvey(cf func() (gh.Config, error), io *iostreams.IOStreams) func(string) (string, error) {
return func(initialValue string) (string, error) {
editorCommand, err := cmdutil.DetermineEditor(cf)

8
pkg/cmd/pr/shared/display.go
View file

@ -17,6 +17,14 @@ func StateTitleWithColor(cs *iostreams.ColorScheme, pr api.PullRequest) string {
return prStateColorFunc(text.Title(pr.State))
}
func PrStateWithDraft(pr *api.PullRequest) string {
if pr.IsDraft && pr.State == "OPEN" {
return "DRAFT"
}
return pr.State
}
func ColorForPRState(pr api.PullRequest) string {
switch pr.State {
case "OPEN":

255
pkg/cmd/pr/shared/finder.go
View file

@ -33,16 +33,19 @@ type progressIndicator interface {
}
type finder struct {
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
remotesFn func() (remotes.Remotes, error)
httpClient func() (*http.Client, error)
branchConfig func(string) (git.BranchConfig, error)
progress progressIndicator
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
remotesFn func() (remotes.Remotes, error)
httpClient func() (*http.Client, error)
pushDefault func() (string, error)
remotePushDefault func() (string, error)
parsePushRevision func(string) (string, error)
branchConfig func(string) (git.BranchConfig, error)
progress progressIndicator
repo ghrepo.Interface
prNumber int
branchName string
baseRefRepo ghrepo.Interface
prNumber int
branchName string
}
func NewFinder(factory *cmdutil.Factory) PRFinder {
@ -57,7 +60,16 @@ func NewFinder(factory *cmdutil.Factory) PRFinder {
branchFn: factory.Branch,
remotesFn: factory.Remotes,
httpClient: factory.HttpClient,
progress: factory.IOStreams,
pushDefault: func() (string, error) {
return factory.GitClient.PushDefault(context.Background())
},
remotePushDefault: func() (string, error) {
return factory.GitClient.RemotePushDefault(context.Background())
},
parsePushRevision: func(branch string) (string, error) {
return factory.GitClient.ParsePushRevision(context.Background(), branch)
},
progress: factory.IOStreams,
branchConfig: func(s string) (git.BranchConfig, error) {
return factory.GitClient.ReadBranchConfig(context.Background(), s)
},
@ -85,46 +97,118 @@ type FindOptions struct {
States []string
}
// TODO: Does this also need the BaseBranchName?
// PR's are represented by the following:
// baseRef -----PR-----> headRef
//
// A ref is described as "remoteName/branchName", so
// baseRepoName/baseBranchName -----PR-----> headRepoName/headBranchName
type PullRequestRefs struct {
BranchName string
HeadRepo ghrepo.Interface
BaseRepo ghrepo.Interface
}
// GetPRHeadLabel returns the string that the GitHub API uses to identify the PR. This is
// either just the branch name or, if the PR is originating from a fork, the fork owner
// and the branch name, like <owner>:<branch>.
func (s *PullRequestRefs) GetPRHeadLabel() string {
if ghrepo.IsSame(s.HeadRepo, s.BaseRepo) {
return s.BranchName
}
return fmt.Sprintf("%s:%s", s.HeadRepo.RepoOwner(), s.BranchName)
}
func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, error) {
// If we have a URL, we don't need git stuff
if len(opts.Fields) == 0 {
return nil, nil, errors.New("Find error: no fields specified")
}
if repo, prNumber, err := f.parseURL(opts.Selector); err == nil {
f.prNumber = prNumber
f.repo = repo
f.baseRefRepo = repo
}
if f.repo == nil {
if f.baseRefRepo == nil {
repo, err := f.baseRepoFn()
if err != nil {
return nil, nil, err
}
f.repo = repo
f.baseRefRepo = repo
}
var prRefs PullRequestRefs
if opts.Selector == "" {
if branch, prNumber, err := f.parseCurrentBranch(); err != nil {
// You must be in a git repo for this case to work
currentBranchName, err := f.branchFn()
if err != nil {
return nil, nil, err
} else if prNumber > 0 {
f.prNumber = prNumber
} else {
f.branchName = branch
}
f.branchName = currentBranchName
// Get the branch config for the current branchName
branchConfig, err := f.branchConfig(f.branchName)
if err != nil {
return nil, nil, err
}
// Determine if the branch is configured to merge to a special PR ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
prNumber, _ := strconv.Atoi(m[1])
f.prNumber = prNumber
}
// Determine the PullRequestRefs from config
if f.prNumber == 0 {
rems, err := f.remotesFn()
if err != nil {
return nil, nil, err
}
// Suppressing these errors as we have other means of computing the PullRequestRefs when these fail.
parsedPushRevision, _ := f.parsePushRevision(f.branchName)
pushDefault, err := f.pushDefault()
if err != nil {
return nil, nil, err
}
remotePushDefault, err := f.remotePushDefault()
if err != nil {
return nil, nil, err
}
prRefs, err = ParsePRRefs(f.branchName, branchConfig, parsedPushRevision, pushDefault, remotePushDefault, f.baseRefRepo, rems)
if err != nil {
return nil, nil, err
}
}
} else if f.prNumber == 0 {
// You gave me a selector but I couldn't find a PR number (it wasn't a URL)
// Try to get a PR number from the selector
prNumber, err := strconv.Atoi(strings.TrimPrefix(opts.Selector, "#"))
// If opts.Selector is a valid number then assume it is the
// PR number unless opts.BaseBranch is specified. This is a
// special case for PR create command which will always want
// to assume that a numerical selector is a branch name rather
// than PR number.
prNumber, err := strconv.Atoi(strings.TrimPrefix(opts.Selector, "#"))
if opts.BaseBranch == "" && err == nil {
f.prNumber = prNumber
} else {
f.branchName = opts.Selector
// We don't expect an error here because parsedPushRevision is empty
prRefs, err = ParsePRRefs(f.branchName, git.BranchConfig{}, "", "", "", f.baseRefRepo, remotes.Remotes{})
if err != nil {
return nil, nil, err
}
}
}
// Set up HTTP client
httpClient, err := f.httpClient()
if err != nil {
return nil, nil, err
@ -143,7 +227,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
if fields.Contains("isInMergeQueue") || fields.Contains("isMergeQueueEnabled") {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
detector := fd.NewDetector(cachedClient, f.repo.RepoHost())
detector := fd.NewDetector(cachedClient, f.baseRefRepo.RepoHost())
prFeatures, err := detector.PullRequestFeatures()
if err != nil {
return nil, nil, err
@ -164,36 +248,39 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
if f.prNumber > 0 {
if numberFieldOnly {
// avoid hitting the API if we already have all the information
return &api.PullRequest{Number: f.prNumber}, f.repo, nil
return &api.PullRequest{Number: f.prNumber}, f.baseRefRepo, nil
}
pr, err = findByNumber(httpClient, f.baseRefRepo, f.prNumber, fields.ToSlice())
if err != nil {
return pr, f.baseRefRepo, err
}
pr, err = findByNumber(httpClient, f.repo, f.prNumber, fields.ToSlice())
} else {
pr, err = findForBranch(httpClient, f.repo, opts.BaseBranch, f.branchName, opts.States, fields.ToSlice())
}
if err != nil {
return pr, f.repo, err
pr, err = findForBranch(httpClient, f.baseRefRepo, opts.BaseBranch, prRefs.GetPRHeadLabel(), opts.States, fields.ToSlice())
if err != nil {
return pr, f.baseRefRepo, err
}
}
g, _ := errgroup.WithContext(context.Background())
if fields.Contains("reviews") {
g.Go(func() error {
return preloadPrReviews(httpClient, f.repo, pr)
return preloadPrReviews(httpClient, f.baseRefRepo, pr)
})
}
if fields.Contains("comments") {
g.Go(func() error {
return preloadPrComments(httpClient, f.repo, pr)
return preloadPrComments(httpClient, f.baseRefRepo, pr)
})
}
if fields.Contains("statusCheckRollup") {
g.Go(func() error {
return preloadPrChecks(httpClient, f.repo, pr)
return preloadPrChecks(httpClient, f.baseRefRepo, pr)
})
}
if getProjectItems {
g.Go(func() error {
apiClient := api.NewClientFromHTTP(httpClient)
err := api.ProjectsV2ItemsForPullRequest(apiClient, f.repo, pr)
err := api.ProjectsV2ItemsForPullRequest(apiClient, f.baseRefRepo, pr)
if err != nil && !api.ProjectsV2IgnorableError(err) {
return err
}
@ -201,7 +288,7 @@ func (f *finder) Find(opts FindOptions) (*api.PullRequest, ghrepo.Interface, err
})
}
return pr, f.repo, g.Wait()
return pr, f.baseRefRepo, g.Wait()
}
var pullURLRE = regexp.MustCompile(`^/([^/]+)/([^/]+)/pull/(\d+)`)
@ -230,59 +317,70 @@ func (f *finder) parseURL(prURL string) (ghrepo.Interface, int, error) {
return repo, prNumber, nil
}
var prHeadRE = regexp.MustCompile(`^refs/pull/(\d+)/head$`)
func (f *finder) parseCurrentBranch() (string, int, error) {
prHeadRef, err := f.branchFn()
if err != nil {
return "", 0, err
func ParsePRRefs(currentBranchName string, branchConfig git.BranchConfig, parsedPushRevision string, pushDefault string, remotePushDefault string, baseRefRepo ghrepo.Interface, rems remotes.Remotes) (PullRequestRefs, error) {
prRefs := PullRequestRefs{
BaseRepo: baseRefRepo,
}
branchConfig, err := f.branchConfig(prHeadRef)
if err != nil {
return "", 0, err
// If @{push} resolves, then we have all the information we need to determine the head repo
// and branch name. It is of the form <remote>/<branch>.
if parsedPushRevision != "" {
for _, r := range rems {
// Find the remote who's name matches the push <remote> prefix
if strings.HasPrefix(parsedPushRevision, r.Name+"/") {
prRefs.BranchName = strings.TrimPrefix(parsedPushRevision, r.Name+"/")
prRefs.HeadRepo = r.Repo
return prRefs, nil
}
}
remoteNames := make([]string, len(rems))
for i, r := range rems {
remoteNames[i] = r.Name
}
return PullRequestRefs{}, fmt.Errorf("no remote for %q found in %q", parsedPushRevision, strings.Join(remoteNames, ", "))
}
// the branch is configured to merge a special PR head ref
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
prNumber, _ := strconv.Atoi(m[1])
return "", prNumber, nil
// We assume the PR's branch name is the same as whatever f.BranchFn() returned earlier
// unless the user has specified push.default = upstream or tracking, then we use the
// branch name from the merge ref.
prRefs.BranchName = currentBranchName
if pushDefault == "upstream" || pushDefault == "tracking" {
prRefs.BranchName = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
var gitRemoteRepo ghrepo.Interface
if branchConfig.RemoteURL != nil {
// the branch merges from a remote specified by URL
if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil {
gitRemoteRepo = r
// To get the HeadRepo, we look to the git config. The HeadRepo comes from one of the following, in order of precedence:
// 1. branch.<name>.pushRemote
// 2. remote.pushDefault
// 3. branch.<name>.remote
if branchConfig.PushRemoteName != "" {
if r, err := rems.FindByName(branchConfig.PushRemoteName); err == nil {
prRefs.HeadRepo = r.Repo
}
} else if branchConfig.PushRemoteURL != nil {
if r, err := ghrepo.FromURL(branchConfig.PushRemoteURL); err == nil {
prRefs.HeadRepo = r
}
} else if remotePushDefault != "" {
if r, err := rems.FindByName(remotePushDefault); err == nil {
prRefs.HeadRepo = r.Repo
}
} else if branchConfig.RemoteName != "" {
// the branch merges from a remote specified by name
rem, _ := f.remotesFn()
if r, err := rem.FindByName(branchConfig.RemoteName); err == nil {
gitRemoteRepo = r
if r, err := rems.FindByName(branchConfig.RemoteName); err == nil {
prRefs.HeadRepo = r.Repo
}
} else if branchConfig.RemoteURL != nil {
if r, err := ghrepo.FromURL(branchConfig.RemoteURL); err == nil {
prRefs.HeadRepo = r
}
}
if gitRemoteRepo != nil {
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
prHeadRef = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
// This is determined by:
// - The repo having a different owner
// - The repo having the same owner but a different name (private org fork)
// I suspect that the implementation of the second case may be broken in the face
// of a repo rename, where the remote hasn't been updated locally. This is a
// frequent issue in commands that use SmartBaseRepoFunc. It's not any worse than not
// supporting this case at all though.
sameOwner := strings.EqualFold(gitRemoteRepo.RepoOwner(), f.repo.RepoOwner())
sameOwnerDifferentRepoName := sameOwner && !strings.EqualFold(gitRemoteRepo.RepoName(), f.repo.RepoName())
if !sameOwner || sameOwnerDifferentRepoName {
prHeadRef = fmt.Sprintf("%s:%s", gitRemoteRepo.RepoOwner(), prHeadRef)
}
// The PR merges from a branch in the same repo as the base branch (usually the default branch)
if prRefs.HeadRepo == nil {
prRefs.HeadRepo = baseRefRepo
}
return prHeadRef, 0, nil
return prRefs, nil
}
func findByNumber(httpClient *http.Client, repo ghrepo.Interface, number int, fields []string) (*api.PullRequest, error) {
@ -315,7 +413,7 @@ func findByNumber(httpClient *http.Client, repo ghrepo.Interface, number int, fi
return &resp.Repository.PullRequest, nil
}
func findForBranch(httpClient *http.Client, repo ghrepo.Interface, baseBranch, headBranch string, stateFilters, fields []string) (*api.PullRequest, error) {
func findForBranch(httpClient *http.Client, repo ghrepo.Interface, baseBranch, headBranchWithOwnerIfFork string, stateFilters, fields []string) (*api.PullRequest, error) {
type response struct {
Repository struct {
PullRequests struct {
@ -342,9 +440,9 @@ func findForBranch(httpClient *http.Client, repo ghrepo.Interface, baseBranch, h
}
}`, api.PullRequestGraphQL(fieldSet.ToSlice()))
branchWithoutOwner := headBranch
if idx := strings.Index(headBranch, ":"); idx >= 0 {
branchWithoutOwner = headBranch[idx+1:]
branchWithoutOwner := headBranchWithOwnerIfFork
if idx := strings.Index(headBranchWithOwnerIfFork, ":"); idx >= 0 {
branchWithoutOwner = headBranchWithOwnerIfFork[idx+1:]
}
variables := map[string]interface{}{
@ -367,18 +465,17 @@ func findForBranch(httpClient *http.Client, repo ghrepo.Interface, baseBranch, h
})
for _, pr := range prs {
headBranchMatches := pr.HeadLabel() == headBranch
headBranchMatches := pr.HeadLabel() == headBranchWithOwnerIfFork
baseBranchEmptyOrMatches := baseBranch == "" || pr.BaseRefName == baseBranch
// When the head is the default branch, it doesn't really make sense to show merged or closed PRs.
// https://github.com/cli/cli/issues/4263
isNotClosedOrMergedWhenHeadIsDefault := pr.State == "OPEN" || resp.Repository.DefaultBranchRef.Name != headBranch
isNotClosedOrMergedWhenHeadIsDefault := pr.State == "OPEN" || resp.Repository.DefaultBranchRef.Name != headBranchWithOwnerIfFork
if headBranchMatches && baseBranchEmptyOrMatches && isNotClosedOrMergedWhenHeadIsDefault {
return &pr, nil
}
}
return nil, &NotFoundError{fmt.Errorf("no pull requests found for branch %q", headBranch)}
return nil, &NotFoundError{fmt.Errorf("no pull requests found for branch %q", headBranchWithOwnerIfFork)}
}
func preloadPrReviews(httpClient *http.Client, repo ghrepo.Interface, pr *api.PullRequest) error {

641
pkg/cmd/pr/shared/finder_test.go
View file

@ -2,6 +2,7 @@ package shared
import (
"errors"
"fmt"
"net/http"
"net/url"
"testing"
@ -15,16 +16,50 @@ import (
)
type args struct {
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
branchConfig func(string) (git.BranchConfig, error)
remotesFn func() (context.Remotes, error)
selector string
fields []string
baseBranch string
baseRepoFn func() (ghrepo.Interface, error)
branchFn func() (string, error)
branchConfig func(string) (git.BranchConfig, error)
pushDefault func() (string, error)
remotePushDefault func() (string, error)
parsePushRevision func(string) (string, error)
selector string
fields []string
baseBranch string
}
func TestFind(t *testing.T) {
// TODO: Abstract these out meaningfully for reuse in parsePRRefs tests
originOwnerUrl, err := url.Parse("https://github.com/ORIGINOWNER/REPO.git")
if err != nil {
t.Fatal(err)
}
remoteOrigin := context.Remote{
Remote: &git.Remote{
Name: "origin",
FetchURL: originOwnerUrl,
},
Repo: ghrepo.New("ORIGINOWNER", "REPO"),
}
remoteOther := context.Remote{
Remote: &git.Remote{
Name: "other",
FetchURL: originOwnerUrl,
},
Repo: ghrepo.New("ORIGINOWNER", "OTHER-REPO"),
}
upstreamOwnerUrl, err := url.Parse("https://github.com/UPSTREAMOWNER/REPO.git")
if err != nil {
t.Fatal(err)
}
remoteUpstream := context.Remote{
Remote: &git.Remote{
Name: "upstream",
FetchURL: upstreamOwnerUrl,
},
Repo: ghrepo.New("UPSTREAMOWNER", "REPO"),
}
tests := []struct {
name string
args args
@ -36,11 +71,13 @@ func TestFind(t *testing.T) {
{
name: "number argument",
args: args{
selector: "13",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
selector: "13",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -50,7 +87,7 @@ func TestFind(t *testing.T) {
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "number argument with base branch",
@ -58,9 +95,16 @@ func TestFind(t *testing.T) {
selector: "13",
baseBranch: "main",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{
PushRemoteName: remoteOrigin.Remote.Name,
}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -73,22 +117,26 @@ func TestFind(t *testing.T) {
"baseRefName": "main",
"headRefName": "13",
"isCrossRepository": false,
"headRepositoryOwner": {"login":"OWNER"}
"headRepositoryOwner": {"login":"ORIGINOWNER"}
}
]}
}}}`))
},
wantPR: 123,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "baseRepo is error",
args: args{
selector: "13",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return nil, errors.New("baseRepoErr")
selector: "13",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(nil, errors.New("baseRepoErr")),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
wantErr: true,
},
@ -103,24 +151,32 @@ func TestFind(t *testing.T) {
{
name: "number only",
args: args{
selector: "13",
fields: []string{"number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
selector: "13",
fields: []string{"number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
httpStub: nil,
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "number with hash argument",
args: args{
selector: "#13",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
selector: "#13",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -130,14 +186,20 @@ func TestFind(t *testing.T) {
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "URL argument",
name: "PR URL argument",
args: args{
selector: "https://example.org/OWNER/REPO/pull/13/files",
fields: []string{"id", "number"},
baseRepoFn: nil,
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -150,13 +212,50 @@ func TestFind(t *testing.T) {
wantRepo: "https://example.org/OWNER/REPO",
},
{
name: "branch argument",
name: "PR URL argument and not in a local git repo",
args: args{
selector: "blueberries",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
selector: "https://example.org/OWNER/REPO/pull/13/files",
fields: []string{"id", "number"},
baseRepoFn: nil,
branchFn: func() (string, error) {
return "", &git.GitError{
Stderr: "fatal: branchFn error",
ExitCode: 128,
}
},
branchConfig: stubBranchConfig(git.BranchConfig{}, &git.GitError{
Stderr: "fatal: branchConfig error",
ExitCode: 128,
}),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", &git.GitError{
Stderr: "fatal: remotePushDefault error",
ExitCode: 128,
}),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
httpmock.GraphQL(`query PullRequestByNumber\b`),
httpmock.StringResponse(`{"data":{"repository":{
"pullRequest":{"number":13}
}}}`))
},
wantPR: 13,
wantRepo: "https://example.org/OWNER/REPO",
},
{
name: "when provided branch argument with an open and closed PR for that branch name, it returns the open PR",
args: args{
selector: "blueberries",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
parsePushRevision: stubParsedPushRevision("", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -169,7 +268,7 @@ func TestFind(t *testing.T) {
"baseRefName": "main",
"headRefName": "blueberries",
"isCrossRepository": false,
"headRepositoryOwner": {"login":"OWNER"}
"headRepositoryOwner": {"login":"ORIGINOWNER"}
},
{
"number": 13,
@ -177,13 +276,13 @@ func TestFind(t *testing.T) {
"baseRefName": "main",
"headRefName": "blueberries",
"isCrossRepository": false,
"headRepositoryOwner": {"login":"OWNER"}
"headRepositoryOwner": {"login":"ORIGINOWNER"}
}
]}
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "branch argument with base branch",
@ -194,6 +293,13 @@ func TestFind(t *testing.T) {
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -233,7 +339,10 @@ func TestFind(t *testing.T) {
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -265,7 +374,10 @@ func TestFind(t *testing.T) {
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -303,26 +415,21 @@ func TestFind(t *testing.T) {
wantErr: true,
},
{
name: "current branch with upstream configuration",
name: "when the current branch is configured to push to and pull from 'upstream' and push.default = upstream but the repo push/pulls from 'origin', it finds the PR associated with the upstream repo and returns origin as the base repo",
args: args{
selector: "",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
selector: "",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{
MergeRef: "refs/heads/blue-upstream-berries",
RemoteName: "origin",
MergeRef: "refs/heads/blue-upstream-berries",
PushRemoteName: "upstream",
}, nil),
remotesFn: func() (context.Remotes, error) {
return context.Remotes{{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("UPSTREAMOWNER", "REPO"),
}}, nil
},
pushDefault: stubPushDefault("upstream", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -341,27 +448,28 @@ func TestFind(t *testing.T) {
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "current branch with upstream RemoteURL configuration",
// The current BRANCH is configured to push to and pull from a URL (upstream, in this example)
// which is different from what the REPO is configured to push to and pull from (origin, in this example)
// and push.default = upstream. It should find the PR associated with the upstream repo and return
// origin as the base repo
name: "when push.default = upstream and the current branch is configured to push/pull from a different remote than the repo",
args: args{
selector: "",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
selector: "",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: func(branch string) (git.BranchConfig, error) {
u, _ := url.Parse("https://github.com/UPSTREAMOWNER/REPO")
return stubBranchConfig(git.BranchConfig{
MergeRef: "refs/heads/blue-upstream-berries",
RemoteURL: u,
}, nil)(branch)
},
remotesFn: nil,
branchConfig: stubBranchConfig(git.BranchConfig{
MergeRef: "refs/heads/blue-upstream-berries",
PushRemoteURL: remoteUpstream.Remote.FetchURL,
}, nil),
pushDefault: stubPushDefault("upstream", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -380,28 +488,21 @@ func TestFind(t *testing.T) {
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "current branch with upstream and fork in same org",
args: args{
selector: "",
fields: []string{"id", "number"},
baseRepoFn: func() (ghrepo.Interface, error) {
return ghrepo.FromFullName("OWNER/REPO")
},
selector: "",
fields: []string{"id", "number"},
baseRepoFn: stubBaseRepoFn(ghrepo.New("ORIGINOWNER", "REPO"), nil),
branchFn: func() (string, error) {
return "blueberries", nil
},
branchConfig: stubBranchConfig(git.BranchConfig{
RemoteName: "origin",
}, nil),
remotesFn: func() (context.Remotes, error) {
return context.Remotes{{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.New("OWNER", "REPO-FORK"),
}}, nil
},
branchConfig: stubBranchConfig(git.BranchConfig{}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
parsePushRevision: stubParsedPushRevision("other/blueberries", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -414,13 +515,13 @@ func TestFind(t *testing.T) {
"baseRefName": "main",
"headRefName": "blueberries",
"isCrossRepository": true,
"headRepositoryOwner": {"login":"OWNER"}
"headRepositoryOwner": {"login":"ORIGINOWNER"}
}
]}
}}}`))
},
wantPR: 13,
wantRepo: "https://github.com/OWNER/REPO",
wantRepo: "https://github.com/ORIGINOWNER/REPO",
},
{
name: "current branch made by pr checkout",
@ -461,6 +562,8 @@ func TestFind(t *testing.T) {
branchConfig: stubBranchConfig(git.BranchConfig{
MergeRef: "refs/pull/13/head",
}, nil),
pushDefault: stubPushDefault("simple", nil),
remotePushDefault: stubRemotePushDefault("", nil),
},
httpStub: func(r *httpmock.Registry) {
r.Register(
@ -521,10 +624,17 @@ func TestFind(t *testing.T) {
httpClient: func() (*http.Client, error) {
return &http.Client{Transport: reg}, nil
},
baseRepoFn: tt.args.baseRepoFn,
branchFn: tt.args.branchFn,
branchConfig: tt.args.branchConfig,
remotesFn: tt.args.remotesFn,
baseRepoFn: tt.args.baseRepoFn,
branchFn: tt.args.branchFn,
branchConfig: tt.args.branchConfig,
pushDefault: tt.args.pushDefault,
remotePushDefault: tt.args.remotePushDefault,
parsePushRevision: tt.args.parsePushRevision,
remotesFn: stubRemotes(context.Remotes{
&remoteOrigin,
&remoteOther,
&remoteUpstream,
}, nil),
}
pr, repo, err := f.Find(FindOptions{
@ -557,42 +667,307 @@ func TestFind(t *testing.T) {
}
}
func Test_parseCurrentBranch(t *testing.T) {
func TestParsePRRefs(t *testing.T) {
originOwnerUrl, err := url.Parse("https://github.com/ORIGINOWNER/REPO.git")
if err != nil {
t.Fatal(err)
}
remoteOrigin := context.Remote{
Remote: &git.Remote{
Name: "origin",
FetchURL: originOwnerUrl,
},
Repo: ghrepo.New("ORIGINOWNER", "REPO"),
}
remoteOther := context.Remote{
Remote: &git.Remote{
Name: "other",
FetchURL: originOwnerUrl,
},
Repo: ghrepo.New("ORIGINOWNER", "REPO"),
}
upstreamOwnerUrl, err := url.Parse("https://github.com/UPSTREAMOWNER/REPO.git")
if err != nil {
t.Fatal(err)
}
remoteUpstream := context.Remote{
Remote: &git.Remote{
Name: "upstream",
FetchURL: upstreamOwnerUrl,
},
Repo: ghrepo.New("UPSTREAMOWNER", "REPO"),
}
tests := []struct {
name string
args args
wantSelector string
wantPR int
wantError error
name string
branchConfig git.BranchConfig
pushDefault string
parsedPushRevision string
remotePushDefault string
currentBranchName string
baseRefRepo ghrepo.Interface
rems context.Remotes
wantPRRefs PullRequestRefs
wantErr error
}{
{
name: "failed branch config",
args: args{
branchConfig: stubBranchConfig(git.BranchConfig{}, errors.New("branchConfigErr")),
branchFn: func() (string, error) {
return "blueberries", nil
},
name: "When the branch is called 'blueberries' with an empty branch config, it returns the correct PullRequestRefs",
branchConfig: git.BranchConfig{},
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantSelector: "",
wantPR: 0,
wantError: errors.New("branchConfigErr"),
wantErr: nil,
},
{
name: "When the branch is called 'otherBranch' with an empty branch config, it returns the correct PullRequestRefs",
branchConfig: git.BranchConfig{},
currentBranchName: "otherBranch",
baseRefRepo: remoteOrigin.Repo,
wantPRRefs: PullRequestRefs{
BranchName: "otherBranch",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the branch name doesn't match the branch name in BranchConfig.Push, it returns the BranchConfig.Push branch name",
parsedPushRevision: "origin/pushBranch",
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
},
wantPRRefs: PullRequestRefs{
BranchName: "pushBranch",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the push revision doesn't match a remote, it returns an error",
parsedPushRevision: "origin/differentPushBranch",
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteUpstream,
&remoteOther,
},
wantPRRefs: PullRequestRefs{},
wantErr: fmt.Errorf("no remote for %q found in %q", "origin/differentPushBranch", "upstream, other"),
},
{
name: "When the branch name doesn't match a different branch name in BranchConfig.Push and the remote isn't 'origin', it returns the BranchConfig.Push branch name",
parsedPushRevision: "other/pushBranch",
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOther,
},
wantPRRefs: PullRequestRefs{
BranchName: "pushBranch",
HeadRepo: remoteOther.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the push remote is the same as the baseRepo, it returns the baseRepo as the PullRequestRefs HeadRepo",
branchConfig: git.BranchConfig{
PushRemoteName: remoteOrigin.Remote.Name,
},
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the push remote is different from the baseRepo, it returns the push remote repo as the PullRequestRefs HeadRepo",
branchConfig: git.BranchConfig{
PushRemoteName: remoteOrigin.Remote.Name,
},
currentBranchName: "blueberries",
baseRefRepo: remoteUpstream.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteUpstream.Repo,
},
wantErr: nil,
},
{
name: "When the push remote defined by a URL and the baseRepo is different from the push remote, it returns the push remote repo as the PullRequestRefs HeadRepo",
branchConfig: git.BranchConfig{
PushRemoteURL: remoteOrigin.Remote.FetchURL,
},
currentBranchName: "blueberries",
baseRefRepo: remoteUpstream.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteOrigin.Repo,
BaseRepo: remoteUpstream.Repo,
},
wantErr: nil,
},
{
name: "When the push remote and merge ref are configured to a different repo and push.default = upstream, it should return the branch name from the other repo",
branchConfig: git.BranchConfig{
PushRemoteName: remoteUpstream.Remote.Name,
MergeRef: "refs/heads/blue-upstream-berries",
},
pushDefault: "upstream",
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blue-upstream-berries",
HeadRepo: remoteUpstream.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the push remote and merge ref are configured to a different repo and push.default = tracking, it should return the branch name from the other repo",
branchConfig: git.BranchConfig{
PushRemoteName: remoteUpstream.Remote.Name,
MergeRef: "refs/heads/blue-upstream-berries",
},
pushDefault: "tracking",
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blue-upstream-berries",
HeadRepo: remoteUpstream.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When remote.pushDefault is set, it returns the correct PullRequestRefs",
branchConfig: git.BranchConfig{},
remotePushDefault: remoteUpstream.Remote.Name,
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteUpstream.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the remote name is set on the branch, it returns the correct PullRequestRefs",
branchConfig: git.BranchConfig{
RemoteName: remoteUpstream.Remote.Name,
},
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteUpstream.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
{
name: "When the remote URL is set on the branch, it returns the correct PullRequestRefs",
branchConfig: git.BranchConfig{
RemoteURL: remoteUpstream.Remote.FetchURL,
},
currentBranchName: "blueberries",
baseRefRepo: remoteOrigin.Repo,
rems: context.Remotes{
&remoteOrigin,
&remoteUpstream,
},
wantPRRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: remoteUpstream.Repo,
BaseRepo: remoteOrigin.Repo,
},
wantErr: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
f := finder{
httpClient: func() (*http.Client, error) {
return &http.Client{}, nil
},
baseRepoFn: tt.args.baseRepoFn,
branchFn: tt.args.branchFn,
branchConfig: tt.args.branchConfig,
remotesFn: tt.args.remotesFn,
prRefs, err := ParsePRRefs(tt.currentBranchName, tt.branchConfig, tt.parsedPushRevision, tt.pushDefault, tt.remotePushDefault, tt.baseRefRepo, tt.rems)
if tt.wantErr != nil {
require.Equal(t, tt.wantErr, err)
} else {
require.NoError(t, err)
}
selector, pr, err := f.parseCurrentBranch()
assert.Equal(t, tt.wantSelector, selector)
assert.Equal(t, tt.wantPR, pr)
assert.Equal(t, tt.wantError, err)
require.Equal(t, tt.wantPRRefs, prRefs)
})
}
}
func TestPRRefs_GetPRHeadLabel(t *testing.T) {
originRepo := ghrepo.New("ORIGINOWNER", "REPO")
upstreamRepo := ghrepo.New("UPSTREAMOWNER", "REPO")
tests := []struct {
name string
prRefs PullRequestRefs
want string
}{
{
name: "When the HeadRepo and BaseRepo match, it returns the branch name",
prRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: originRepo,
BaseRepo: originRepo,
},
want: "blueberries",
},
{
name: "When the HeadRepo and BaseRepo do not match, it returns the prepended HeadRepo owner to the branch name",
prRefs: PullRequestRefs{
BranchName: "blueberries",
HeadRepo: originRepo,
BaseRepo: upstreamRepo,
},
want: "ORIGINOWNER:blueberries",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, tt.prRefs.GetPRHeadLabel())
})
}
}
@ -602,3 +977,33 @@ func stubBranchConfig(branchConfig git.BranchConfig, err error) func(string) (gi
return branchConfig, err
}
}
func stubRemotes(remotes context.Remotes, err error) func() (context.Remotes, error) {
return func() (context.Remotes, error) {
return remotes, err
}
}
func stubBaseRepoFn(baseRepo ghrepo.Interface, err error) func() (ghrepo.Interface, error) {
return func() (ghrepo.Interface, error) {
return baseRepo, err
}
}
func stubPushDefault(pushDefault string, err error) func() (string, error) {
return func() (string, error) {
return pushDefault, err
}
}
func stubRemotePushDefault(remotePushDefault string, err error) func() (string, error) {
return func() (string, error) {
return remotePushDefault, err
}
}
func stubParsedPushRevision(parsedPushRevision string, err error) func(string) (string, error) {
return func(_ string) (string, error) {
return parsedPushRevision, err
}
}

196
pkg/cmd/pr/shared/lister.go Normal file
View file

@ -0,0 +1,196 @@
package shared
import (
"fmt"
"net/http"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmdutil"
api "github.com/cli/cli/v2/api"
)
type PRLister interface {
List(opt ListOptions) (*api.PullRequestAndTotalCount, error)
}
type ListOptions struct {
LimitResults int
State string
BaseBranch string
HeadBranch string
Fields []string
}
type lister struct {
baseRepoFn func() (ghrepo.Interface, error)
httpClient func() (*http.Client, error)
}
func NewLister(factory *cmdutil.Factory) PRLister {
return &lister{
baseRepoFn: factory.BaseRepo,
httpClient: factory.HttpClient,
}
}
func (l *lister) List(opts ListOptions) (*api.PullRequestAndTotalCount, error) {
repo, err := l.baseRepoFn()
if err != nil {
return nil, err
}
client, err := l.httpClient()
if err != nil {
return nil, err
}
return listPullRequests(client, repo, opts)
}
func listPullRequests(httpClient *http.Client, repo ghrepo.Interface, filters ListOptions) (*api.PullRequestAndTotalCount, error) {
type response struct {
Repository struct {
PullRequests struct {
Nodes []api.PullRequest
PageInfo struct {
HasNextPage bool
EndCursor string
}
TotalCount int
}
}
}
limit := filters.LimitResults
fragment := fmt.Sprintf("fragment pr on PullRequest{%s}", api.PullRequestGraphQL(filters.Fields))
query := fragment + `
query PullRequestList(
$owner: String!,
$repo: String!,
$limit: Int!,
$endCursor: String,
$baseBranch: String,
$headBranch: String,
$state: [PullRequestState!] = OPEN
) {
repository(owner: $owner, name: $repo) {
pullRequests(
states: $state,
baseRefName: $baseBranch,
headRefName: $headBranch,
first: $limit,
after: $endCursor,
orderBy: {field: CREATED_AT, direction: DESC}
) {
totalCount
nodes {
...pr
}
pageInfo {
hasNextPage
endCursor
}
}
}
}`
pageLimit := min(limit, 100)
variables := map[string]interface{}{
"owner": repo.RepoOwner(),
"repo": repo.RepoName(),
}
switch filters.State {
case "open":
variables["state"] = []string{"OPEN"}
case "closed":
variables["state"] = []string{"CLOSED", "MERGED"}
case "merged":
variables["state"] = []string{"MERGED"}
case "all":
variables["state"] = []string{"OPEN", "CLOSED", "MERGED"}
default:
return nil, fmt.Errorf("invalid state: %s", filters.State)
}
if filters.BaseBranch != "" {
variables["baseBranch"] = filters.BaseBranch
}
if filters.HeadBranch != "" {
variables["headBranch"] = filters.HeadBranch
}
res := api.PullRequestAndTotalCount{}
var check = make(map[int]struct{})
client := api.NewClientFromHTTP(httpClient)
loop:
for {
variables["limit"] = pageLimit
var data response
err := client.GraphQL(repo.RepoHost(), query, variables, &data)
if err != nil {
return nil, err
}
prData := data.Repository.PullRequests
res.TotalCount = prData.TotalCount
for _, pr := range prData.Nodes {
if _, exists := check[pr.Number]; exists && pr.Number > 0 {
continue
}
check[pr.Number] = struct{}{}
res.PullRequests = append(res.PullRequests, pr)
if len(res.PullRequests) == limit {
break loop
}
}
if prData.PageInfo.HasNextPage {
variables["endCursor"] = prData.PageInfo.EndCursor
pageLimit = min(pageLimit, limit-len(res.PullRequests))
} else {
break
}
}
return &res, nil
}
type mockLister struct {
called bool
expectFields []string
result *api.PullRequestAndTotalCount
err error
}
func NewMockLister(result *api.PullRequestAndTotalCount, err error) *mockLister {
return &mockLister{
result: result,
err: err,
}
}
func (m *mockLister) List(opt ListOptions) (*api.PullRequestAndTotalCount, error) {
m.called = true
if m.err != nil {
return nil, m.err
}
if m.expectFields != nil {
if !isEqualSet(m.expectFields, opt.Fields) {
return nil, fmt.Errorf("unexpected fields: %v", opt.Fields)
}
}
return m.result, m.err
}
func (m *mockLister) ExpectFields(fields []string) {
m.expectFields = fields
}

119
pkg/cmd/pr/status/status.go
View file

@ -7,9 +7,9 @@ import (
"net/http"
"regexp"
"strconv"
"strings"
"time"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
ghContext "github.com/cli/cli/v2/context"
"github.com/cli/cli/v2/git"
@ -52,7 +52,15 @@ func NewCmdStatus(f *cmdutil.Factory, runF func(*StatusOptions) error) *cobra.Co
cmd := &cobra.Command{
Use: "status",
Short: "Show status of relevant pull requests",
Args: cmdutil.NoArgsQuoteReminder,
Long: heredoc.Docf(`
Show status of relevant pull requests.
The status shows a summary of pull requests that includes information such as
pull request number, title, CI checks, reviews, etc.
To see more details of CI checks, run %[1]sgh pr checks%[1]s.
`, "`"),
Args: cmdutil.NoArgsQuoteReminder,
RunE: func(cmd *cobra.Command, args []string) error {
// support `-R, --repo` override
opts.BaseRepo = f.BaseRepo
@ -78,27 +86,57 @@ func statusRun(opts *StatusOptions) error {
return err
}
baseRepo, err := opts.BaseRepo()
baseRefRepo, err := opts.BaseRepo()
if err != nil {
return err
}
var currentBranch string
var currentBranchName string
var currentPRNumber int
var currentPRHeadRef string
var currentHeadRefBranchName string
if !opts.HasRepoOverride {
currentBranch, err = opts.Branch()
// We must be in a repo without the override
currentBranchName, err = opts.Branch()
if err != nil && !errors.Is(err, git.ErrNotOnAnyBranch) {
return fmt.Errorf("could not query for pull request for current branch: %w", err)
}
remotes, _ := opts.Remotes()
branchConfig, err := opts.GitClient.ReadBranchConfig(ctx, currentBranch)
branchConfig, err := opts.GitClient.ReadBranchConfig(ctx, currentBranchName)
if err != nil {
return err
}
currentPRNumber, currentPRHeadRef, err = prSelectorForCurrentBranch(branchConfig, baseRepo, currentBranch, remotes)
// Determine if the branch is configured to merge to a special PR ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
currentPRNumber, _ = strconv.Atoi(m[1])
}
if currentPRNumber == 0 {
remotes, err := opts.Remotes()
if err != nil {
return err
}
// Suppressing these errors as we have other means of computing the PullRequestRefs when these fail.
parsedPushRevision, _ := opts.GitClient.ParsePushRevision(ctx, currentBranchName)
remotePushDefault, err := opts.GitClient.RemotePushDefault(ctx)
if err != nil {
return err
}
pushDefault, err := opts.GitClient.PushDefault(ctx)
if err != nil {
return err
}
prRefs, err := shared.ParsePRRefs(currentBranchName, branchConfig, parsedPushRevision, pushDefault, remotePushDefault, baseRefRepo, remotes)
if err != nil {
return err
}
currentHeadRefBranchName = prRefs.BranchName
}
if err != nil {
return fmt.Errorf("could not query for pull request for current branch: %w", err)
}
@ -107,7 +145,7 @@ func statusRun(opts *StatusOptions) error {
options := requestOptions{
Username: "@me",
CurrentPR: currentPRNumber,
HeadRef: currentPRHeadRef,
HeadRef: currentHeadRefBranchName,
ConflictStatus: opts.ConflictStatus,
}
if opts.Exporter != nil {
@ -116,7 +154,7 @@ func statusRun(opts *StatusOptions) error {
if opts.Detector == nil {
cachedClient := api.NewCachedHTTPClient(httpClient, time.Hour*24)
opts.Detector = fd.NewDetector(cachedClient, baseRepo.RepoHost())
opts.Detector = fd.NewDetector(cachedClient, baseRefRepo.RepoHost())
}
prFeatures, err := opts.Detector.PullRequestFeatures()
if err != nil {
@ -124,7 +162,7 @@ func statusRun(opts *StatusOptions) error {
}
options.CheckRunAndStatusContextCountsSupported = prFeatures.CheckRunAndStatusContextCounts
prPayload, err := pullRequestStatus(httpClient, baseRepo, options)
prPayload, err := pullRequestStatus(httpClient, baseRefRepo, options)
if err != nil {
return err
}
@ -151,21 +189,21 @@ func statusRun(opts *StatusOptions) error {
cs := opts.IO.ColorScheme()
fmt.Fprintln(out, "")
fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRepo))
fmt.Fprintf(out, "Relevant pull requests in %s\n", ghrepo.FullName(baseRefRepo))
fmt.Fprintln(out, "")
if !opts.HasRepoOverride {
shared.PrintHeader(opts.IO, "Current branch")
currentPR := prPayload.CurrentPR
if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranch {
if currentPR != nil && currentPR.State != "OPEN" && prPayload.DefaultBranch == currentBranchName {
currentPR = nil
}
if currentPR != nil {
printPrs(opts.IO, 1, *currentPR)
} else if currentPRHeadRef == "" {
} else if currentHeadRefBranchName == "" {
shared.PrintMessage(opts.IO, " There is no current branch")
} else {
shared.PrintMessage(opts.IO, fmt.Sprintf(" There is no pull request associated with %s", cs.Cyan("["+currentPRHeadRef+"]")))
shared.PrintMessage(opts.IO, fmt.Sprintf(" There is no pull request associated with %s", cs.Cyan("["+currentHeadRefBranchName+"]")))
}
fmt.Fprintln(out)
}
@ -189,55 +227,6 @@ func statusRun(opts *StatusOptions) error {
return nil
}
func prSelectorForCurrentBranch(branchConfig git.BranchConfig, baseRepo ghrepo.Interface, prHeadRef string, rem ghContext.Remotes) (int, string, error) {
// the branch is configured to merge a special PR head ref
prHeadRE := regexp.MustCompile(`^refs/pull/(\d+)/head$`)
if m := prHeadRE.FindStringSubmatch(branchConfig.MergeRef); m != nil {
prNumber, err := strconv.Atoi(m[1])
if err != nil {
return 0, "", err
}
return prNumber, prHeadRef, nil
}
var branchOwner string
if branchConfig.RemoteURL != nil {
// the branch merges from a remote specified by URL
r, err := ghrepo.FromURL(branchConfig.RemoteURL)
if err != nil {
// TODO: We aren't returning the error because we discovered that it was shadowed
// before refactoring to its current return pattern. Thus, we aren't confident
// that returning the error won't break existing behavior.
return 0, prHeadRef, nil
}
branchOwner = r.RepoOwner()
} else if branchConfig.RemoteName != "" {
// the branch merges from a remote specified by name
r, err := rem.FindByName(branchConfig.RemoteName)
if err != nil {
// TODO: We aren't returning the error because we discovered that it was shadowed
// before refactoring to its current return pattern. Thus, we aren't confident
// that returning the error won't break existing behavior.
return 0, prHeadRef, nil
}
branchOwner = r.RepoOwner()
}
if branchOwner != "" {
selector := prHeadRef
if strings.HasPrefix(branchConfig.MergeRef, "refs/heads/") {
selector = strings.TrimPrefix(branchConfig.MergeRef, "refs/heads/")
}
// prepend `OWNER:` if this branch is pushed to a fork
if !strings.EqualFold(branchOwner, baseRepo.RepoOwner()) {
selector = fmt.Sprintf("%s:%s", branchOwner, selector)
}
return 0, selector, nil
}
return 0, prHeadRef, nil
}
func totalApprovals(pr *api.PullRequest) int {
approvals := 0
for _, review := range pr.LatestReviews.Nodes {

248
pkg/cmd/pr/status/status_test.go
View file

@ -4,7 +4,6 @@ import (
"bytes"
"io"
"net/http"
"net/url"
"regexp"
"strings"
"testing"
@ -96,10 +95,13 @@ func TestPRStatus(t *testing.T) {
defer http.Verify(t)
http.Register(httpmock.GraphQL(`query PullRequestStatus\b`), httpmock.FileResponse("./fixtures/prStatus.json"))
// stub successful git command
// stub successful git commands
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -130,6 +132,9 @@ func TestPRStatus_reviewsAndChecks(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -160,6 +165,9 @@ func TestPRStatus_reviewsAndChecksWithStatesByCount(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommandWithDetector(http, "blueberries", true, "", &fd.EnabledDetectorMock{})
if err != nil {
@ -189,6 +197,9 @@ func TestPRStatus_currentBranch_showTheMostRecentPR(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -222,6 +233,9 @@ func TestPRStatus_currentBranch_defaultBranch(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -261,6 +275,9 @@ func TestPRStatus_currentBranch_Closed(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -283,6 +300,9 @@ func TestPRStatus_currentBranch_Closed_defaultBranch(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -305,6 +325,9 @@ func TestPRStatus_currentBranch_Merged(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -327,6 +350,9 @@ func TestPRStatus_currentBranch_Merged_defaultBranch(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -349,6 +375,9 @@ func TestPRStatus_blankSlate(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref blueberries@{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "blueberries", true, "")
if err != nil {
@ -407,6 +436,9 @@ func TestPRStatus_detachedHead(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 0, "")
rs.Register(`git config remote.pushDefault`, 0, "")
rs.Register(`git rev-parse --abbrev-ref @{push}`, 0, "")
rs.Register(`git config push.default`, 0, "")
output, err := runCommand(http, "", true, "")
if err != nil {
@ -434,216 +466,8 @@ Requesting a code review from you
func TestPRStatus_error_ReadBranchConfig(t *testing.T) {
rs, cleanup := run.Stub()
defer cleanup(t)
rs.Register(`git config --get-regexp \^branch\\.`, 1, "")
// We only need the one stub because this fails early
rs.Register(`git config --get-regexp \^branch\\.`, 2, "")
_, err := runCommand(initFakeHTTP(), "blueberries", true, "")
assert.Error(t, err)
}
func Test_prSelectorForCurrentBranch(t *testing.T) {
tests := []struct {
name string
branchConfig git.BranchConfig
baseRepo ghrepo.Interface
prHeadRef string
remotes context.Remotes
wantPrNumber int
wantSelector string
wantError error
}{
{
name: "Empty branch config",
branchConfig: git.BranchConfig{},
prHeadRef: "monalisa/main",
wantPrNumber: 0,
wantSelector: "monalisa/main",
wantError: nil,
},
{
name: "The branch is configured to merge a special PR head ref",
branchConfig: git.BranchConfig{
MergeRef: "refs/pull/42/head",
},
prHeadRef: "monalisa/main",
wantPrNumber: 42,
wantSelector: "monalisa/main",
wantError: nil,
},
{
name: "Branch merges from a remote specified by URL",
branchConfig: git.BranchConfig{
RemoteURL: &url.URL{
Scheme: "ssh",
User: url.User("git"),
Host: "github.com",
Path: "monalisa/playground.git",
},
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "monalisa/main",
wantError: nil,
},
{
name: "Branch merges from a remote specified by name",
branchConfig: git.BranchConfig{
RemoteName: "upstream",
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "monalisa/main",
wantError: nil,
},
{
name: "Branch is a fork and merges from a remote specified by URL",
branchConfig: git.BranchConfig{
RemoteURL: &url.URL{
Scheme: "ssh",
User: url.User("git"),
Host: "github.com",
Path: "forkName/playground.git",
},
MergeRef: "refs/heads/main",
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "forkName:main",
wantError: nil,
},
{
name: "Branch is a fork and merges from a remote specified by name",
branchConfig: git.BranchConfig{
RemoteName: "origin",
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "forkName:monalisa/main",
wantError: nil,
},
{
name: "Branch specifies a mergeRef and merges from a remote specified by name",
branchConfig: git.BranchConfig{
RemoteName: "upstream",
MergeRef: "refs/heads/main",
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "main",
wantError: nil,
},
{
name: "Branch is a fork, specifies a mergeRef, and merges from a remote specified by name",
branchConfig: git.BranchConfig{
RemoteName: "origin",
MergeRef: "refs/heads/main",
},
baseRepo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "forkName:main",
wantError: nil,
},
{
name: "Remote URL errors",
branchConfig: git.BranchConfig{
RemoteURL: &url.URL{
Scheme: "ssh",
User: url.User("git"),
Host: "github.com",
Path: "/\\invalid?Path/",
},
},
prHeadRef: "monalisa/main",
wantPrNumber: 0,
wantSelector: "monalisa/main",
wantError: nil,
},
{
name: "Remote Name errors",
branchConfig: git.BranchConfig{
RemoteName: "nonexistentRemote",
},
prHeadRef: "monalisa/main",
remotes: context.Remotes{
&context.Remote{
Remote: &git.Remote{Name: "origin"},
Repo: ghrepo.NewWithHost("forkName", "playground", "github.com"),
},
&context.Remote{
Remote: &git.Remote{Name: "upstream"},
Repo: ghrepo.NewWithHost("monalisa", "playground", "github.com"),
},
},
wantPrNumber: 0,
wantSelector: "monalisa/main",
wantError: nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
prNum, headRef, err := prSelectorForCurrentBranch(tt.branchConfig, tt.baseRepo, tt.prHeadRef, tt.remotes)
assert.Equal(t, tt.wantPrNumber, prNum)
assert.Equal(t, tt.wantSelector, headRef)
assert.Equal(t, tt.wantError, err)
})
}
}

16
pkg/cmd/project/item-edit/item_edit.go
View file

@ -23,7 +23,8 @@ type editItemOpts struct {
fieldID string
projectID string
text string
number float32
number float64
numberChanged bool
date string
singleSelectOptionID string
iterationID string
@ -63,7 +64,7 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error)
Short: "Edit an item in a project",
Long: heredoc.Docf(`
Edit either a draft issue or a project item. Both usages require the ID of the item to edit.
For non-draft issues, the ID of the project is also required, and only a single field value can be updated per invocation.
Remove project item field value using %[1]s--clear%[1]s flag.
@ -76,10 +77,11 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error)
gh project item-edit --id <item-ID> --field-id <field-ID> --project-id <project-ID> --clear
`),
RunE: func(cmd *cobra.Command, args []string) error {
opts.numberChanged = cmd.Flags().Changed("number")
if err := cmdutil.MutuallyExclusive(
"only one of `--text`, `--number`, `--date`, `--single-select-option-id` or `--iteration-id` may be used",
opts.text != "",
opts.number != 0,
opts.numberChanged,
opts.date != "",
opts.singleSelectOptionID != "",
opts.iterationID != "",
@ -89,7 +91,7 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error)
if err := cmdutil.MutuallyExclusive(
"cannot use `--text`, `--number`, `--date`, `--single-select-option-id` or `--iteration-id` in conjunction with `--clear`",
opts.text != "" || opts.number != 0 || opts.date != "" || opts.singleSelectOptionID != "" || opts.iterationID != "",
opts.text != "" || opts.numberChanged || opts.date != "" || opts.singleSelectOptionID != "" || opts.iterationID != "",
opts.clear,
); err != nil {
return err
@ -123,7 +125,7 @@ func NewCmdEditItem(f *cmdutil.Factory, runF func(config editItemConfig) error)
editItemCmd.Flags().StringVar(&opts.fieldID, "field-id", "", "ID of the field to update")
editItemCmd.Flags().StringVar(&opts.projectID, "project-id", "", "ID of the project to which the field belongs to")
editItemCmd.Flags().StringVar(&opts.text, "text", "", "Text value for the field")
editItemCmd.Flags().Float32Var(&opts.number, "number", 0, "Number value for the field")
editItemCmd.Flags().Float64Var(&opts.number, "number", 0, "Number value for the field")
editItemCmd.Flags().StringVar(&opts.date, "date", "", "Date value for the field (YYYY-MM-DD)")
editItemCmd.Flags().StringVar(&opts.singleSelectOptionID, "single-select-option-id", "", "ID of the single select option value to set on the field")
editItemCmd.Flags().StringVar(&opts.iterationID, "iteration-id", "", "ID of the iteration value to set on the field")
@ -146,7 +148,7 @@ func runEditItem(config editItemConfig) error {
}
// update item values
if config.opts.text != "" || config.opts.number != 0 || config.opts.date != "" || config.opts.singleSelectOptionID != "" || config.opts.iterationID != "" {
if config.opts.text != "" || config.opts.numberChanged || config.opts.date != "" || config.opts.singleSelectOptionID != "" || config.opts.iterationID != "" {
return updateItemValues(config)
}
@ -172,7 +174,7 @@ func buildUpdateItem(config editItemConfig, date time.Time) (*UpdateProjectV2Fie
value = githubv4.ProjectV2FieldValue{
Text: githubv4.NewString(githubv4.String(config.opts.text)),
}
} else if config.opts.number != 0 {
} else if config.opts.numberChanged {
value = githubv4.ProjectV2FieldValue{
Number: githubv4.NewFloat(githubv4.Float(config.opts.number)),
}

80
pkg/cmd/project/item-edit/item_edit_test.go
View file

@ -47,6 +47,22 @@ func TestNewCmdeditItem(t *testing.T) {
itemID: "123",
},
},
{
name: "number with floating point value",
cli: "--number 123.45 --id 123",
wants: editItemOpts{
number: 123.45,
itemID: "123",
},
},
{
name: "number zero",
cli: "--number 0 --id 123",
wants: editItemOpts{
number: 0,
itemID: "123",
},
},
{
name: "field-id",
cli: "--field-id FIELD_ID --id 123",
@ -255,7 +271,7 @@ func TestRunItemEdit_Number(t *testing.T) {
// edit item
gock.New("https://api.github.com").
Post("/graphql").
BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"number":2}}}}`).
BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"number":123.45}}}}`).
Reply(200).
JSON(map[string]interface{}{
"data": map[string]interface{}{
@ -284,10 +300,64 @@ func TestRunItemEdit_Number(t *testing.T) {
config := editItemConfig{
io: ios,
opts: editItemOpts{
number: 2,
itemID: "item_id",
projectID: "project_id",
fieldID: "field_id",
number: 123.45,
numberChanged: true,
itemID: "item_id",
projectID: "project_id",
fieldID: "field_id",
},
client: client,
}
err := runEditItem(config)
assert.NoError(t, err)
assert.Equal(
t,
"Edited item \"title\"\n",
stdout.String())
}
func TestRunItemEdit_NumberZero(t *testing.T) {
defer gock.Off()
// gock.Observe(gock.DumpRequest)
// edit item
gock.New("https://api.github.com").
Post("/graphql").
BodyString(`{"query":"mutation UpdateItemValues.*","variables":{"input":{"projectId":"project_id","itemId":"item_id","fieldId":"field_id","value":{"number":0}}}}`).
Reply(200).
JSON(map[string]interface{}{
"data": map[string]interface{}{
"updateProjectV2ItemFieldValue": map[string]interface{}{
"projectV2Item": map[string]interface{}{
"ID": "item_id",
"content": map[string]interface{}{
"__typename": "Issue",
"body": "body",
"title": "title",
"number": 1,
"repository": map[string]interface{}{
"nameWithOwner": "my-repo",
},
},
},
},
},
})
client := queries.NewTestClient()
ios, _, stdout, _ := iostreams.Test()
ios.SetStdoutTTY(true)
config := editItemConfig{
io: ios,
opts: editItemOpts{
number: 0,
numberChanged: true,
itemID: "item_id",
projectID: "project_id",
fieldID: "field_id",
},
client: client,
}

3
pkg/cmd/project/shared/queries/export_data_test.go
View file

@ -295,6 +295,7 @@ func TestJSONProjectItem_DraftIssue_ProjectV2ItemFieldIterationValue(t *testing.
iterationFieldValue := FieldValueNodes{Type: "ProjectV2ItemFieldIterationValue"}
iterationFieldValue.ProjectV2ItemFieldIterationValue.Title = "Iteration Title"
iterationFieldValue.ProjectV2ItemFieldIterationValue.Field = iterationField
iterationFieldValue.ProjectV2ItemFieldIterationValue.IterationId = "iterationId"
draftIssue := ProjectItem{
Id: "draftIssueId",
@ -321,7 +322,7 @@ func TestJSONProjectItem_DraftIssue_ProjectV2ItemFieldIterationValue(t *testing.
assert.NoError(t, err)
assert.JSONEq(
t,
`{"items":[{"sprint":{"title":"Iteration Title","startDate":"","duration":0},"content":{"type":"DraftIssue","body":"a body","title":"Pull Request title","id":"draftIssueId"},"id":"draftIssueId"}],"totalCount":5}`,
`{"items":[{"sprint":{"title":"Iteration Title","startDate":"","duration":0,"iterationId":"iterationId"},"content":{"type":"DraftIssue","body":"a body","title":"Pull Request title","id":"draftIssueId"},"id":"draftIssueId"}],"totalCount":5}`,
string(out))
}

18
pkg/cmd/project/shared/queries/queries.go
View file

@ -235,10 +235,11 @@ type FieldValueNodes struct {
Field ProjectField
} `graphql:"... on ProjectV2ItemFieldDateValue"`
ProjectV2ItemFieldIterationValue struct {
Title string
StartDate string
Duration int
Field ProjectField
Title string
StartDate string
Duration int
Field ProjectField
IterationId string
} `graphql:"... on ProjectV2ItemFieldIterationValue"`
ProjectV2ItemFieldLabelValue struct {
Labels struct {
@ -249,7 +250,7 @@ type FieldValueNodes struct {
Field ProjectField
} `graphql:"... on ProjectV2ItemFieldLabelValue"`
ProjectV2ItemFieldNumberValue struct {
Number float32
Number float64
Field ProjectField
} `graphql:"... on ProjectV2ItemFieldNumberValue"`
ProjectV2ItemFieldSingleSelectValue struct {
@ -1488,9 +1489,10 @@ func projectFieldValueData(v FieldValueNodes) interface{} {
return v.ProjectV2ItemFieldDateValue.Date
case "ProjectV2ItemFieldIterationValue":
return map[string]interface{}{
"title": v.ProjectV2ItemFieldIterationValue.Title,
"startDate": v.ProjectV2ItemFieldIterationValue.StartDate,
"duration": v.ProjectV2ItemFieldIterationValue.Duration,
"title": v.ProjectV2ItemFieldIterationValue.Title,
"startDate": v.ProjectV2ItemFieldIterationValue.StartDate,
"duration": v.ProjectV2ItemFieldIterationValue.Duration,
"iterationId": v.ProjectV2ItemFieldIterationValue.IterationId,
}
case "ProjectV2ItemFieldNumberValue":
return v.ProjectV2ItemFieldNumberValue.Number

6
pkg/cmd/project/shared/queries/queries_test.go
View file

@ -395,8 +395,9 @@ func TestProjectItems_FieldTitle(t *testing.T) {
"fieldValues": map[string]interface{}{
"nodes": []map[string]interface{}{
{
"__typename": "ProjectV2ItemFieldIterationValue",
"title": "Iteration Title 1",
"__typename": "ProjectV2ItemFieldIterationValue",
"title": "Iteration Title 1",
"iterationId": "iterationId1",
},
{
"__typename": "ProjectV2ItemFieldMilestoneValue",
@ -426,6 +427,7 @@ func TestProjectItems_FieldTitle(t *testing.T) {
assert.Len(t, project.Items.Nodes, 1)
assert.Len(t, project.Items.Nodes[0].FieldValues.Nodes, 2)
assert.Equal(t, project.Items.Nodes[0].FieldValues.Nodes[0].ProjectV2ItemFieldIterationValue.Title, "Iteration Title 1")
assert.Equal(t, project.Items.Nodes[0].FieldValues.Nodes[0].ProjectV2ItemFieldIterationValue.IterationId, "iterationId1")
assert.Equal(t, project.Items.Nodes[0].FieldValues.Nodes[1].ProjectV2ItemFieldMilestoneValue.Milestone.Title, "Milestone Title 1")
}

22
pkg/cmd/release/create/create.go
View file

@ -60,6 +60,7 @@ type CreateOptions struct {
NotesStartTag string
VerifyTag bool
NotesFromTag bool
FailOnNoCommits bool
}
func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Command {
@ -99,6 +100,11 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
When using automatically generated release notes, a release title will also be automatically
generated unless a title was explicitly passed. Additional release notes can be prepended to
automatically generated notes by using the %[1]s--notes%[1]s flag.
By default, the release is created even if there are no new commits since the last release.
This may result in the same or duplicate release which may not be desirable in some cases.
Use %[1]s--fail-on-no-commits%[1]s to fail if no new commits are available. This flag has no
effect if there are no existing releases or this is the very first release.
`, "`"),
Example: heredoc.Doc(`
Interactively create a release
@ -120,7 +126,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
$ gh release create v1.2.3 --notes-from-tag
Don't mark the release as latest
$ gh release create v1.2.3 --latest=false
$ gh release create v1.2.3 --latest=false
Upload all tarballs in a directory as release assets
$ gh release create v1.2.3 ./dist/*.tgz
@ -130,6 +136,9 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
Create a release and start a discussion
$ gh release create v1.2.3 --discussion-category "General"
# Create a release only if there are new commits available since the last release
$ gh release create v1.2.3 --fail-on-no-commits
`),
Aliases: []string{"new"},
RunE: func(cmd *cobra.Command, args []string) error {
@ -194,6 +203,7 @@ func NewCmdCreate(f *cmdutil.Factory, runF func(*CreateOptions) error) *cobra.Co
cmdutil.NilBoolFlag(cmd, &opts.IsLatest, "latest", "", "Mark this release as \"Latest\" (default [automatic based on date and version]). --latest=false to explicitly NOT set as latest")
cmd.Flags().BoolVarP(&opts.VerifyTag, "verify-tag", "", false, "Abort in case the git tag doesn't already exist in the remote repository")
cmd.Flags().BoolVarP(&opts.NotesFromTag, "notes-from-tag", "", false, "Automatically generate notes from annotated tag")
cmd.Flags().BoolVar(&opts.FailOnNoCommits, "fail-on-no-commits", false, "Fail if there are no commits since the last release (no impact on the first release)")
_ = cmdutil.RegisterBranchCompletionFlags(f.GitClient, cmd, "target")
@ -211,6 +221,16 @@ func createRun(opts *CreateOptions) error {
return err
}
if opts.FailOnNoCommits {
isNew, err := isNewRelease(httpClient, baseRepo)
if err != nil {
return fmt.Errorf("failed to check whether there were new commits since last release: %v", err)
}
if !isNew {
return fmt.Errorf("no new commits since the last release")
}
}
var existingTag bool
if opts.TagName == "" {
tags, err := getTags(httpClient, baseRepo, 5)

154
pkg/cmd/release/create/create_test.go
View file

@ -52,17 +52,18 @@ func Test_NewCmdCreate(t *testing.T) {
args: "",
isTTY: true,
want: CreateOptions{
TagName: "",
Target: "",
Name: "",
Body: "",
BodyProvided: false,
Draft: false,
Prerelease: false,
RepoOverride: "",
Concurrency: 5,
Assets: []*shared.AssetForUpload(nil),
VerifyTag: false,
TagName: "",
Target: "",
Name: "",
Body: "",
BodyProvided: false,
Draft: false,
Prerelease: false,
RepoOverride: "",
Concurrency: 5,
Assets: []*shared.AssetForUpload(nil),
VerifyTag: false,
FailOnNoCommits: false,
},
},
{
@ -76,17 +77,18 @@ func Test_NewCmdCreate(t *testing.T) {
args: "v1.2.3",
isTTY: true,
want: CreateOptions{
TagName: "v1.2.3",
Target: "",
Name: "",
Body: "",
BodyProvided: false,
Draft: false,
Prerelease: false,
RepoOverride: "",
Concurrency: 5,
Assets: []*shared.AssetForUpload(nil),
VerifyTag: false,
TagName: "v1.2.3",
Target: "",
Name: "",
Body: "",
BodyProvided: false,
Draft: false,
Prerelease: false,
RepoOverride: "",
Concurrency: 5,
Assets: []*shared.AssetForUpload(nil),
VerifyTag: false,
FailOnNoCommits: false,
},
},
{
@ -347,6 +349,19 @@ func Test_NewCmdCreate(t *testing.T) {
isTTY: false,
wantErr: "using `--notes-from-tag` with `--generate-notes` or `--notes-start-tag` is not supported",
},
{
name: "with --fail-on-no-commits",
args: "v1.2.3 --fail-on-no-commits",
isTTY: false,
want: CreateOptions{
TagName: "v1.2.3",
BodyProvided: false,
Concurrency: 5,
Assets: []*shared.AssetForUpload(nil),
NotesFromTag: false,
FailOnNoCommits: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
@ -402,6 +417,7 @@ func Test_NewCmdCreate(t *testing.T) {
assert.Equal(t, tt.want.IsLatest, opts.IsLatest)
assert.Equal(t, tt.want.VerifyTag, opts.VerifyTag)
assert.Equal(t, tt.want.NotesFromTag, opts.NotesFromTag)
assert.Equal(t, tt.want.FailOnNoCommits, opts.FailOnNoCommits)
require.Equal(t, len(tt.want.Assets), len(opts.Assets))
for i := range tt.want.Assets {
@ -460,6 +476,100 @@ func Test_createRun(t *testing.T) {
wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n",
wantStderr: ``,
},
{
name: "create a release if there are new commits and the last release does not exist",
isTTY: true,
opts: CreateOptions{
TagName: "v1.2.3",
Name: "The Big 1.2",
Body: "* Fixed bugs",
BodyProvided: true,
Target: "",
FailOnNoCommits: true,
},
runStubs: defaultRunStubs,
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(404, `{
"message": "Not Found",
"documentation_url": "https://docs.github.com/rest/releases/releases#get-the-latest-release",
"status": "404"
}`))
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{
"url": "https://api.github.com/releases/123",
"upload_url": "https://api.github.com/assets/upload",
"html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3"
}`, func(params map[string]interface{}) {
assert.Equal(t, map[string]interface{}{
"tag_name": "v1.2.3",
"name": "The Big 1.2",
"body": "* Fixed bugs",
"draft": false,
"prerelease": false,
}, params)
}))
},
wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n",
wantStderr: ``,
},
{
name: "create a release if there are new commits and the last release exists",
isTTY: true,
opts: CreateOptions{
TagName: "v1.2.3",
Name: "The Big 1.2",
Body: "* Fixed bugs",
BodyProvided: true,
Target: "",
FailOnNoCommits: true,
},
runStubs: defaultRunStubs,
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{
"tag_name": "v1.2.2"
}`))
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{
"status": "ahead"
}`))
reg.Register(httpmock.REST("POST", "repos/OWNER/REPO/releases"), httpmock.RESTPayload(201, `{
"url": "https://api.github.com/releases/123",
"upload_url": "https://api.github.com/assets/upload",
"html_url": "https://github.com/OWNER/REPO/releases/tag/v1.2.3"
}`, func(params map[string]interface{}) {
assert.Equal(t, map[string]interface{}{
"tag_name": "v1.2.3",
"name": "The Big 1.2",
"body": "* Fixed bugs",
"draft": false,
"prerelease": false,
}, params)
}))
},
wantStdout: "https://github.com/OWNER/REPO/releases/tag/v1.2.3\n",
wantStderr: ``,
},
{
name: "create a release if there are no new commits but the last release exists",
isTTY: true,
opts: CreateOptions{
TagName: "v1.2.3",
Name: "The Big 1.2",
Body: "* Fixed bugs",
BodyProvided: true,
Target: "",
FailOnNoCommits: true,
},
httpStubs: func(t *testing.T, reg *httpmock.Registry) {
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/releases/latest"), httpmock.StatusStringResponse(200, `{
"tag_name": "v1.2.2"
}`))
reg.Register(httpmock.REST("GET", "repos/OWNER/REPO/compare/v1.2.2...HEAD"), httpmock.StatusStringResponse(200, `{
"status": "identical"
}`))
},
wantErr: "no new commits since the last release",
wantStdout: "",
wantStderr: ``,
},
{
name: "with discussion category",
isTTY: true,

29
pkg/cmd/release/create/http.go
View file

@ -2,6 +2,7 @@ package create
import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
@ -299,3 +300,31 @@ func tokenHasWorkflowScope(resp *http.Response) bool {
return slices.Contains(strings.Split(scopes, ","), "workflow")
}
// isNewRelease checks if there are new commits since the latest release.
func isNewRelease(httpClient *http.Client, repo ghrepo.Interface) (bool, error) {
ctx := context.Background()
release, err := shared.FetchLatestRelease(ctx, httpClient, repo)
if err != nil {
if errors.Is(err, shared.ErrReleaseNotFound) {
return true, nil
} else {
return false, err
}
}
tagName := release.TagName
path := fmt.Sprintf("repos/%s/%s/compare/%s...HEAD?per_page=1", repo.RepoOwner(), repo.RepoName(), tagName)
var comparisonStatus struct {
Status string `json:"status"`
}
apiClient := api.NewClientFromHTTP(httpClient)
if err := apiClient.REST(repo.RepoHost(), "GET", path, nil, &comparisonStatus); err != nil {
return false, err
}
isNew := comparisonStatus.Status == "ahead"
return isNew, nil
}

8
pkg/cmd/release/shared/fetch.go
View file

@ -124,7 +124,7 @@ func (rel *Release) ExportData(fields []string) map[string]interface{} {
return data
}
var errNotFound = errors.New("release not found")
var ErrReleaseNotFound = errors.New("release not found")
type fetchResult struct {
release *Release
@ -150,7 +150,7 @@ func FetchRelease(ctx context.Context, httpClient *http.Client, repo ghrepo.Inte
}()
res := <-results
if errors.Is(res.error, errNotFound) {
if errors.Is(res.error, ErrReleaseNotFound) {
res = <-results
cancel() // satisfy the linter even though no goroutines are running anymore
} else {
@ -190,7 +190,7 @@ func fetchDraftRelease(ctx context.Context, httpClient *http.Client, repo ghrepo
}
if query.Repository.Release == nil || !query.Repository.Release.IsDraft {
return nil, errNotFound
return nil, ErrReleaseNotFound
}
// Then, use REST to get information about the draft release. In theory, we could have fetched
@ -213,7 +213,7 @@ func fetchReleasePath(ctx context.Context, httpClient *http.Client, host string,
if resp.StatusCode == 404 {
_, _ = io.Copy(io.Discard, resp.Body)
return nil, errNotFound
return nil, ErrReleaseNotFound
} else if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}

16
pkg/cmd/release/view/view_test.go
View file

@ -144,15 +144,15 @@ func Test_viewRun(t *testing.T) {
wantStdout: heredoc.Doc(`
v1.2.3
MonaLisa released this about 1 day ago
• Fixed bugs
Assets
windows.zip 12 B
linux.tgz 34 B
View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3
`),
wantStderr: ``,
@ -168,15 +168,15 @@ func Test_viewRun(t *testing.T) {
wantStdout: heredoc.Doc(`
v1.2.3
MonaLisa released this about 1 day ago
• Fixed bugs
Assets
windows.zip 12 B
linux.tgz 34 B
View on GitHub: https://github.com/OWNER/REPO/releases/tags/v1.2.3
`),
wantStderr: ``,

20
pkg/cmd/repo/autolink/autolink.go
View file

@ -2,7 +2,10 @@ package autolink
import (
"github.com/MakeNowJust/heredoc"
cmdCreate "github.com/cli/cli/v2/pkg/cmd/repo/autolink/create"
cmdDelete "github.com/cli/cli/v2/pkg/cmd/repo/autolink/delete"
cmdList "github.com/cli/cli/v2/pkg/cmd/repo/autolink/list"
cmdView "github.com/cli/cli/v2/pkg/cmd/repo/autolink/view"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/spf13/cobra"
)
@ -12,18 +15,19 @@ func NewCmdAutolink(f *cmdutil.Factory) *cobra.Command {
Use: "autolink <command>",
Short: "Manage autolink references",
Long: heredoc.Docf(`
Work with GitHub autolink references.
GitHub autolinks require admin access to configure and can be found at
https://github.com/{owner}/{repo}/settings/key_links.
Use %[1]sgh repo autolink list --web%[1]s to open this page for the current repository.
For more information about GitHub autolinks, see https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-autolinks-to-reference-external-resources
`, "`"),
Autolinks link issues, pull requests, commit messages, and release descriptions to external third-party services.
Autolinks require %[1]sadmin%[1]s role to view or manage.
For more information, see <https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-autolinks-to-reference-external-resources>
`, "`"),
}
cmdutil.EnableRepoOverride(cmd, f)
cmd.AddCommand(cmdList.NewCmdList(f, nil))
cmd.AddCommand(cmdCreate.NewCmdCreate(f, nil))
cmd.AddCommand(cmdView.NewCmdView(f, nil))
cmd.AddCommand(cmdDelete.NewCmdDelete(f, nil))
return cmd
}

118
pkg/cmd/repo/autolink/create/create.go Normal file
View file

@ -0,0 +1,118 @@
package create
import (
"fmt"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
type createOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
AutolinkClient AutolinkCreateClient
IO *iostreams.IOStreams
Exporter cmdutil.Exporter
KeyPrefix string
URLTemplate string
Numeric bool
}
type AutolinkCreateClient interface {
Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error)
}
func NewCmdCreate(f *cmdutil.Factory, runF func(*createOptions) error) *cobra.Command {
opts := &createOptions{
Browser: f.Browser,
IO: f.IOStreams,
}
cmd := &cobra.Command{
Use: "create <keyPrefix> <urlTemplate>",
Short: "Create a new autolink reference",
Long: heredoc.Docf(`
Create a new autolink reference for a repository.
The %[1]skeyPrefix%[1]s argument specifies the prefix that will generate a link when it is appended by certain characters.
The %[1]surlTemplate%[1]s argument specifies the target URL that will be generated when the keyPrefix is found, which
must contain %[1]s<num>%[1]s variable for the reference number.
By default, autolinks are alphanumeric with %[1]s--numeric%[1]s flag used to create a numeric autolink.
The %[1]s<num>%[1]s variable behavior differs depending on whether the autolink is alphanumeric or numeric:
- alphanumeric: matches %[1]sA-Z%[1]s (case insensitive), %[1]s0-9%[1]s, and %[1]s-%[1]s
- numeric: matches %[1]s0-9%[1]s
If the template contains multiple instances of %[1]s<num>%[1]s, only the first will be replaced.
`, "`"),
Example: heredoc.Doc(`
# Create an alphanumeric autolink to example.com for the key prefix "TICKET-".
# Generates https://example.com/TICKET?query=123abc from "TICKET-123abc".
$ gh repo autolink create TICKET- "https://example.com/TICKET?query=<num>"
# Create a numeric autolink to example.com for the key prefix "STORY-".
# Generates https://example.com/STORY?id=123 from "STORY-123".
$ gh repo autolink create STORY- "https://example.com/STORY?id=<num>" --numeric
`),
Args: cmdutil.ExactArgs(2, "Cannot create autolink: keyPrefix and urlTemplate arguments are both required"),
Aliases: []string{"new"},
RunE: func(c *cobra.Command, args []string) error {
opts.BaseRepo = f.BaseRepo
httpClient, err := f.HttpClient()
if err != nil {
return err
}
opts.AutolinkClient = &AutolinkCreator{HTTPClient: httpClient}
opts.KeyPrefix = args[0]
opts.URLTemplate = args[1]
if runF != nil {
return runF(opts)
}
return createRun(opts)
},
}
cmd.Flags().BoolVarP(&opts.Numeric, "numeric", "n", false, "Mark autolink as numeric")
return cmd
}
func createRun(opts *createOptions) error {
repo, err := opts.BaseRepo()
if err != nil {
return err
}
request := AutolinkCreateRequest{
KeyPrefix: opts.KeyPrefix,
URLTemplate: opts.URLTemplate,
IsAlphanumeric: !opts.Numeric,
}
autolink, err := opts.AutolinkClient.Create(repo, request)
if err != nil {
return fmt.Errorf("error creating autolink: %w", err)
}
cs := opts.IO.ColorScheme()
fmt.Fprintf(opts.IO.Out,
"%s Created repository autolink %s on %s\n",
cs.SuccessIconWithColor(cs.Green),
cs.Cyanf("%d", autolink.ID),
cs.Bold(ghrepo.FullName(repo)))
return nil
}

190
pkg/cmd/repo/autolink/create/create_test.go Normal file
View file

@ -0,0 +1,190 @@
package create
import (
"bytes"
"fmt"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdCreate(t *testing.T) {
tests := []struct {
name string
input string
output createOptions
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
wantErr: true,
errMsg: "Cannot create autolink: keyPrefix and urlTemplate arguments are both required",
},
{
name: "one argument",
input: "TEST-",
wantErr: true,
errMsg: "Cannot create autolink: keyPrefix and urlTemplate arguments are both required",
},
{
name: "two argument",
input: "TICKET- https://example.com/TICKET?query=<num>",
output: createOptions{
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
},
{
name: "numeric flag",
input: "TICKET- https://example.com/TICKET?query=<num> --numeric",
output: createOptions{
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
Numeric: true,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: ios,
}
f.HttpClient = func() (*http.Client, error) {
return &http.Client{}, nil
}
argv, err := shlex.Split(tt.input)
require.NoError(t, err)
var gotOpts *createOptions
cmd := NewCmdCreate(f, func(opts *createOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
require.EqualError(t, err, tt.errMsg)
} else {
require.NoError(t, err)
assert.Equal(t, tt.output.KeyPrefix, gotOpts.KeyPrefix)
assert.Equal(t, tt.output.URLTemplate, gotOpts.URLTemplate)
assert.Equal(t, tt.output.Numeric, gotOpts.Numeric)
}
})
}
}
type stubAutolinkCreator struct {
err error
}
func (g stubAutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error) {
if g.err != nil {
return nil, g.err
}
return &shared.Autolink{
ID: 1,
KeyPrefix: request.KeyPrefix,
URLTemplate: request.URLTemplate,
IsAlphanumeric: request.IsAlphanumeric,
}, nil
}
type testAutolinkClientCreateError struct{}
func (e testAutolinkClientCreateError) Error() string {
return "autolink client create error"
}
func TestCreateRun(t *testing.T) {
tests := []struct {
name string
opts *createOptions
stubCreator stubAutolinkCreator
expectedErr error
errMsg string
wantStdout string
wantStderr string
}{
{
name: "success, alphanumeric",
opts: &createOptions{
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
stubCreator: stubAutolinkCreator{},
wantStdout: "✓ Created repository autolink 1 on OWNER/REPO\n",
},
{
name: "success, numeric",
opts: &createOptions{
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
Numeric: true,
},
stubCreator: stubAutolinkCreator{},
wantStdout: "✓ Created repository autolink 1 on OWNER/REPO\n",
},
{
name: "client error",
opts: &createOptions{
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
stubCreator: stubAutolinkCreator{err: testAutolinkClientCreateError{}},
expectedErr: testAutolinkClientCreateError{},
errMsg: fmt.Sprint("error creating autolink: ", testAutolinkClientCreateError{}.Error()),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, stderr := iostreams.Test()
opts := tt.opts
opts.IO = ios
opts.Browser = &browser.Stub{}
opts.IO = ios
opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }
opts.AutolinkClient = &tt.stubCreator
err := createRun(opts)
if tt.expectedErr != nil {
require.Error(t, err)
assert.ErrorIs(t, err, tt.expectedErr)
assert.Equal(t, tt.errMsg, err.Error())
} else {
require.NoError(t, err)
}
if tt.wantStdout != "" {
assert.Equal(t, tt.wantStdout, stdout.String())
}
if tt.wantStderr != "" {
assert.Equal(t, tt.wantStderr, stderr.String())
}
})
}
}

79
pkg/cmd/repo/autolink/create/http.go Normal file
View file

@ -0,0 +1,79 @@
package create
import (
"bytes"
"encoding/json"
"errors"
"fmt"
"net/http"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
)
type AutolinkCreator struct {
HTTPClient *http.Client
}
type AutolinkCreateRequest struct {
IsAlphanumeric bool `json:"is_alphanumeric"`
KeyPrefix string `json:"key_prefix"`
URLTemplate string `json:"url_template"`
}
func (a *AutolinkCreator) Create(repo ghrepo.Interface, request AutolinkCreateRequest) (*shared.Autolink, error) {
path := fmt.Sprintf("repos/%s/%s/autolinks", repo.RepoOwner(), repo.RepoName())
url := ghinstance.RESTPrefix(repo.RepoHost()) + path
requestByte, err := json.Marshal(request)
if err != nil {
return nil, err
}
requestBody := bytes.NewReader(requestByte)
req, err := http.NewRequest(http.MethodPost, url, requestBody)
if err != nil {
return nil, err
}
resp, err := a.HTTPClient.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()
err = handleAutolinkCreateError(resp)
if err != nil {
return nil, err
}
var autolink shared.Autolink
err = json.NewDecoder(resp.Body).Decode(&autolink)
if err != nil {
return nil, err
}
return &autolink, nil
}
func handleAutolinkCreateError(resp *http.Response) error {
switch resp.StatusCode {
case http.StatusCreated:
return nil
case http.StatusNotFound:
err := api.HandleHTTPError(resp)
var httpErr api.HTTPError
if errors.As(err, &httpErr) {
httpErr.Message = "Must have admin rights to Repository."
return httpErr
}
return err
default:
return api.HandleHTTPError(resp)
}
}

155
pkg/cmd/repo/autolink/create/http_test.go Normal file
View file

@ -0,0 +1,155 @@
package create
import (
"fmt"
"net/http"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAutolinkCreator_Create(t *testing.T) {
repo := ghrepo.New("OWNER", "REPO")
tests := []struct {
name string
req AutolinkCreateRequest
stubStatus int
stubRespJSON string
expectedAutolink *shared.Autolink
expectErr bool
expectedErrMsg string
}{
{
name: "201 successful creation",
req: AutolinkCreateRequest{
IsAlphanumeric: true,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
stubStatus: http.StatusCreated,
stubRespJSON: `{
"id": 1,
"is_alphanumeric": true,
"key_prefix": "TICKET-",
"url_template": "https://example.com/TICKET?query=<num>"
}`,
expectedAutolink: &shared.Autolink{
ID: 1,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
{
name: "422 URL template not valid URL",
req: AutolinkCreateRequest{
IsAlphanumeric: true,
KeyPrefix: "TICKET-",
URLTemplate: "foo/<num>",
},
stubStatus: http.StatusUnprocessableEntity,
stubRespJSON: `{
"message": "Validation Failed",
"errors": [
{
"resource": "KeyLink",
"code": "custom",
"field": "url_template",
"message": "url_template must be an absolute URL"
}
],
"documentation_url": "https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository",
"status": "422"
}`,
expectErr: true,
expectedErrMsg: heredoc.Doc(`
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
url_template must be an absolute URL`),
},
{
name: "404 repo not found",
req: AutolinkCreateRequest{
IsAlphanumeric: true,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
stubStatus: http.StatusNotFound,
stubRespJSON: `{
"message": "Not Found",
"documentation_url": "https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository",
"status": "404"
}`,
expectErr: true,
expectedErrMsg: "HTTP 404: Must have admin rights to Repository. (https://api.github.com/repos/OWNER/REPO/autolinks)",
},
{
name: "422 URL template missing <num>",
req: AutolinkCreateRequest{
IsAlphanumeric: true,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET",
},
stubStatus: http.StatusUnprocessableEntity,
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"custom","field":"url_template","message":"url_template is missing a <num> token"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
expectErr: true,
expectedErrMsg: heredoc.Doc(`
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
url_template is missing a <num> token`),
},
{
name: "422 already exists",
req: AutolinkCreateRequest{
IsAlphanumeric: true,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
},
stubStatus: http.StatusUnprocessableEntity,
stubRespJSON: `{"message":"Validation Failed","errors":[{"resource":"KeyLink","code":"already_exists","field":"key_prefix"}],"documentation_url":"https://docs.github.com/rest/repos/autolinks#create-an-autolink-reference-for-a-repository","status":"422"}`,
expectErr: true,
expectedErrMsg: heredoc.Doc(`
HTTP 422: Validation Failed (https://api.github.com/repos/OWNER/REPO/autolinks)
KeyLink.key_prefix already exists`),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST(
http.MethodPost,
fmt.Sprintf("repos/%s/%s/autolinks", repo.RepoOwner(), repo.RepoName())),
httpmock.RESTPayload(tt.stubStatus, tt.stubRespJSON,
func(payload map[string]interface{}) {
require.Equal(t, map[string]interface{}{
"is_alphanumeric": tt.req.IsAlphanumeric,
"key_prefix": tt.req.KeyPrefix,
"url_template": tt.req.URLTemplate,
}, payload)
},
),
)
defer reg.Verify(t)
autolinkCreator := &AutolinkCreator{
HTTPClient: &http.Client{Transport: reg},
}
autolink, err := autolinkCreator.Create(repo, tt.req)
if tt.expectErr {
require.EqualError(t, err, tt.expectedErrMsg)
} else {
require.NoError(t, err)
assert.Equal(t, tt.expectedAutolink, autolink)
}
})
}
}

107
pkg/cmd/repo/autolink/delete/delete.go Normal file
View file

@ -0,0 +1,107 @@
package delete
import (
"fmt"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/view"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
type deleteOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
AutolinkDeleteClient AutolinkDeleteClient
AutolinkViewClient view.AutolinkViewClient
IO *iostreams.IOStreams
ID string
Confirmed bool
Prompter prompter.Prompter
}
type AutolinkDeleteClient interface {
Delete(repo ghrepo.Interface, id string) error
}
func NewCmdDelete(f *cmdutil.Factory, runF func(*deleteOptions) error) *cobra.Command {
opts := &deleteOptions{
Browser: f.Browser,
IO: f.IOStreams,
Prompter: f.Prompter,
}
cmd := &cobra.Command{
Use: "delete <id>",
Short: "Delete an autolink reference",
Long: "Delete an autolink reference for a repository.",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.BaseRepo = f.BaseRepo
httpClient, err := f.HttpClient()
if err != nil {
return err
}
opts.AutolinkDeleteClient = &AutolinkDeleter{HTTPClient: httpClient}
opts.AutolinkViewClient = &view.AutolinkViewer{HTTPClient: httpClient}
opts.ID = args[0]
if !opts.IO.CanPrompt() && !opts.Confirmed {
return cmdutil.FlagErrorf("--yes required when not running interactively")
}
if runF != nil {
return runF(opts)
}
return deleteRun(opts)
},
}
cmd.Flags().BoolVar(&opts.Confirmed, "yes", false, "Confirm deletion without prompting")
return cmd
}
func deleteRun(opts *deleteOptions) error {
repo, err := opts.BaseRepo()
if err != nil {
return err
}
out := opts.IO.Out
cs := opts.IO.ColorScheme()
autolink, err := opts.AutolinkViewClient.View(repo, opts.ID)
if err != nil {
return fmt.Errorf("%s %w", cs.Red("error deleting autolink:"), err)
}
if opts.IO.CanPrompt() && !opts.Confirmed {
fmt.Fprintf(out, "Autolink %s has key prefix %s.\n", cs.Cyan(opts.ID), autolink.KeyPrefix)
err := opts.Prompter.ConfirmDeletion(autolink.KeyPrefix)
if err != nil {
return err
}
}
err = opts.AutolinkDeleteClient.Delete(repo, opts.ID)
if err != nil {
return err
}
if opts.IO.IsStdoutTTY() {
fmt.Fprintf(out, "%s Autolink %s deleted from %s\n", cs.SuccessIcon(), cs.Cyan(opts.ID), cs.Bold(ghrepo.FullName(repo)))
}
return nil
}

335
pkg/cmd/repo/autolink/delete/delete_test.go Normal file
View file

@ -0,0 +1,335 @@
package delete
import (
"bytes"
"errors"
"net/http"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/prompter"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestNewCmdDelete(t *testing.T) {
tests := []struct {
name string
input string
output deleteOptions
isTTY bool
wantErr bool
errMsg string
}{
{
name: "no argument",
input: "",
isTTY: true,
wantErr: true,
errMsg: "accepts 1 arg(s), received 0",
},
{
name: "id provided",
input: "123",
isTTY: true,
output: deleteOptions{ID: "123"},
},
{
name: "yes flag",
input: "123 --yes",
isTTY: true,
output: deleteOptions{ID: "123", Confirmed: true},
},
{
name: "non-TTY",
input: "123 --yes",
isTTY: false,
output: deleteOptions{ID: "123", Confirmed: true},
},
{
name: "non-TTY missing yes flag",
input: "123",
isTTY: false,
wantErr: true,
errMsg: "--yes required when not running interactively",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
ios.SetStdinTTY(tt.isTTY)
ios.SetStdoutTTY(tt.isTTY)
ios.SetStderrTTY(tt.isTTY)
f := &cmdutil.Factory{
IOStreams: ios,
}
f.HttpClient = func() (*http.Client, error) {
return &http.Client{}, nil
}
argv, err := shlex.Split(tt.input)
require.NoError(t, err)
var gotOpts *deleteOptions
cmd := NewCmdDelete(f, func(opts *deleteOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
require.EqualError(t, err, tt.errMsg)
} else {
require.NoError(t, err)
assert.Equal(t, tt.output.ID, gotOpts.ID)
assert.Equal(t, tt.output.Confirmed, gotOpts.Confirmed)
}
})
}
}
type stubAutolinkDeleter struct {
err error
}
func (d *stubAutolinkDeleter) Delete(repo ghrepo.Interface, id string) error {
return d.err
}
type stubAutolinkViewer struct {
autolink *shared.Autolink
err error
}
func (g stubAutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) {
return g.autolink, g.err
}
var errTestPrompt = errors.New("prompt error")
var errTestAutolinkClientView = errors.New("autolink client view error")
var errTestAutolinkClientDelete = errors.New("autolink client delete error")
func TestDeleteRun(t *testing.T) {
tests := []struct {
name string
opts *deleteOptions
isTTY bool
stubDeleter stubAutolinkDeleter
stubViewer stubAutolinkViewer
prompterStubs func(*prompter.PrompterMock)
wantStdout string
expectedErr error
expectedErrMsg string
}{
{
name: "delete",
opts: &deleteOptions{
ID: "123",
},
isTTY: true,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
stubDeleter: stubAutolinkDeleter{
err: nil,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.ConfirmDeletionFunc = func(_ string) error {
return nil
}
},
wantStdout: heredoc.Doc(`
Autolink 123 has key prefix TICKET-.
✓ Autolink 123 deleted from OWNER/REPO
`),
},
{
name: "delete with confirm flag",
opts: &deleteOptions{
ID: "123",
Confirmed: true,
},
isTTY: true,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
stubDeleter: stubAutolinkDeleter{},
wantStdout: "✓ Autolink 123 deleted from OWNER/REPO\n",
},
{
name: "confirmation fails",
opts: &deleteOptions{
ID: "123",
},
isTTY: true,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
stubDeleter: stubAutolinkDeleter{},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.ConfirmDeletionFunc = func(_ string) error {
return errTestPrompt
}
},
expectedErr: errTestPrompt,
expectedErrMsg: errTestPrompt.Error(),
wantStdout: "Autolink 123 has key prefix TICKET-.\n",
},
{
name: "view error",
opts: &deleteOptions{
ID: "123",
},
isTTY: true,
stubViewer: stubAutolinkViewer{
err: errTestAutolinkClientView,
},
stubDeleter: stubAutolinkDeleter{},
expectedErr: errTestAutolinkClientView,
expectedErrMsg: "error deleting autolink: autolink client view error",
},
{
name: "delete error",
opts: &deleteOptions{
ID: "123",
},
isTTY: true,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
stubDeleter: stubAutolinkDeleter{
err: errTestAutolinkClientDelete,
},
prompterStubs: func(pm *prompter.PrompterMock) {
pm.ConfirmDeletionFunc = func(_ string) error {
return nil
}
},
expectedErr: errTestAutolinkClientDelete,
expectedErrMsg: errTestAutolinkClientDelete.Error(),
wantStdout: "Autolink 123 has key prefix TICKET-.\n",
},
{
name: "no TTY",
opts: &deleteOptions{
ID: "123",
Confirmed: true,
},
isTTY: false,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
}},
stubDeleter: stubAutolinkDeleter{},
wantStdout: "",
},
{
name: "no TTY view error",
opts: &deleteOptions{
ID: "123",
},
isTTY: false,
stubViewer: stubAutolinkViewer{
err: errTestAutolinkClientView,
},
stubDeleter: stubAutolinkDeleter{},
expectedErr: errTestAutolinkClientView,
expectedErrMsg: "error deleting autolink: autolink client view error",
},
{
name: "no TTY delete error",
opts: &deleteOptions{
ID: "123",
Confirmed: true,
},
isTTY: false,
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
stubDeleter: stubAutolinkDeleter{
err: errTestAutolinkClientDelete,
},
expectedErr: errTestAutolinkClientDelete,
expectedErrMsg: errTestAutolinkClientDelete.Error(),
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
ios.SetStdinTTY(tt.isTTY)
ios.SetStdoutTTY(tt.isTTY)
opts := tt.opts
opts.IO = ios
opts.Browser = &browser.Stub{}
opts.IO = ios
opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }
opts.AutolinkDeleteClient = &tt.stubDeleter
opts.AutolinkViewClient = &tt.stubViewer
pm := &prompter.PrompterMock{}
if tt.prompterStubs != nil {
tt.prompterStubs(pm)
}
tt.opts.Prompter = pm
err := deleteRun(opts)
if tt.expectedErr != nil {
require.Error(t, err, "expected error but got none")
assert.ErrorIs(t, err, tt.expectedErr, "unexpected error")
assert.Equal(t, tt.expectedErrMsg, err.Error(), "unexpected error message")
} else {
require.NoError(t, err)
}
assert.Equal(t, tt.wantStdout, stdout.String(), "unexpected stdout")
})
}
}

37
pkg/cmd/repo/autolink/delete/http.go Normal file
View file

@ -0,0 +1,37 @@
package delete
import (
"fmt"
"net/http"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
)
type AutolinkDeleter struct {
HTTPClient *http.Client
}
func (a *AutolinkDeleter) Delete(repo ghrepo.Interface, id string) error {
path := fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), id)
url := ghinstance.RESTPrefix(repo.RepoHost()) + path
req, err := http.NewRequest(http.MethodDelete, url, nil)
if err != nil {
return err
}
resp, err := a.HTTPClient.Do(req)
if err != nil {
return err
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusNotFound {
return fmt.Errorf("error deleting autolink: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/%s)", path)
} else if resp.StatusCode > 299 {
return api.HandleHTTPError(resp)
}
return nil
}

73
pkg/cmd/repo/autolink/delete/http_test.go Normal file
View file

@ -0,0 +1,73 @@
package delete
import (
"fmt"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/require"
)
func TestAutolinkDeleter_Delete(t *testing.T) {
repo := ghrepo.New("OWNER", "REPO")
tests := []struct {
name string
id string
stubStatus int
stubRespJSON string
expectErr bool
expectedErrMsg string
}{
{
name: "204 successful delete",
id: "123",
stubStatus: http.StatusNoContent,
},
{
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"}`,
stubStatus: http.StatusInternalServerError,
expectErr: true,
expectedErrMsg: "HTTP 500 (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST(
http.MethodDelete,
fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id),
),
httpmock.StatusJSONResponse(tt.stubStatus, tt.stubRespJSON),
)
defer reg.Verify(t)
autolinkDeleter := &AutolinkDeleter{
HTTPClient: &http.Client{Transport: reg},
}
err := autolinkDeleter.Delete(repo, tt.id)
if tt.expectErr {
require.EqualError(t, err, tt.expectedErrMsg)
} else {
require.NoError(t, err)
}
})
}
}

7
pkg/cmd/repo/autolink/list/http.go
View file

@ -8,16 +8,17 @@ import (
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
)
type AutolinkLister struct {
HTTPClient *http.Client
}
func (a *AutolinkLister) List(repo ghrepo.Interface) ([]autolink, error) {
func (a *AutolinkLister) List(repo ghrepo.Interface) ([]shared.Autolink, error) {
path := fmt.Sprintf("repos/%s/%s/autolinks", repo.RepoOwner(), repo.RepoName())
url := ghinstance.RESTPrefix(repo.RepoHost()) + path
req, err := http.NewRequest("GET", url, nil)
req, err := http.NewRequest(http.MethodGet, url, nil)
if err != nil {
return nil, err
}
@ -33,7 +34,7 @@ func (a *AutolinkLister) List(repo ghrepo.Interface) ([]autolink, error) {
} else if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}
var autolinks []autolink
var autolinks []shared.Autolink
err = json.NewDecoder(resp.Body).Decode(&autolinks)
if err != nil {
return nil, err

19
pkg/cmd/repo/autolink/list/http_test.go
View file

@ -6,28 +6,29 @@ import (
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAutoLinkLister_List(t *testing.T) {
func TestAutolinkLister_List(t *testing.T) {
tests := []struct {
name string
repo ghrepo.Interface
resp []autolink
resp []shared.Autolink
status int
}{
{
name: "no autolinks",
repo: ghrepo.New("OWNER", "REPO"),
resp: []autolink{},
status: 200,
resp: []shared.Autolink{},
status: http.StatusOK,
},
{
name: "two autolinks",
repo: ghrepo.New("OWNER", "REPO"),
resp: []autolink{
resp: []shared.Autolink{
{
ID: 1,
IsAlphanumeric: true,
@ -41,12 +42,12 @@ func TestAutoLinkLister_List(t *testing.T) {
URLTemplate: "https://example2.com",
},
},
status: 200,
status: http.StatusOK,
},
{
name: "http error",
repo: ghrepo.New("OWNER", "REPO"),
status: 404,
status: http.StatusNotFound,
},
}
@ -54,7 +55,7 @@ func TestAutoLinkLister_List(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST("GET", fmt.Sprintf("repos/%s/%s/autolinks", tt.repo.RepoOwner(), tt.repo.RepoName())),
httpmock.REST(http.MethodGet, fmt.Sprintf("repos/%s/%s/autolinks", tt.repo.RepoOwner(), tt.repo.RepoName())),
httpmock.StatusJSONResponse(tt.status, tt.resp),
)
defer reg.Verify(t)
@ -63,7 +64,7 @@ func TestAutoLinkLister_List(t *testing.T) {
HTTPClient: &http.Client{Transport: reg},
}
autolinks, err := autolinkLister.List(tt.repo)
if tt.status == 404 {
if tt.status == http.StatusNotFound {
require.Error(t, err)
assert.Equal(t, "error getting autolinks: HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks)", err.Error())
} else {

47
pkg/cmd/repo/autolink/list/list.go
View file

@ -9,41 +9,24 @@ import (
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/internal/tableprinter"
"github.com/cli/cli/v2/internal/text"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
var autolinkFields = []string{
"id",
"isAlphanumeric",
"keyPrefix",
"urlTemplate",
}
type autolink struct {
ID int `json:"id"`
IsAlphanumeric bool `json:"is_alphanumeric"`
KeyPrefix string `json:"key_prefix"`
URLTemplate string `json:"url_template"`
}
func (s *autolink) ExportData(fields []string) map[string]interface{} {
return cmdutil.StructExportData(s, fields)
}
type listOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
AutolinkClient AutolinkClient
AutolinkClient AutolinkListClient
IO *iostreams.IOStreams
Exporter cmdutil.Exporter
WebMode bool
}
type AutolinkClient interface {
List(repo ghrepo.Interface) ([]autolink, error)
type AutolinkListClient interface {
List(repo ghrepo.Interface) ([]shared.Autolink, error)
}
func NewCmdList(f *cmdutil.Factory, runF func(*listOptions) error) *cobra.Command {
@ -80,7 +63,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*listOptions) error) *cobra.Comman
}
cmd.Flags().BoolVarP(&opts.WebMode, "web", "w", false, "List autolink references in the web browser")
cmdutil.AddJSONFlags(cmd, &opts.Exporter, autolinkFields)
cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.AutolinkFields)
return cmd
}
@ -106,8 +89,14 @@ func listRun(opts *listOptions) error {
return err
}
cs := opts.IO.ColorScheme()
if len(autolinks) == 0 {
return cmdutil.NewNoResultsError(fmt.Sprintf("no autolinks found in %s", ghrepo.FullName(repo)))
return cmdutil.NewNoResultsError(
fmt.Sprintf(
"no autolinks found in %s",
cs.Bold(ghrepo.FullName(repo))),
)
}
if opts.Exporter != nil {
@ -115,14 +104,18 @@ func listRun(opts *listOptions) error {
}
if opts.IO.IsStdoutTTY() {
title := listHeader(ghrepo.FullName(repo), len(autolinks))
title := fmt.Sprintf(
"Showing %s in %s",
text.Pluralize(len(autolinks), "autolink reference"),
cs.Bold(ghrepo.FullName(repo)),
)
fmt.Fprintf(opts.IO.Out, "\n%s\n\n", title)
}
tp := tableprinter.New(opts.IO, tableprinter.WithHeader("ID", "KEY PREFIX", "URL TEMPLATE", "ALPHANUMERIC"))
for _, autolink := range autolinks {
tp.AddField(fmt.Sprintf("%d", autolink.ID))
tp.AddField(cs.Cyanf("%d", autolink.ID))
tp.AddField(autolink.KeyPrefix)
tp.AddField(autolink.URLTemplate)
tp.AddField(strconv.FormatBool(autolink.IsAlphanumeric))
@ -131,7 +124,3 @@ func listRun(opts *listOptions) error {
return tp.Render()
}
func listHeader(repoName string, count int) string {
return fmt.Sprintf("Showing %s in %s", text.Pluralize(count, "autolink reference"), repoName)
}

29
pkg/cmd/repo/autolink/list/list_test.go
View file

@ -8,6 +8,7 @@ import (
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/pkg/jsonfieldstest"
@ -95,12 +96,12 @@ func TestNewCmdList(t *testing.T) {
}
}
type stubAutoLinkLister struct {
autolinks []autolink
type stubAutolinkLister struct {
autolinks []shared.Autolink
err error
}
func (g stubAutoLinkLister) List(repo ghrepo.Interface) ([]autolink, error) {
func (g stubAutolinkLister) List(repo ghrepo.Interface) ([]shared.Autolink, error) {
return g.autolinks, g.err
}
@ -115,7 +116,7 @@ func TestListRun(t *testing.T) {
name string
opts *listOptions
isTTY bool
stubLister stubAutoLinkLister
stubLister stubAutolinkLister
expectedErr error
wantStdout string
wantStderr string
@ -124,8 +125,8 @@ func TestListRun(t *testing.T) {
name: "list tty",
opts: &listOptions{},
isTTY: true,
stubLister: stubAutoLinkLister{
autolinks: []autolink{
stubLister: stubAutolinkLister{
autolinks: []shared.Autolink{
{
ID: 1,
KeyPrefix: "TICKET-",
@ -160,8 +161,8 @@ func TestListRun(t *testing.T) {
}(),
},
isTTY: true,
stubLister: stubAutoLinkLister{
autolinks: []autolink{
stubLister: stubAutolinkLister{
autolinks: []shared.Autolink{
{
ID: 1,
KeyPrefix: "TICKET-",
@ -183,8 +184,8 @@ func TestListRun(t *testing.T) {
name: "list non-tty",
opts: &listOptions{},
isTTY: false,
stubLister: stubAutoLinkLister{
autolinks: []autolink{
stubLister: stubAutolinkLister{
autolinks: []shared.Autolink{
{
ID: 1,
KeyPrefix: "TICKET-",
@ -209,8 +210,8 @@ func TestListRun(t *testing.T) {
name: "no results",
opts: &listOptions{},
isTTY: true,
stubLister: stubAutoLinkLister{
autolinks: []autolink{},
stubLister: stubAutolinkLister{
autolinks: []shared.Autolink{},
},
expectedErr: cmdutil.NewNoResultsError("no autolinks found in OWNER/REPO"),
wantStderr: "",
@ -219,8 +220,8 @@ func TestListRun(t *testing.T) {
name: "client error",
opts: &listOptions{},
isTTY: true,
stubLister: stubAutoLinkLister{
autolinks: []autolink{},
stubLister: stubAutolinkLister{
autolinks: []shared.Autolink{},
err: testAutolinkClientListError{},
},
expectedErr: testAutolinkClientListError{},

21
pkg/cmd/repo/autolink/shared/autolink.go Normal file
View file

@ -0,0 +1,21 @@
package shared
import "github.com/cli/cli/v2/pkg/cmdutil"
type Autolink struct {
ID int `json:"id"`
IsAlphanumeric bool `json:"is_alphanumeric"`
KeyPrefix string `json:"key_prefix"`
URLTemplate string `json:"url_template"`
}
var AutolinkFields = []string{
"id",
"isAlphanumeric",
"keyPrefix",
"urlTemplate",
}
func (a *Autolink) ExportData(fields []string) map[string]interface{} {
return cmdutil.StructExportData(a, fields)
}

46
pkg/cmd/repo/autolink/view/http.go Normal file
View file

@ -0,0 +1,46 @@
package view
import (
"encoding/json"
"fmt"
"net/http"
"github.com/cli/cli/v2/api"
"github.com/cli/cli/v2/internal/ghinstance"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
)
type AutolinkViewer struct {
HTTPClient *http.Client
}
func (a *AutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) {
path := fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), id)
url := ghinstance.RESTPrefix(repo.RepoHost()) + path
req, err := http.NewRequest(http.MethodGet, url, nil)
if err != nil {
return nil, err
}
resp, err := a.HTTPClient.Do(req)
if err != nil {
return nil, err
}
defer resp.Body.Close()
if resp.StatusCode == http.StatusNotFound {
return nil, fmt.Errorf("HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/%s)", path)
} else if resp.StatusCode > 299 {
return nil, api.HandleHTTPError(resp)
}
var autolink shared.Autolink
err = json.NewDecoder(resp.Body).Decode(&autolink)
if err != nil {
return nil, err
}
return &autolink, nil
}

102
pkg/cmd/repo/autolink/view/http_test.go Normal file
View file

@ -0,0 +1,102 @@
package view
import (
"fmt"
"net/http"
"testing"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/httpmock"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestAutolinkViewer_View(t *testing.T) {
repo := ghrepo.New("OWNER", "REPO")
tests := []struct {
name string
id string
stubStatus int
stubRespJSON string
expectedAutolink *shared.Autolink
expectErr bool
expectedErrMsg string
}{
{
name: "200 successful alphanumeric view",
id: "123",
stubStatus: http.StatusOK,
stubRespJSON: `{
"id": 123,
"key_prefix": "TICKET-",
"url_template": "https://example.com/TICKET?query=<num>",
"is_alphanumeric": true
}`,
expectedAutolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
{
name: "200 successful numeric view",
id: "123",
stubStatus: http.StatusOK,
stubRespJSON: `{
"id": 123,
"key_prefix": "TICKET-",
"url_template": "https://example.com/TICKET?query=<num>",
"is_alphanumeric": false
}`,
expectedAutolink: &shared.Autolink{
ID: 123,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: false,
},
},
{
name: "404 repo or autolink not found",
id: "123",
stubStatus: http.StatusNotFound,
stubRespJSON: `{
"message": "Not Found",
"documentation_url": "https://docs.github.com/rest/repos/autolinks#get-an-autolink-reference-of-a-repository",
"status": "404"
}`,
expectErr: true,
expectedErrMsg: "HTTP 404: Perhaps you are missing admin rights to the repository? (https://api.github.com/repos/OWNER/REPO/autolinks/123)",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reg := &httpmock.Registry{}
reg.Register(
httpmock.REST(
http.MethodGet,
fmt.Sprintf("repos/%s/%s/autolinks/%s", repo.RepoOwner(), repo.RepoName(), tt.id),
),
httpmock.StatusStringResponse(tt.stubStatus, tt.stubRespJSON),
)
defer reg.Verify(t)
autolinkViewer := &AutolinkViewer{
HTTPClient: &http.Client{Transport: reg},
}
autolink, err := autolinkViewer.View(repo, tt.id)
if tt.expectErr {
require.EqualError(t, err, tt.expectedErrMsg)
} else {
require.NoError(t, err)
assert.Equal(t, tt.expectedAutolink, autolink)
}
})
}
}

96
pkg/cmd/repo/autolink/view/view.go Normal file
View file

@ -0,0 +1,96 @@
package view
import (
"fmt"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/spf13/cobra"
)
type viewOptions struct {
BaseRepo func() (ghrepo.Interface, error)
Browser browser.Browser
AutolinkClient AutolinkViewClient
IO *iostreams.IOStreams
Exporter cmdutil.Exporter
ID string
}
type AutolinkViewClient interface {
View(repo ghrepo.Interface, id string) (*shared.Autolink, error)
}
func NewCmdView(f *cmdutil.Factory, runF func(*viewOptions) error) *cobra.Command {
opts := &viewOptions{
Browser: f.Browser,
IO: f.IOStreams,
}
cmd := &cobra.Command{
Use: "view <id>",
Short: "View an autolink reference",
Long: "View an autolink reference for a repository.",
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
httpClient, err := f.HttpClient()
if err != nil {
return err
}
opts.BaseRepo = f.BaseRepo
opts.ID = args[0]
opts.AutolinkClient = &AutolinkViewer{HTTPClient: httpClient}
if runF != nil {
return runF(opts)
}
return viewRun(opts)
},
}
cmdutil.AddJSONFlags(cmd, &opts.Exporter, shared.AutolinkFields)
return cmd
}
func viewRun(opts *viewOptions) error {
repo, err := opts.BaseRepo()
if err != nil {
return err
}
out := opts.IO.Out
cs := opts.IO.ColorScheme()
autolink, err := opts.AutolinkClient.View(repo, opts.ID)
if err != nil {
return fmt.Errorf("%s %w", cs.Red("error viewing autolink:"), err)
}
if opts.Exporter != nil {
return opts.Exporter.Write(opts.IO, autolink)
}
fmt.Fprintf(out, "Autolink in %s\n\n", cs.Bold(ghrepo.FullName(repo)))
fmt.Fprint(out, cs.Bold("ID: "))
fmt.Fprintln(out, cs.Cyanf("%d", autolink.ID))
fmt.Fprint(out, cs.Bold("Key Prefix: "))
fmt.Fprintln(out, autolink.KeyPrefix)
fmt.Fprint(out, cs.Bold("URL Template: "))
fmt.Fprintln(out, autolink.URLTemplate)
fmt.Fprint(out, cs.Bold("Alphanumeric: "))
fmt.Fprintln(out, autolink.IsAlphanumeric)
return nil
}

197
pkg/cmd/repo/autolink/view/view_test.go Normal file
View file

@ -0,0 +1,197 @@
package view
import (
"bytes"
"net/http"
"testing"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/internal/browser"
"github.com/cli/cli/v2/internal/ghrepo"
"github.com/cli/cli/v2/pkg/cmd/repo/autolink/shared"
"github.com/cli/cli/v2/pkg/cmdutil"
"github.com/cli/cli/v2/pkg/iostreams"
"github.com/cli/cli/v2/pkg/jsonfieldstest"
"github.com/google/shlex"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestJSONFields(t *testing.T) {
jsonfieldstest.ExpectCommandToSupportJSONFields(t, NewCmdView, []string{
"id",
"isAlphanumeric",
"keyPrefix",
"urlTemplate",
})
}
func TestNewCmdView(t *testing.T) {
tests := []struct {
name string
input string
output viewOptions
wantErr bool
wantExporter bool
errMsg string
}{
{
name: "no argument",
input: "",
wantErr: true,
errMsg: "accepts 1 arg(s), received 0",
},
{
name: "id provided",
input: "123",
output: viewOptions{ID: "123"},
},
{
name: "json flag",
input: "123 --json id",
output: viewOptions{ID: "123"},
wantExporter: true,
},
{
name: "invalid json flag",
input: "123 --json invalid",
wantErr: true,
errMsg: "Unknown JSON field: \"invalid\"\nAvailable fields:\n id\n isAlphanumeric\n keyPrefix\n urlTemplate",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, _, _ := iostreams.Test()
f := &cmdutil.Factory{
IOStreams: ios,
}
f.HttpClient = func() (*http.Client, error) {
return &http.Client{}, nil
}
argv, err := shlex.Split(tt.input)
require.NoError(t, err)
var gotOpts *viewOptions
cmd := NewCmdView(f, func(opts *viewOptions) error {
gotOpts = opts
return nil
})
cmd.SetArgs(argv)
cmd.SetIn(&bytes.Buffer{})
cmd.SetOut(&bytes.Buffer{})
cmd.SetErr(&bytes.Buffer{})
_, err = cmd.ExecuteC()
if tt.wantErr {
require.EqualError(t, err, tt.errMsg)
} else {
require.NoError(t, err)
assert.Equal(t, tt.wantExporter, gotOpts.Exporter != nil)
assert.Equal(t, tt.output.ID, gotOpts.ID)
}
})
}
}
type stubAutolinkViewer struct {
autolink *shared.Autolink
err error
}
func (g stubAutolinkViewer) View(repo ghrepo.Interface, id string) (*shared.Autolink, error) {
return g.autolink, g.err
}
type testAutolinkClientViewError struct{}
func (e testAutolinkClientViewError) Error() string {
return "autolink client view error"
}
func TestViewRun(t *testing.T) {
tests := []struct {
name string
opts *viewOptions
stubViewer stubAutolinkViewer
expectedErr error
wantStdout string
}{
{
name: "view",
opts: &viewOptions{
ID: "1",
},
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 1,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
wantStdout: heredoc.Doc(`
Autolink in OWNER/REPO
ID: 1
Key Prefix: TICKET-
URL Template: https://example.com/TICKET?query=<num>
Alphanumeric: true
`),
},
{
name: "view json",
opts: &viewOptions{
Exporter: func() cmdutil.Exporter {
exporter := cmdutil.NewJSONExporter()
exporter.SetFields([]string{"id"})
return exporter
}(),
},
stubViewer: stubAutolinkViewer{
autolink: &shared.Autolink{
ID: 1,
KeyPrefix: "TICKET-",
URLTemplate: "https://example.com/TICKET?query=<num>",
IsAlphanumeric: true,
},
},
wantStdout: "{\"id\":1}\n",
},
{
name: "client error",
opts: &viewOptions{},
stubViewer: stubAutolinkViewer{
autolink: nil,
err: testAutolinkClientViewError{},
},
expectedErr: testAutolinkClientViewError{},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ios, _, stdout, _ := iostreams.Test()
opts := tt.opts
opts.IO = ios
opts.Browser = &browser.Stub{}
opts.IO = ios
opts.BaseRepo = func() (ghrepo.Interface, error) { return ghrepo.New("OWNER", "REPO"), nil }
opts.AutolinkClient = &tt.stubViewer
err := viewRun(opts)
if tt.expectedErr != nil {
require.Error(t, err)
assert.ErrorIs(t, err, tt.expectedErr)
} else {
require.NoError(t, err)
assert.Equal(t, tt.wantStdout, stdout.String())
}
})
}
}

4
pkg/cmd/repo/delete/delete.go
View file

@ -41,10 +41,10 @@ func NewCmdDelete(f *cmdutil.Factory, runF func(*DeleteOptions) error) *cobra.Co
Short: "Delete a repository",
Long: heredoc.Docf(`
Delete a GitHub repository.
With no argument, deletes the current repository. Otherwise, deletes the specified repository.
Deletion requires authorization with the %[1]sdelete_repo%[1]s scope.
Deletion requires authorization with the %[1]sdelete_repo%[1]s scope.
To authorize, run %[1]sgh auth refresh -s delete_repo%[1]s
`, "`"),
Args: cobra.MaximumNArgs(1),

2
pkg/cmd/repo/deploy-key/add/add.go
View file

@ -34,7 +34,7 @@ func NewCmdAdd(f *cmdutil.Factory, runF func(*AddOptions) error) *cobra.Command
Short: "Add a deploy key to a GitHub repository",
Long: heredoc.Doc(`
Add a deploy key to a GitHub repository.
Note that any key added by gh will be associated with the current authentication token.
If you de-authorize the GitHub CLI app or authentication token from your account, any
deploy keys added by GitHub CLI will be removed as well.

2
pkg/cmd/repo/gitignore/view/view.go
View file

@ -34,7 +34,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
Short: "View an available repository gitignore template",
Long: heredoc.Docf(`
View an available repository %[1]s.gitignore%[1]s template.
%[1]s<template>%[1]s is a case-sensitive %[1]s.gitignore%[1]s template name.
For a list of available templates, run %[1]sgh repo gitignore list%[1]s.

2
pkg/cmd/repo/license/list/list.go
View file

@ -31,7 +31,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Short: "List common repository licenses",
Long: heredoc.Doc(`
List common repository licenses.
For even more licenses, visit <https://choosealicense.com/appendix>
`),
Aliases: []string{"ls"},

2
pkg/cmd/repo/license/view/view.go
View file

@ -34,7 +34,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
}
cmd := &cobra.Command{
Use: "view {<license-key> | <SPDX-ID>}",
Use: "view {<license-key> | <spdx-id>}",
Short: "View a specific repository license",
Long: heredoc.Docf(`
View a specific repository license by license key or SPDX ID.

2
pkg/cmd/repo/license/view/view_test.go
View file

@ -220,7 +220,7 @@ func TestViewRun(t *testing.T) {
wantErr: true,
errMsg: heredoc.Docf(`
'404' is not a valid license name or SPDX ID.
Run %[1]sgh repo license list%[1]s to see available commonly used licenses. For even more licenses, visit https://choosealicense.com/appendix`, "`"),
httpStubs: func(reg *httpmock.Registry) {
reg.Register(

13
pkg/cmd/repo/rename/rename.go
View file

@ -4,6 +4,7 @@ import (
"context"
"fmt"
"net/http"
"strings"
"github.com/MakeNowJust/heredoc"
"github.com/cli/cli/v2/api"
@ -51,14 +52,14 @@ func NewCmdRename(f *cmdutil.Factory, runf func(*RenameOptions) error) *cobra.Co
Short: "Rename a repository",
Long: heredoc.Docf(`
Rename a GitHub repository.
%[1]s<new-name>%[1]s is the desired repository name without the owner.
By default, the current repository is renamed. Otherwise, the repository specified
with %[1]s--repo%[1]s is renamed.
To transfer repository ownership to another user account or organization,
you must follow additional steps on GitHub.com
you must follow additional steps on <github.com>.
For more information on transferring repository ownership, see:
<https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository>
@ -124,6 +125,10 @@ func renameRun(opts *RenameOptions) error {
}
}
if strings.Contains(newRepoName, "/") {
return fmt.Errorf("New repository name cannot contain '/' character - to transfer a repository to a new owner, you must follow additional steps on <github.com>. For more information on transferring repository ownership, see <https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository>.")
}
if opts.DoConfirm {
var confirmed bool
if confirmed, err = opts.Prompter.Confirm(fmt.Sprintf(

16
pkg/cmd/repo/rename/rename_test.go
View file

@ -113,6 +113,8 @@ func TestRenameRun(t *testing.T) {
promptStubs func(*prompter.MockPrompter)
wantOut string
tty bool
wantErr bool
errMsg string
}{
{
name: "none argument",
@ -217,6 +219,16 @@ func TestRenameRun(t *testing.T) {
},
wantOut: "",
},
{
name: "error on name with slash",
tty: true,
opts: RenameOptions{
newRepoSelector: "org/new-name",
},
wantErr: true,
errMsg: "New repository name cannot contain '/' character - to transfer a repository to a new owner, you must follow additional steps on <github.com>. For more information on transferring repository ownership, see <https://docs.github.com/en/repositories/creating-and-managing-repositories/transferring-a-repository>.",
},
}
for _, tt := range testCases {
@ -268,6 +280,10 @@ func TestRenameRun(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
defer reg.Verify(t)
err := renameRun(&tt.opts)
if tt.wantErr {
assert.EqualError(t, err, tt.errMsg)
return
}
assert.NoError(t, err)
assert.Equal(t, tt.wantOut, stdout.String())
})

7
pkg/cmd/repo/setdefault/setdefault.go
View file

@ -119,15 +119,18 @@ func setDefaultRun(opts *SetDefaultOptions) error {
currentDefaultRepo, _ := remotes.ResolvedRemote()
cs := opts.IO.ColorScheme()
if opts.ViewMode {
if currentDefaultRepo != nil {
fmt.Fprintln(opts.IO.Out, displayRemoteRepoName(currentDefaultRepo))
} else {
fmt.Fprintln(opts.IO.ErrOut, "no default repository has been set; use `gh repo set-default` to select one")
fmt.Fprintf(opts.IO.ErrOut,
"%s No default remote repository has been set. To learn more about the default repository, run: gh repo set-default --help\n",
cs.FailureIcon())
}
return nil
}
cs := opts.IO.ColorScheme()
if opts.UnsetMode {
var msg string

4
pkg/cmd/repo/setdefault/setdefault_test.go
View file

@ -176,7 +176,7 @@ func TestDefaultRun(t *testing.T) {
Repo: repo1,
},
},
wantStderr: "no default repository has been set; use `gh repo set-default` to select one\n",
wantStderr: "X No default remote repository has been set. To learn more about the default repository, run: gh repo set-default --help\n",
},
{
name: "view mode no current default",
@ -188,7 +188,7 @@ func TestDefaultRun(t *testing.T) {
Repo: repo1,
},
},
wantStderr: "no default repository has been set; use `gh repo set-default` to select one\n",
wantStderr: "X No default remote repository has been set. To learn more about the default repository, run: gh repo set-default --help\n",
},
{
name: "view mode with base resolved current default",

2
pkg/cmd/root/help_topic.go
View file

@ -43,7 +43,7 @@ var HelpTopics = []helpTopic{
short: "Environment variables that can be used with gh",
long: heredoc.Docf(`
%[1]sGH_TOKEN%[1]s, %[1]sGITHUB_TOKEN%[1]s (in order of precedence): an authentication token that will be used when
a command targets either github.com or a subdomain of ghe.com. Setting this avoids being prompted to
a command targets either <github.com> or a subdomain of <ghe.com>. Setting this avoids being prompted to
authenticate and takes precedence over previously stored credentials.
%[1]sGH_ENTERPRISE_TOKEN%[1]s, %[1]sGITHUB_ENTERPRISE_TOKEN%[1]s (in order of precedence): an authentication

2
pkg/cmd/ruleset/list/list.go
View file

@ -49,7 +49,7 @@ func NewCmdList(f *cmdutil.Factory, runF func(*ListOptions) error) *cobra.Comman
Use the %[1]s--parents%[1]s flag to control whether rulesets configured at higher levels that also apply to the provided
repository or organization should be returned. The default is %[1]strue%[1]s.
Your access token must have the %[1]sadmin:org%[1]s scope to use the %[1]s--org%[1]s flag, which can be granted by running %[1]sgh auth refresh -s admin:org%[1]s.
`, "`"),
Example: heredoc.Doc(`

2
pkg/cmd/ruleset/view/view.go
View file

@ -49,7 +49,7 @@ func NewCmdView(f *cmdutil.Factory, runF func(*ViewOptions) error) *cobra.Comman
If no ID is provided, an interactive prompt will be used to choose
the ruleset to view.
Use the %[1]s--parents%[1]s flag to control whether rulesets configured at higher
levels that also apply to the provided repository or organization should
be returned. The default is %[1]strue%[1]s.

12
pkg/cmd/ruleset/view/view_test.go
View file

@ -158,14 +158,14 @@ func Test_viewRun(t *testing.T) {
Source: my-owner/repo-name (Repository)
Enforcement: Active
You can bypass: pull requests only
Bypass List
- OrganizationAdmin (ID: 1), mode: always
- RepositoryRole (ID: 5), mode: always
Conditions
- ref_name: [exclude: []] [include: [~ALL]]
Rules
- commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com]
- commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf]
@ -212,14 +212,14 @@ func Test_viewRun(t *testing.T) {
ID: 74
Source: my-owner (Organization)
Enforcement: Evaluate Mode (not enforced)
Bypass List
This ruleset cannot be bypassed
Conditions
- ref_name: [exclude: []] [include: [~ALL]]
- repository_name: [exclude: []] [include: [~ALL]] [protected: true]
Rules
- commit_author_email_pattern: [name: ] [negate: false] [operator: ends_with] [pattern: @example.com]
- commit_message_pattern: [name: ] [negate: false] [operator: contains] [pattern: asdf]

2
pkg/cmd/run/list/list.go
View file

@ -119,6 +119,8 @@ func listRun(opts *ListOptions) error {
}
opts.IO.StartProgressIndicator()
defer opts.IO.StopProgressIndicator()
if opts.WorkflowSelector != "" {
// initially the workflow state is limited to 'active'
states := []workflowShared.WorkflowState{workflowShared.Active}

2
pkg/cmd/run/rerun/rerun.go
View file

@ -50,7 +50,7 @@ func NewCmdRerun(f *cmdutil.Factory, runF func(*RerunOptions) error) *cobra.Comm
However, this %[1]s<number>%[1]s should not be used with the %[1]s--job%[1]s flag and will result in the
API returning %[1]s404 NOT FOUND%[1]s. Instead, you can get the correct job IDs using the following command:
gh run view <run-id> --json jobs --jq '.jobs[] | {name, databaseId}'
You will need to use databaseId field for triggering job re-runs.

Some files were not shown because too many files have changed in this diff Show more