From c9d0f220808d6c484c841079b0eb43ca8ef7c387 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Thu, 30 Nov 2023 21:46:58 +0100 Subject: [PATCH 01/12] feat: align ci job id args --- commands/ci/ci.go | 2 +- commands/ci/ciutils/utils.go | 179 +++++++++++++++++++- commands/ci/ciutils/utils_test.go | 124 ++++++++++++++ commands/ci/legacyci/pipeline_ci.go | 2 +- commands/ci/retry/retry.go | 38 ++++- commands/ci/retry/retry_test.go | 176 +++++++++++++++---- commands/ci/status/status.go | 16 +- commands/ci/trace/trace.go | 158 +++-------------- commands/ci/trace/trace_integration_test.go | 80 +-------- commands/ci/trace/trace_test.go | 167 ++++++++++++++++++ docs/source/ci/ci/trace.md | 6 +- docs/source/ci/retry.md | 16 +- docs/source/ci/trace.md | 6 +- 13 files changed, 697 insertions(+), 273 deletions(-) create mode 100644 commands/ci/ciutils/utils_test.go create mode 100644 commands/ci/trace/trace_test.go diff --git a/commands/ci/ci.go b/commands/ci/ci.go index c6d50ce12..e49470f5e 100644 --- a/commands/ci/ci.go +++ b/commands/ci/ci.go @@ -28,7 +28,7 @@ func NewCmdCI(f *cmdutils.Factory) *cobra.Command { cmdutils.EnableRepoOverride(ciCmd, f) ciCmd.AddCommand(legacyCICmd.NewCmdCI(f)) - ciCmd.AddCommand(ciTraceCmd.NewCmdTrace(f, nil)) + ciCmd.AddCommand(ciTraceCmd.NewCmdTrace(f)) ciCmd.AddCommand(ciViewCmd.NewCmdView(f)) ciCmd.AddCommand(ciLintCmd.NewCmdLint(f)) ciCmd.AddCommand(pipeDeleteCmd.NewCmdDelete(f)) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 6fff9f3bf..61b25258f 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -4,15 +4,22 @@ import ( "context" "fmt" "io" + "regexp" "sync" "time" + "gitlab.com/gitlab-org/cli/commands/cmdutils" + "gitlab.com/gitlab-org/cli/internal/glrepo" + "gitlab.com/gitlab-org/cli/pkg/git" "gitlab.com/gitlab-org/cli/pkg/iostreams" + "gitlab.com/gitlab-org/cli/pkg/prompt" "gitlab.com/gitlab-org/cli/api" "gitlab.com/gitlab-org/cli/pkg/tableprinter" "gitlab.com/gitlab-org/cli/pkg/utils" + "github.com/AlecAivazis/survey/v2" + "github.com/AlecAivazis/survey/v2/terminal" "github.com/pkg/errors" "github.com/xanzy/go-gitlab" ) @@ -78,10 +85,10 @@ func RunTraceSha(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid if err != nil || job == nil { return errors.Wrap(err, "failed to find job") } - return RunTrace(ctx, apiClient, w, pid, job, name) + return runTrace(ctx, apiClient, w, pid, job, name) } -func RunTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid interface{}, job *gitlab.Job, name string) error { +func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid interface{}, job *gitlab.Job, name string) error { fmt.Fprintln(w, "Getting job trace...") for range time.NewTicker(time.Second * 3).C { if ctx.Err() == context.Canceled { @@ -122,3 +129,171 @@ func RunTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid in } return nil } + +func GetJobId(opts *JobOptions) (int, error) { + jobID := utils.StringToInt(opts.JobName) + + if jobID < 1 { + pipelineId, err := getPipelineId(opts) + if err != nil { + return 0, err + } + if opts.JobName != "" { + jobs, _, err := opts.ApiClient.Jobs.ListPipelineJobs(opts.Repo.FullName(), pipelineId, nil) + if err != nil { + return 0, err + } + for _, job := range jobs { + if job.Name == opts.JobName { + jobID = job.ID + break + } + } + if jobID < 1 { + return 0, cmdutils.SilentError + } + } else { + jobID, err = getJobIdInteractive(opts) + if err != nil { + return 0, err + } + + } + } + return jobID, nil +} + +func getPipelineId(opts *JobOptions) (int, error) { + if opts.PipelineId == 0 { + + branch, err := getBranch(opts) + if err != nil { + return 0, err + } + + commit, err := api.GetCommit(opts.ApiClient, opts.Repo.FullName(), branch) + if err != nil { + return 0, err + } + return commit.LastPipeline.ID, nil + } + return opts.PipelineId, nil +} + +func getBranch(opts *JobOptions) (string, error) { + if opts.Branch == "" { + branch, err := git.CurrentBranch() + if err != nil { + return "", err + } + return branch, nil + } + return opts.Branch, nil +} + +func getJobIdInteractive(opts *JobOptions) (int, error) { + branch, err := getBranch(opts) + if err != nil { + return 0, err + } + fmt.Fprintf(opts.IO.StdOut, "\nSearching for latest pipeline on %s...\n", branch) + + pipeline, err := api.GetLastPipeline(opts.ApiClient, opts.Repo.FullName(), branch) + if err != nil { + return 0, err + } + + fmt.Fprintf(opts.IO.StdOut, "Getting jobs for pipeline %d...\n\n", pipeline.ID) + + jobs, err := api.GetPipelineJobs(opts.ApiClient, pipeline.ID, opts.Repo.FullName()) + if err != nil { + return 0, err + } + + var jobOptions []string + var selectedJob string + + for _, job := range jobs { + jobOptions = append(jobOptions, fmt.Sprintf("%s (%d) - %s", job.Name, job.ID, job.Status)) + } + + promptOpts := &survey.Select{ + Message: "Select pipeline job to trace:", + Options: jobOptions, + } + + err = prompt.AskOne(promptOpts, &selectedJob) + if err != nil { + if errors.Is(err, terminal.InterruptErr) { + return 0, nil + } + + return 0, err + } + + if selectedJob != "" { + re := regexp.MustCompile(`(?s)\((.*)\)`) + m := re.FindAllStringSubmatch(selectedJob, -1) + return utils.StringToInt(m[0][1]), nil + } else if len(jobs) > 0 { + return jobs[0].ID, nil + } else { + // use commit statuses to show external jobs + cs, err := api.GetCommitStatuses(opts.ApiClient, opts.Repo.FullName(), pipeline.SHA) + if err != nil { + return 0, nil + } + + c := opts.IO.Color() + + fmt.Fprint(opts.IO.StdOut, "Getting external jobs...") + for _, status := range cs { + var s string + + switch status.Status { + case "success": + s = c.Green(status.Status) + case "error": + s = c.Red(status.Status) + default: + s = c.Gray(status.Status) + } + fmt.Fprintf(opts.IO.StdOut, "(%s) %s\nURL: %s\n\n", s, c.Bold(status.Name), c.Gray(status.TargetURL)) + } + + return 0, nil + } +} + +type JobOptions struct { + ApiClient *gitlab.Client + Repo glrepo.Interface + JobName string + Branch string + PipelineId int + IO *iostreams.IOStreams +} + +func TraceJob(opts *JobOptions) error { + jobID, err := GetJobId(opts) + if err != nil { + return err + } + if jobID < 1 { + fmt.Fprintln(opts.IO.StdErr, "invalid job id:", opts.JobName) + return cmdutils.SilentError + } + + job, err := api.GetPipelineJob(opts.ApiClient, jobID, opts.Repo.FullName()) + if err != nil { + return err + } + fmt.Fprintln(opts.IO.StdOut) + + err = runTrace(context.Background(), opts.ApiClient, opts.IO.StdOut, opts.Repo.FullName(), job, job.Name) + if err != nil { + return err + } + + return nil +} diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go new file mode 100644 index 000000000..c12c171b4 --- /dev/null +++ b/commands/ci/ciutils/utils_test.go @@ -0,0 +1,124 @@ +package ciutils + +import ( + "net/http" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/cli/commands/cmdtest" + "gitlab.com/gitlab-org/cli/pkg/httpmock" + "gitlab.com/gitlab-org/cli/pkg/iostreams" + "gitlab.com/gitlab-org/cli/pkg/prompt" +) + +func TestGetJobId(t *testing.T) { + type httpMock struct { + method string + path string + status int + body string + } + + tests := []struct { + name string + jobName string + pipelineId int + httpMocks []httpMock + expectedOut int + }{ + { + name: "when getJobId with integer is requested", + jobName: "1122", + httpMocks: []httpMock{}, + expectedOut: 1122, + }, { + name: "when getJobId with name and pipelineId is requested", + jobName: "lint", + pipelineId: 123, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + + expectedOut: 1122, + }, { + name: "when getJobId with name and last pipeline is requested", + jobName: "lint", + pipelineId: 0, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/repository/commits/main", + http.StatusOK, + `{ + "last_pipeline" : { + "id": 123 + } + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + expectedOut: 1122, + } + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := &httpmock.Mocker{ + MatchURL: httpmock.PathAndQuerystring, + } + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } + + ios, _, _, _ := iostreams.Test() + f := cmdtest.InitFactory(ios, fakeHTTP) + + _, _ = f.HttpClient() + + apiClient, _ := f.HttpClient() + repo, _ := f.BaseRepo() + + output, err := GetJobId(&JobOptions{ + IO: f.IO, + Repo: repo, + ApiClient: apiClient, + JobName: tc.jobName, + PipelineId: tc.pipelineId, + Branch: "main", + }) + + require.Nil(t, err) + + assert.Equal(t, tc.expectedOut, output) + }) + } +} diff --git a/commands/ci/legacyci/pipeline_ci.go b/commands/ci/legacyci/pipeline_ci.go index 2e7981e67..87c4de004 100644 --- a/commands/ci/legacyci/pipeline_ci.go +++ b/commands/ci/legacyci/pipeline_ci.go @@ -22,7 +22,7 @@ func NewCmdCI(f *cmdutils.Factory) *cobra.Command { `), } - pipelineCICmd.AddCommand(ciTraceCmd.NewCmdTrace(f, nil)) + pipelineCICmd.AddCommand(ciTraceCmd.NewCmdTrace(f)) pipelineCICmd.AddCommand(ciViewCmd.NewCmdView(f)) pipelineCICmd.AddCommand(ciLintCmd.NewCmdLint(f)) pipelineCICmd.Deprecated = "This command is deprecated. All the commands under it has been moved to `ci` or `pipeline` command. See https://gitlab.com/gitlab-org/cli/issues/372 for more info." diff --git a/commands/ci/retry/retry.go b/commands/ci/retry/retry.go index 2da83ac2d..ab7b07b7c 100644 --- a/commands/ci/retry/retry.go +++ b/commands/ci/retry/retry.go @@ -4,38 +4,60 @@ import ( "fmt" "gitlab.com/gitlab-org/cli/api" + "gitlab.com/gitlab-org/cli/commands/ci/ciutils" "gitlab.com/gitlab-org/cli/commands/cmdutils" - "gitlab.com/gitlab-org/cli/pkg/utils" "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" ) func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { + opts := &ciutils.JobOptions{ + IO: f.IO, + } pipelineRetryCmd := &cobra.Command{ Use: "retry ", Short: `Retry a CI/CD job`, Aliases: []string{}, Example: heredoc.Doc(` - glab ci retry 871528 + $ glab ci retry + #=> interactively select a job to retry + + $ glab ci retry 224356863 + #=> retry job with id 224356863 + + $ glab ci retry lint + #=> retry job with name lint `), Long: ``, - Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { var err error - apiClient, err := f.HttpClient() + if len(args) != 0 { + opts.JobName = args[0] + } + + repo, err := f.BaseRepo() if err != nil { return err } + opts.Repo = repo - repo, err := f.BaseRepo() + apiClient, err := f.HttpClient() if err != nil { return err } + opts.ApiClient = apiClient + + opts.Branch, _ = cmd.Flags().GetString("branch") + opts.PipelineId, _ = cmd.Flags().GetInt("pipeline-id") + opts.IO = f.IO - jobID := utils.StringToInt(args[0]) + jobID, err := ciutils.GetJobId(opts) + if err != nil { + return err + } if jobID < 1 { fmt.Fprintln(f.IO.StdErr, "invalid job id:", args[0]) return cmdutils.SilentError @@ -45,11 +67,13 @@ func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { if err != nil { return cmdutils.WrapError(err, fmt.Sprintf("Could not retry job with ID: %d", jobID)) } - fmt.Fprintln(f.IO.StdOut, "Retried job (id:", job.ID, "), status:", job.Status, ", ref:", job.Ref, ", weburl: ", job.WebURL, ")") + fmt.Fprintln(f.IO.StdOut, "Retried job (ID:", job.ID, "), status:", job.Status, ", ref:", job.Ref, ", weburl: ", job.WebURL, ")") return nil }, } + pipelineRetryCmd.Flags().StringP("branch", "b", "", "The branch to search for the job. (Default: current branch)") + pipelineRetryCmd.Flags().IntP("pipeline-id", "p", 0, "The pipeline ID to search for the job.") return pipelineRetryCmd } diff --git a/commands/ci/retry/retry_test.go b/commands/ci/retry/retry_test.go index 3c464e885..efceb9b1f 100644 --- a/commands/ci/retry/retry_test.go +++ b/commands/ci/retry/retry_test.go @@ -4,17 +4,16 @@ import ( "net/http" "testing" - "gitlab.com/gitlab-org/cli/pkg/iostreams" - - "github.com/MakeNowJust/heredoc" + "gitlab.com/gitlab-org/cli/commands/cmdtest" "github.com/stretchr/testify/assert" - "gitlab.com/gitlab-org/cli/commands/cmdtest" + "github.com/stretchr/testify/require" "gitlab.com/gitlab-org/cli/pkg/httpmock" + "gitlab.com/gitlab-org/cli/pkg/iostreams" "gitlab.com/gitlab-org/cli/test" ) -func runCommand(rt http.RoundTripper, cli string) (*test.CmdOut, error) { +func runCommand(rt http.RoundTripper, args string) (*test.CmdOut, error) { ios, _, stdout, stderr := iostreams.Test() factory := cmdtest.InitFactory(ios, rt) @@ -22,40 +21,151 @@ func runCommand(rt http.RoundTripper, cli string) (*test.CmdOut, error) { cmd := NewCmdRetry(factory) - return cmdtest.ExecuteCommand(cmd, cli, stdout, stderr) + return cmdtest.ExecuteCommand(cmd, args, stdout, stderr) } func TestCiRetry(t *testing.T) { - fakeHTTP := httpmock.New() - defer fakeHTTP.Verify(t) + type httpMock struct { + method string + path string + status int + body string + } - // test will fail with unmatched HTTP stub if this POST is not performed - fakeHTTP.RegisterResponder(http.MethodPost, "/projects/OWNER/REPO/jobs/1122/retry", - httpmock.NewStringResponse(http.StatusCreated, ` + tests := []struct { + name string + args string + httpMocks []httpMock + expectedOut string + }{ { - "id": 1123, - "status": "pending", - "stage": "build", - "name": "build-job", - "ref": "branch-name", - "tag": false, - "coverage": null, - "allow_failure": false, - "created_at": "2022-12-01T05:13:13.703Z", - "web_url": "https://gitlab.com/OWNER/REPO/-/jobs/1123" - } - `)) - - jobId := "1122" - output, err := runCommand(fakeHTTP, jobId) - if err != nil { - t.Errorf("error running command `ci retry %s`: %v", jobId, err) + name: "when retry with job-id", + args: "1122", + httpMocks: []httpMock{ + { + http.MethodPost, + "/api/v4/projects/OWNER/REPO/jobs/1122/retry", + http.StatusCreated, + `{ + "id": 1123, + "status": "pending", + "stage": "build", + "name": "build-job", + "ref": "branch-name", + "tag": false, + "coverage": null, + "allow_failure": false, + "created_at": "2022-12-01T05:13:13.703Z", + "web_url": "https://gitlab.com/OWNER/REPO/-/jobs/1123" + }`, + }, + }, + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", + }, + { + name: "when retry with job-name", + args: "lint -b main -p 123", + httpMocks: []httpMock{ + { + http.MethodPost, + "/api/v4/projects/OWNER/REPO/jobs/1122/retry", + http.StatusCreated, + `{ + "id": 1123, + "status": "pending", + "stage": "build", + "name": "build-job", + "ref": "branch-name", + "tag": false, + "coverage": null, + "allow_failure": false, + "created_at": "2022-12-01T05:13:13.703Z", + "web_url": "https://gitlab.com/OWNER/REPO/-/jobs/1123" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", + }, + { + name: "when retry with job-name and last pipeline", + args: "lint -b main", + httpMocks: []httpMock{ + { + http.MethodPost, + "/api/v4/projects/OWNER/REPO/jobs/1122/retry", + http.StatusCreated, + `{ + "id": 1123, + "status": "pending", + "stage": "build", + "name": "build-job", + "ref": "branch-name", + "tag": false, + "coverage": null, + "allow_failure": false, + "created_at": "2022-12-01T05:13:13.703Z", + "web_url": "https://gitlab.com/OWNER/REPO/-/jobs/1123" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/repository/commits/main", + http.StatusOK, + `{ + "last_pipeline" : { + "id": 123 + } + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", + }, } - out := output.String() + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := &httpmock.Mocker{ + MatchURL: httpmock.PathAndQuerystring, + } + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } - assert.Equal(t, heredoc.Doc(` - Retried job (id: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 ) -`), out) - assert.Empty(t, output.Stderr()) + output, err := runCommand(fakeHTTP, tc.args) + require.Nil(t, err) + + assert.Equal(t, tc.expectedOut, output.String()) + assert.Empty(t, output.Stderr()) + }) + } } diff --git a/commands/ci/status/status.go b/commands/ci/status/status.go index 623df2276..14276a9ed 100644 --- a/commands/ci/status/status.go +++ b/commands/ci/status/status.go @@ -5,7 +5,7 @@ import ( "time" "gitlab.com/gitlab-org/cli/api" - ciTraceCmd "gitlab.com/gitlab-org/cli/commands/ci/trace" + "gitlab.com/gitlab-org/cli/commands/ci/ciutils" "gitlab.com/gitlab-org/cli/commands/cmdutils" "gitlab.com/gitlab-org/cli/pkg/git" "gitlab.com/gitlab-org/cli/pkg/utils" @@ -144,16 +144,12 @@ func NewCmdStatus(f *cmdutils.Factory) *cobra.Command { isRunning = false } } - if retry == "View Logs" { - // ToDo: bad idea to call another sub-command. should be fixed to avoid cyclo imports - // and the a shared function placed in the ciutils sub-module - return ciTraceCmd.TraceRun(&ciTraceCmd.TraceOpts{ - Branch: branch, - JobID: 0, - BaseRepo: f.BaseRepo, - HTTPClient: f.HttpClient, - IO: f.IO, + return ciutils.TraceJob(&ciutils.JobOptions{ + Branch: branch, + Repo: repo, + ApiClient: apiClient, + IO: f.IO, }) } } diff --git a/commands/ci/trace/trace.go b/commands/ci/trace/trace.go index 563365aa8..34f1098d9 100644 --- a/commands/ci/trace/trace.go +++ b/commands/ci/trace/trace.go @@ -1,40 +1,15 @@ package trace import ( - "context" - "errors" - "fmt" - "regexp" - - "gitlab.com/gitlab-org/cli/pkg/iostreams" - - "gitlab.com/gitlab-org/cli/internal/glrepo" - "gitlab.com/gitlab-org/cli/pkg/prompt" - - "gitlab.com/gitlab-org/cli/api" "gitlab.com/gitlab-org/cli/commands/ci/ciutils" "gitlab.com/gitlab-org/cli/commands/cmdutils" - "gitlab.com/gitlab-org/cli/pkg/git" - "gitlab.com/gitlab-org/cli/pkg/utils" - "github.com/AlecAivazis/survey/v2" - "github.com/AlecAivazis/survey/v2/terminal" "github.com/MakeNowJust/heredoc" "github.com/spf13/cobra" - "github.com/xanzy/go-gitlab" ) -type TraceOpts struct { - Branch string - JobID int - - BaseRepo func() (glrepo.Interface, error) - HTTPClient func() (*gitlab.Client, error) - IO *iostreams.IOStreams -} - -func NewCmdTrace(f *cmdutils.Factory, runE func(traceOpts *TraceOpts) error) *cobra.Command { - opts := &TraceOpts{ +func NewCmdTrace(f *cmdutils.Factory) *cobra.Command { + opts := &ciutils.JobOptions{ IO: f.IO, } pipelineCITraceCmd := &cobra.Command{ @@ -46,129 +21,38 @@ func NewCmdTrace(f *cmdutils.Factory, runE func(traceOpts *TraceOpts) error) *co $ glab ci trace 224356863 #=> trace job with id 224356863 + + $ glab ci trace lint + #=> trace job with name lint `), RunE: func(cmd *cobra.Command, args []string) error { var err error - // support `-R, --repo` override - // - // NOTE: it is important to assign the BaseRepo and HTTPClient in RunE because - // they are overridden in a PersistentRun hook (when `-R, --repo` is specified) - // which runs before RunE is executed - opts.BaseRepo = f.BaseRepo - opts.HTTPClient = f.HttpClient - if len(args) != 0 { - opts.JobID = utils.StringToInt(args[0]) - } - if opts.Branch == "" { - opts.Branch, err = git.CurrentBranch() - if err != nil { - return err - } - } - if runE != nil { - return runE(opts) + opts.JobName = args[0] } - return TraceRun(opts) - }, - } - - pipelineCITraceCmd.Flags().StringVarP(&opts.Branch, "branch", "b", "", "Check pipeline status for a branch. (Default is the current branch)") - return pipelineCITraceCmd -} - -func TraceRun(opts *TraceOpts) error { - apiClient, err := opts.HTTPClient() - if err != nil { - return err - } - - repo, err := opts.BaseRepo() - if err != nil { - return err - } - if opts.JobID < 1 { - fmt.Fprintf(opts.IO.StdOut, "\nSearching for latest pipeline on %s...\n", opts.Branch) - - pipeline, err := api.GetLastPipeline(apiClient, repo.FullName(), opts.Branch) - if err != nil { - return err - } - - fmt.Fprintf(opts.IO.StdOut, "Getting jobs for pipeline %d...\n\n", pipeline.ID) - - jobs, err := api.GetPipelineJobs(apiClient, pipeline.ID, repo.FullName()) - if err != nil { - return err - } - - var jobOptions []string - var selectedJob string - - for _, job := range jobs { - jobOptions = append(jobOptions, fmt.Sprintf("%s (%d) - %s", job.Name, job.ID, job.Status)) - } - - promptOpts := &survey.Select{ - Message: "Select pipeline job to trace:", - Options: jobOptions, - } - - err = prompt.AskOne(promptOpts, &selectedJob) - if err != nil { - if errors.Is(err, terminal.InterruptErr) { - return nil - } - - return err - } - - if selectedJob != "" { - re := regexp.MustCompile(`(?s)\((.*)\)`) - m := re.FindAllStringSubmatch(selectedJob, -1) - opts.JobID = utils.StringToInt(m[0][1]) - } else if len(jobs) > 0 { - opts.JobID = jobs[0].ID - } else { - // use commit statuses to show external jobs - cs, err := api.GetCommitStatuses(apiClient, repo.FullName(), pipeline.SHA) + repo, err := f.BaseRepo() if err != nil { - return nil + return err } + opts.Repo = repo - c := opts.IO.Color() - - fmt.Fprint(opts.IO.StdOut, "Getting external jobs...") - for _, status := range cs { - var s string - - switch status.Status { - case "success": - s = c.Green(status.Status) - case "error": - s = c.Red(status.Status) - default: - s = c.Gray(status.Status) - } - fmt.Fprintf(opts.IO.StdOut, "(%s) %s\nURL: %s\n\n", s, c.Bold(status.Name), c.Gray(status.TargetURL)) + apiClient, err := f.HttpClient() + if err != nil { + return err } + opts.ApiClient = apiClient - return nil - } - } + opts.Branch, _ = cmd.Flags().GetString("branch") + opts.PipelineId, _ = cmd.Flags().GetInt("pipeline-id") + opts.IO = f.IO - job, err := api.GetPipelineJob(apiClient, opts.JobID, repo.FullName()) - if err != nil { - return err - } - fmt.Fprintln(opts.IO.StdOut) - - err = ciutils.RunTrace(context.Background(), apiClient, opts.IO.StdOut, repo.FullName(), job, job.Name) - if err != nil { - return err + return ciutils.TraceJob(opts) + }, } - return nil + pipelineCITraceCmd.Flags().StringP("branch", "b", "", "The branch to search for the job. (Default: current branch)") + pipelineCITraceCmd.Flags().IntP("pipeline-id", "p", 0, "The pipeline ID to search for the job.") + return pipelineCITraceCmd } diff --git a/commands/ci/trace/trace_integration_test.go b/commands/ci/trace/trace_integration_test.go index 8b3318a18..5e629ed51 100644 --- a/commands/ci/trace/trace_integration_test.go +++ b/commands/ci/trace/trace_integration_test.go @@ -14,7 +14,6 @@ import ( "github.com/spf13/cobra" "gitlab.com/gitlab-org/cli/commands/cmdutils" - "gitlab.com/gitlab-org/cli/internal/config" "gitlab.com/gitlab-org/cli/commands/cmdtest" ) @@ -29,83 +28,6 @@ func TestMain(m *testing.M) { cmdtest.InitTest(m, "ci_trace_test") } -func TestNewCmdTrace_Integration(t *testing.T) { - glTestHost := test.GetHostOrSkip(t) - - defer config.StubConfig(`--- -git_protocol: https -hosts: - gitlab.com: - username: root -`, "")() - - var io *iostreams.IOStreams - io, _, stdout, _ = iostreams.Test() - stubFactory, _ = cmdtest.StubFactoryWithConfig(glTestHost + "/cli-automated-testing/test.git") - stubFactory.IO = io - stubFactory.IO.IsaTTY = true - stubFactory.IO.IsErrTTY = true - - tests := []struct { - name string - args string - wantOpts *TraceOpts - }{ - { - name: "Has no arg", - args: ``, - wantOpts: &TraceOpts{ - Branch: "master", - JobID: 0, - }, - }, - { - name: "Has arg with job-id", - args: `3509632552`, - wantOpts: &TraceOpts{ - Branch: "master", - JobID: 3509632552, - }, - }, - { - name: "On a specified repo with job ID", - args: "3509632552 -X cli-automated-testing/test", - wantOpts: &TraceOpts{ - Branch: "master", - JobID: 3509632552, - }, - }, - } - - var actualOpts *TraceOpts - cmd = NewCmdTrace(stubFactory, func(opts *TraceOpts) error { - actualOpts = opts - return nil - }) - cmd.Flags().StringP("repo", "X", "", "") - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - tt.wantOpts.IO = stubFactory.IO - - argv, err := shlex.Split(tt.args) - if err != nil { - t.Fatal(err) - } - cmd.SetArgs(argv) - _, err = cmd.ExecuteC() - if err != nil { - t.Fatal(err) - } - - assert.Equal(t, tt.wantOpts.JobID, actualOpts.JobID) - assert.Equal(t, tt.wantOpts.Branch, actualOpts.Branch) - assert.Equal(t, tt.wantOpts.Branch, actualOpts.Branch) - assert.Equal(t, tt.wantOpts.IO, actualOpts.IO) - }) - } -} - func TestTraceRun(t *testing.T) { glTestHost := test.GetHostOrSkip(t) @@ -148,7 +70,7 @@ func TestTraceRun(t *testing.T) { }, } - cmd = NewCmdTrace(stubFactory, nil) + cmd = NewCmdTrace(stubFactory) cmd.Flags().StringP("repo", "X", "", "") for _, tt := range tests { diff --git a/commands/ci/trace/trace_test.go b/commands/ci/trace/trace_test.go new file mode 100644 index 000000000..a7895233b --- /dev/null +++ b/commands/ci/trace/trace_test.go @@ -0,0 +1,167 @@ +package trace + +import ( + "net/http" + "testing" + + "gitlab.com/gitlab-org/cli/commands/cmdtest" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gitlab.com/gitlab-org/cli/pkg/httpmock" + "gitlab.com/gitlab-org/cli/pkg/iostreams" + "gitlab.com/gitlab-org/cli/test" +) + +func runCommand(rt http.RoundTripper, args string) (*test.CmdOut, error) { + ios, _, stdout, stderr := iostreams.Test() + factory := cmdtest.InitFactory(ios, rt) + + _, _ = factory.HttpClient() + + cmd := NewCmdTrace(factory) + + return cmdtest.ExecuteCommand(cmd, args, stdout, stderr) +} + +func TestCiTrace(t *testing.T) { + type httpMock struct { + method string + path string + status int + body string + } + + tests := []struct { + name string + args string + httpMocks []httpMock + expectedOut string + }{ + { + name: "when trace for job-id is requested", + args: "1122", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusOK, + `Lorem ipsum`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + }, + expectedOut: "\nGetting job trace...\nShowing logs for lint job #1122\nLorem ipsum", + }, { + name: "when trace for job-name is requested", + args: "lint -b main -p 123", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusOK, + `Lorem ipsum`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + expectedOut: "\nGetting job trace...\n", + }, + { + name: "when trace for job-name and last pipeline is requested", + args: "lint -b main", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusOK, + `Lorem ipsum`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/repository/commits/main", + http.StatusOK, + `{ + "last_pipeline" : { + "id": 123 + } + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/pipelines/123/jobs", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + expectedOut: "\nGetting job trace...\n", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := &httpmock.Mocker{ + MatchURL: httpmock.PathAndQuerystring, + } + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } + + output, err := runCommand(fakeHTTP, tc.args) + require.Nil(t, err) + + assert.Equal(t, tc.expectedOut, output.String()) + assert.Empty(t, output.Stderr()) + }) + } +} diff --git a/docs/source/ci/ci/trace.md b/docs/source/ci/ci/trace.md index b7da375b1..68e90ae81 100644 --- a/docs/source/ci/ci/trace.md +++ b/docs/source/ci/ci/trace.md @@ -26,12 +26,16 @@ $ glab ci trace $ glab ci trace 224356863 #=> trace job with id 224356863 +$ glab ci trace lint +#=> trace job with name lint + ``` ## Options ```plaintext - -b, --branch string Check pipeline status for a branch. (Default is the current branch) + -b, --branch string The branch to search for the job. (Default: current branch) + -p, --pipeline-id int The pipeline ID to search for the job. ``` ## Options inherited from parent commands diff --git a/docs/source/ci/retry.md b/docs/source/ci/retry.md index 44541a3f5..9ee716357 100644 --- a/docs/source/ci/retry.md +++ b/docs/source/ci/retry.md @@ -20,8 +20,22 @@ glab ci retry [flags] ## Examples ```plaintext -glab ci retry 871528 +$ glab ci retry +#=> interactively select a job to retry +$ glab ci retry 224356863 +#=> retry job with id 224356863 + +$ glab ci retry lint +#=> retry job with name lint + +``` + +## Options + +```plaintext + -b, --branch string The branch to search for the job. (Default: current branch) + -p, --pipeline-id int The pipeline ID to search for the job. ``` ## Options inherited from parent commands diff --git a/docs/source/ci/trace.md b/docs/source/ci/trace.md index b92b44672..e6db795fb 100644 --- a/docs/source/ci/trace.md +++ b/docs/source/ci/trace.md @@ -26,12 +26,16 @@ $ glab ci trace $ glab ci trace 224356863 #=> trace job with id 224356863 +$ glab ci trace lint +#=> trace job with name lint + ``` ## Options ```plaintext - -b, --branch string Check pipeline status for a branch. (Default is the current branch) + -b, --branch string The branch to search for the job. (Default: current branch) + -p, --pipeline-id int The pipeline ID to search for the job. ``` ## Options inherited from parent commands -- GitLab From a5302d7734acd20366a19d48e22acfa371f30d2e Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 18:37:52 +0000 Subject: [PATCH 02/12] feat: apply review suggestions --- commands/ci/ciutils/utils.go | 98 +++++++++++++++++------------------- commands/ci/retry/retry.go | 6 +-- commands/ci/trace/trace.go | 6 +-- 3 files changed, 53 insertions(+), 57 deletions(-) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 61b25258f..3dbcb0083 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -131,64 +131,65 @@ func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid in } func GetJobId(opts *JobOptions) (int, error) { - jobID := utils.StringToInt(opts.JobName) + // If the user hasn't supplied an argument, we display the jobs list interactively. + if opts.JobName == "" { + return getJobIdInteractive(opts) + } - if jobID < 1 { - pipelineId, err := getPipelineId(opts) - if err != nil { - return 0, err - } - if opts.JobName != "" { - jobs, _, err := opts.ApiClient.Jobs.ListPipelineJobs(opts.Repo.FullName(), pipelineId, nil) - if err != nil { - return 0, err - } - for _, job := range jobs { - if job.Name == opts.JobName { - jobID = job.ID - break - } - } - if jobID < 1 { - return 0, cmdutils.SilentError - } - } else { - jobID, err = getJobIdInteractive(opts) - if err != nil { - return 0, err - } + // If the user supplied a job ID, we can use it directly. + if jobID, err := strconv.Atoi(opts.JobName); err == nil { + return jobID, nil + } + // Otherwise, we try to find the latest job ID based on the job name. + pipelineId, err := getPipelineId(opts) + if err != nil { + return 0, fmt.Errorf("get pipeline: %w", err) + } + + jobs, _, err := opts.ApiClient.Jobs.ListPipelineJobs(opts.Repo.FullName(), pipelineId, nil) + if err != nil { + return 0, fmt.Errorf("list pipeline jobs: %w", err) + } + + for _, job := range jobs { + if job.Name == opts.JobName { + return job.ID, nil } } - return jobID, nil + + return 0, fmt.Errorf("pipeline %d contains no jobs", pipelineId) } func getPipelineId(opts *JobOptions) (int, error) { - if opts.PipelineId == 0 { + if opts.PipelineId != 0 { + return opts.PipelineId, nil + } - branch, err := getBranch(opts) - if err != nil { - return 0, err - } + branch, err := getBranch(opts) + if err != nil { + return 0, fmt.Errorf("get branch: %w", err) + } - commit, err := api.GetCommit(opts.ApiClient, opts.Repo.FullName(), branch) - if err != nil { - return 0, err - } - return commit.LastPipeline.ID, nil + commit, err := api.GetCommit(opts.ApiClient, opts.Repo.FullName(), branch) + if err != nil { + return 0, fmt.Errorf("get commit: %w", err) } - return opts.PipelineId, nil + + return commit.LastPipeline.ID, nil } func getBranch(opts *JobOptions) (string, error) { - if opts.Branch == "" { - branch, err := git.CurrentBranch() - if err != nil { - return "", err - } - return branch, nil + if opts.Branch != "" { + return opts.Branch, nil } - return opts.Branch, nil + + branch, err := git.CurrentBranch() + if err != nil { + return "", err + } + + return branch, nil } func getJobIdInteractive(opts *JobOptions) (int, error) { @@ -280,7 +281,7 @@ func TraceJob(opts *JobOptions) error { return err } if jobID < 1 { - fmt.Fprintln(opts.IO.StdErr, "invalid job id:", opts.JobName) + fmt.Fprintln(opts.IO.StdErr, "invalid job ID:", opts.JobName) return cmdutils.SilentError } @@ -290,10 +291,5 @@ func TraceJob(opts *JobOptions) error { } fmt.Fprintln(opts.IO.StdOut) - err = runTrace(context.Background(), opts.ApiClient, opts.IO.StdOut, opts.Repo.FullName(), job, job.Name) - if err != nil { - return err - } - - return nil + return runTrace(context.Background(), opts.ApiClient, opts.IO.StdOut, opts.Repo.FullName(), job, job.Name) } diff --git a/commands/ci/retry/retry.go b/commands/ci/retry/retry.go index ab7b07b7c..e2bf4fd81 100644 --- a/commands/ci/retry/retry.go +++ b/commands/ci/retry/retry.go @@ -21,13 +21,13 @@ func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { Aliases: []string{}, Example: heredoc.Doc(` $ glab ci retry - #=> interactively select a job to retry + #=> Interactively select a job to retry $ glab ci retry 224356863 - #=> retry job with id 224356863 + #=> Retry job with ID 224356863 $ glab ci retry lint - #=> retry job with name lint + #=> Retry job with the name 'lint' `), Long: ``, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/commands/ci/trace/trace.go b/commands/ci/trace/trace.go index 34f1098d9..adf1dba2c 100644 --- a/commands/ci/trace/trace.go +++ b/commands/ci/trace/trace.go @@ -17,13 +17,13 @@ func NewCmdTrace(f *cmdutils.Factory) *cobra.Command { Short: `Trace a CI/CD job log in real time`, Example: heredoc.Doc(` $ glab ci trace - #=> interactively select a job to trace + #=> Interactively select a job to trace $ glab ci trace 224356863 - #=> trace job with id 224356863 + #=> Trace job with ID 224356863 $ glab ci trace lint - #=> trace job with name lint + #=> Trace job with the name 'lint' `), RunE: func(cmd *cobra.Command, args []string) error { var err error -- GitLab From 54db3e4ef29e83a71a2adfc79048a10cc0b92c00 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 19:42:54 +0100 Subject: [PATCH 03/12] feat: remove using #=> in examples --- commands/alias/set/alias_set.go | 4 ++-- commands/ci/ciutils/utils.go | 1 + commands/ci/lint/lint.go | 2 +- commands/ci/retry/retry.go | 6 +++--- commands/ci/trace/trace.go | 6 +++--- commands/project/contributors/repo_contributors.go | 2 +- docs/source/alias/set.md | 4 ++-- docs/source/ci/ci/lint.md | 2 +- docs/source/ci/ci/trace.md | 6 +++--- docs/source/ci/lint.md | 2 +- docs/source/ci/retry.md | 6 +++--- docs/source/ci/trace.md | 6 +++--- docs/source/repo/contributors.md | 2 +- 13 files changed, 25 insertions(+), 24 deletions(-) diff --git a/commands/alias/set/alias_set.go b/commands/alias/set/alias_set.go index 9e7dc7b54..7f55d8998 100644 --- a/commands/alias/set/alias_set.go +++ b/commands/alias/set/alias_set.go @@ -50,7 +50,7 @@ func NewCmdSet(f *cmdutils.Factory, runF func(*SetOptions) error) *cobra.Command Example: heredoc.Doc(` $ glab alias set mrv 'mr view' $ glab mrv -w 123 - #=> glab mr view -w 123 + # glab mr view -w 123 $ glab alias set createissue 'glab create issue --title "$1"' $ glab createissue "My Issue" --description "Something is broken." @@ -58,7 +58,7 @@ func NewCmdSet(f *cmdutils.Factory, runF func(*SetOptions) error) *cobra.Command $ glab alias set --shell igrep 'glab issue list --assignee="$1" | grep $2' $ glab igrep user foo - #=> glab issue list --assignee="user" | grep "foo" + # glab issue list --assignee="user" | grep "foo" `), Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 3dbcb0083..b5b385eb4 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -5,6 +5,7 @@ import ( "fmt" "io" "regexp" + "strconv" "sync" "time" diff --git a/commands/ci/lint/lint.go b/commands/ci/lint/lint.go index 719eb8ebd..6c74729cc 100644 --- a/commands/ci/lint/lint.go +++ b/commands/ci/lint/lint.go @@ -22,7 +22,7 @@ func NewCmdLint(f *cmdutils.Factory) *cobra.Command { Args: cobra.MaximumNArgs(1), Example: heredoc.Doc(` $ glab ci lint - #=> Uses .gitlab-ci.yml in the current directory + # Uses .gitlab-ci.yml in the current directory $ glab ci lint .gitlab-ci.yml diff --git a/commands/ci/retry/retry.go b/commands/ci/retry/retry.go index e2bf4fd81..084a84f36 100644 --- a/commands/ci/retry/retry.go +++ b/commands/ci/retry/retry.go @@ -21,13 +21,13 @@ func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { Aliases: []string{}, Example: heredoc.Doc(` $ glab ci retry - #=> Interactively select a job to retry + # Interactively select a job to retry $ glab ci retry 224356863 - #=> Retry job with ID 224356863 + # Retry job with ID 224356863 $ glab ci retry lint - #=> Retry job with the name 'lint' + # Retry job with the name 'lint' `), Long: ``, RunE: func(cmd *cobra.Command, args []string) error { diff --git a/commands/ci/trace/trace.go b/commands/ci/trace/trace.go index adf1dba2c..42f5efa61 100644 --- a/commands/ci/trace/trace.go +++ b/commands/ci/trace/trace.go @@ -17,13 +17,13 @@ func NewCmdTrace(f *cmdutils.Factory) *cobra.Command { Short: `Trace a CI/CD job log in real time`, Example: heredoc.Doc(` $ glab ci trace - #=> Interactively select a job to trace + # Interactively select a job to trace $ glab ci trace 224356863 - #=> Trace job with ID 224356863 + # Trace job with ID 224356863 $ glab ci trace lint - #=> Trace job with the name 'lint' + # Trace job with the name 'lint' `), RunE: func(cmd *cobra.Command, args []string) error { var err error diff --git a/commands/project/contributors/repo_contributors.go b/commands/project/contributors/repo_contributors.go index d3a7e8fb2..ca16f070d 100644 --- a/commands/project/contributors/repo_contributors.go +++ b/commands/project/contributors/repo_contributors.go @@ -36,7 +36,7 @@ func NewCmdContributors(f *cmdutils.Factory) *cobra.Command { $ glab repo contributors $ glab repo contributors -R gitlab-com/www-gitlab-com - #=> Supports repo override + # Supports repo override `), Args: cobra.ExactArgs(0), Aliases: []string{"users"}, diff --git a/docs/source/alias/set.md b/docs/source/alias/set.md index 6b2e2848c..7fa387b52 100644 --- a/docs/source/alias/set.md +++ b/docs/source/alias/set.md @@ -39,7 +39,7 @@ glab alias set '' [flags] ```plaintext $ glab alias set mrv 'mr view' $ glab mrv -w 123 -#=> glab mr view -w 123 +# glab mr view -w 123 $ glab alias set createissue 'glab create issue --title "$1"' $ glab createissue "My Issue" --description "Something is broken." @@ -47,7 +47,7 @@ $ glab createissue "My Issue" --description "Something is broken." $ glab alias set --shell igrep 'glab issue list --assignee="$1" | grep $2' $ glab igrep user foo -#=> glab issue list --assignee="user" | grep "foo" +# glab issue list --assignee="user" | grep "foo" ``` diff --git a/docs/source/ci/ci/lint.md b/docs/source/ci/ci/lint.md index 6695247e0..5081ce5d2 100644 --- a/docs/source/ci/ci/lint.md +++ b/docs/source/ci/ci/lint.md @@ -21,7 +21,7 @@ glab ci ci lint [flags] ```plaintext $ glab ci lint -#=> Uses .gitlab-ci.yml in the current directory +# Uses .gitlab-ci.yml in the current directory $ glab ci lint .gitlab-ci.yml diff --git a/docs/source/ci/ci/trace.md b/docs/source/ci/ci/trace.md index 68e90ae81..f569f8889 100644 --- a/docs/source/ci/ci/trace.md +++ b/docs/source/ci/ci/trace.md @@ -21,13 +21,13 @@ glab ci ci trace [] [flags] ```plaintext $ glab ci trace -#=> interactively select a job to trace +# Interactively select a job to trace $ glab ci trace 224356863 -#=> trace job with id 224356863 +# Trace job with ID 224356863 $ glab ci trace lint -#=> trace job with name lint +# Trace job with the name 'lint' ``` diff --git a/docs/source/ci/lint.md b/docs/source/ci/lint.md index cc74d6619..15cc9a69f 100644 --- a/docs/source/ci/lint.md +++ b/docs/source/ci/lint.md @@ -21,7 +21,7 @@ glab ci lint [flags] ```plaintext $ glab ci lint -#=> Uses .gitlab-ci.yml in the current directory +# Uses .gitlab-ci.yml in the current directory $ glab ci lint .gitlab-ci.yml diff --git a/docs/source/ci/retry.md b/docs/source/ci/retry.md index 9ee716357..cd98b66ca 100644 --- a/docs/source/ci/retry.md +++ b/docs/source/ci/retry.md @@ -21,13 +21,13 @@ glab ci retry [flags] ```plaintext $ glab ci retry -#=> interactively select a job to retry +# Interactively select a job to retry $ glab ci retry 224356863 -#=> retry job with id 224356863 +# Retry job with ID 224356863 $ glab ci retry lint -#=> retry job with name lint +# Retry job with the name 'lint' ``` diff --git a/docs/source/ci/trace.md b/docs/source/ci/trace.md index e6db795fb..29fd68a6d 100644 --- a/docs/source/ci/trace.md +++ b/docs/source/ci/trace.md @@ -21,13 +21,13 @@ glab ci trace [] [flags] ```plaintext $ glab ci trace -#=> interactively select a job to trace +# Interactively select a job to trace $ glab ci trace 224356863 -#=> trace job with id 224356863 +# Trace job with ID 224356863 $ glab ci trace lint -#=> trace job with name lint +# Trace job with the name 'lint' ``` diff --git a/docs/source/repo/contributors.md b/docs/source/repo/contributors.md index ead22dde5..19aba7d90 100644 --- a/docs/source/repo/contributors.md +++ b/docs/source/repo/contributors.md @@ -29,7 +29,7 @@ users $ glab repo contributors $ glab repo contributors -R gitlab-com/www-gitlab-com -#=> Supports repo override +# Supports repo override ``` -- GitLab From c7189deb1b2bea0b7c9012847fe56fb2195292b4 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 20:02:16 +0100 Subject: [PATCH 04/12] feat: separate inputs from dependencies --- commands/ci/ciutils/utils.go | 51 +++++++++++++++---------------- commands/ci/ciutils/utils_test.go | 12 ++++---- commands/ci/retry/retry.go | 32 +++++++++---------- commands/ci/status/status.go | 5 +-- commands/ci/trace/trace.go | 33 ++++++++++---------- commands/ci/trace/trace_test.go | 3 +- 6 files changed, 68 insertions(+), 68 deletions(-) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index b5b385eb4..250f79a93 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -9,7 +9,6 @@ import ( "sync" "time" - "gitlab.com/gitlab-org/cli/commands/cmdutils" "gitlab.com/gitlab-org/cli/internal/glrepo" "gitlab.com/gitlab-org/cli/pkg/git" "gitlab.com/gitlab-org/cli/pkg/iostreams" @@ -131,19 +130,19 @@ func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid in return nil } -func GetJobId(opts *JobOptions) (int, error) { +func GetJobId(inputs *JobInputs, opts *JobOptions) (int, error) { // If the user hasn't supplied an argument, we display the jobs list interactively. - if opts.JobName == "" { - return getJobIdInteractive(opts) + if inputs.JobName == "" { + return getJobIdInteractive(inputs, opts) } // If the user supplied a job ID, we can use it directly. - if jobID, err := strconv.Atoi(opts.JobName); err == nil { + if jobID, err := strconv.Atoi(inputs.JobName); err == nil { return jobID, nil } // Otherwise, we try to find the latest job ID based on the job name. - pipelineId, err := getPipelineId(opts) + pipelineId, err := getPipelineId(inputs, opts) if err != nil { return 0, fmt.Errorf("get pipeline: %w", err) } @@ -154,7 +153,7 @@ func GetJobId(opts *JobOptions) (int, error) { } for _, job := range jobs { - if job.Name == opts.JobName { + if job.Name == inputs.JobName { return job.ID, nil } } @@ -162,12 +161,12 @@ func GetJobId(opts *JobOptions) (int, error) { return 0, fmt.Errorf("pipeline %d contains no jobs", pipelineId) } -func getPipelineId(opts *JobOptions) (int, error) { - if opts.PipelineId != 0 { - return opts.PipelineId, nil +func getPipelineId(inputs *JobInputs, opts *JobOptions) (int, error) { + if inputs.PipelineId != 0 { + return inputs.PipelineId, nil } - branch, err := getBranch(opts) + branch, err := getBranch(inputs.Branch, opts) if err != nil { return 0, fmt.Errorf("get branch: %w", err) } @@ -180,9 +179,9 @@ func getPipelineId(opts *JobOptions) (int, error) { return commit.LastPipeline.ID, nil } -func getBranch(opts *JobOptions) (string, error) { - if opts.Branch != "" { - return opts.Branch, nil +func getBranch(branch string, opts *JobOptions) (string, error) { + if branch != "" { + return branch, nil } branch, err := git.CurrentBranch() @@ -193,8 +192,8 @@ func getBranch(opts *JobOptions) (string, error) { return branch, nil } -func getJobIdInteractive(opts *JobOptions) (int, error) { - branch, err := getBranch(opts) +func getJobIdInteractive(inputs *JobInputs, opts *JobOptions) (int, error) { + branch, err := getBranch(inputs.Branch, opts) if err != nil { return 0, err } @@ -267,24 +266,24 @@ func getJobIdInteractive(opts *JobOptions) (int, error) { } } -type JobOptions struct { - ApiClient *gitlab.Client - Repo glrepo.Interface +type JobInputs struct { JobName string Branch string PipelineId int - IO *iostreams.IOStreams } -func TraceJob(opts *JobOptions) error { - jobID, err := GetJobId(opts) +type JobOptions struct { + ApiClient *gitlab.Client + Repo glrepo.Interface + IO *iostreams.IOStreams +} + +func TraceJob(inputs *JobInputs, opts *JobOptions) error { + jobID, err := GetJobId(inputs, opts) if err != nil { + fmt.Fprintln(opts.IO.StdErr, "invalid job ID:", inputs.JobName) return err } - if jobID < 1 { - fmt.Fprintln(opts.IO.StdErr, "invalid job ID:", opts.JobName) - return cmdutils.SilentError - } job, err := api.GetPipelineJob(opts.ApiClient, jobID, opts.Repo.FullName()) if err != nil { diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index c12c171b4..f68a8918f 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -9,7 +9,6 @@ import ( "gitlab.com/gitlab-org/cli/commands/cmdtest" "gitlab.com/gitlab-org/cli/pkg/httpmock" "gitlab.com/gitlab-org/cli/pkg/iostreams" - "gitlab.com/gitlab-org/cli/pkg/prompt" ) func TestGetJobId(t *testing.T) { @@ -85,7 +84,7 @@ func TestGetJobId(t *testing.T) { }, }, expectedOut: 1122, - } + }, } for _, tc := range tests { @@ -107,13 +106,14 @@ func TestGetJobId(t *testing.T) { apiClient, _ := f.HttpClient() repo, _ := f.BaseRepo() - output, err := GetJobId(&JobOptions{ - IO: f.IO, - Repo: repo, - ApiClient: apiClient, + output, err := GetJobId(&JobInputs{ JobName: tc.jobName, PipelineId: tc.pipelineId, Branch: "main", + }, &JobOptions{ + IO: f.IO, + Repo: repo, + ApiClient: apiClient, }) require.Nil(t, err) diff --git a/commands/ci/retry/retry.go b/commands/ci/retry/retry.go index 084a84f36..d6399cad5 100644 --- a/commands/ci/retry/retry.go +++ b/commands/ci/retry/retry.go @@ -12,9 +12,6 @@ import ( ) func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { - opts := &ciutils.JobOptions{ - IO: f.IO, - } pipelineRetryCmd := &cobra.Command{ Use: "retry ", Short: `Retry a CI/CD job`, @@ -33,33 +30,36 @@ func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { RunE: func(cmd *cobra.Command, args []string) error { var err error - if len(args) != 0 { - opts.JobName = args[0] - } - repo, err := f.BaseRepo() if err != nil { return err } - opts.Repo = repo - apiClient, err := f.HttpClient() if err != nil { return err } - opts.ApiClient = apiClient - - opts.Branch, _ = cmd.Flags().GetString("branch") - opts.PipelineId, _ = cmd.Flags().GetInt("pipeline-id") - opts.IO = f.IO - jobID, err := ciutils.GetJobId(opts) + jobName := "" + if len(args) != 0 { + jobName = args[0] + } + branch, _ := cmd.Flags().GetString("branch") + pipelineId, _ := cmd.Flags().GetInt("pipeline-id") + jobID, err := ciutils.GetJobId(&ciutils.JobInputs{ + JobName: jobName, + Branch: branch, + PipelineId: pipelineId, + }, &ciutils.JobOptions{ + ApiClient: apiClient, + IO: f.IO, + Repo: repo, + }) if err != nil { return err } if jobID < 1 { - fmt.Fprintln(f.IO.StdErr, "invalid job id:", args[0]) + fmt.Fprintln(f.IO.StdErr, "invalid job ID:", args[0]) return cmdutils.SilentError } diff --git a/commands/ci/status/status.go b/commands/ci/status/status.go index 14276a9ed..c053485bd 100644 --- a/commands/ci/status/status.go +++ b/commands/ci/status/status.go @@ -145,8 +145,9 @@ func NewCmdStatus(f *cmdutils.Factory) *cobra.Command { } } if retry == "View Logs" { - return ciutils.TraceJob(&ciutils.JobOptions{ - Branch: branch, + return ciutils.TraceJob(&ciutils.JobInputs{ + Branch: branch, + }, &ciutils.JobOptions{ Repo: repo, ApiClient: apiClient, IO: f.IO, diff --git a/commands/ci/trace/trace.go b/commands/ci/trace/trace.go index 42f5efa61..8139c9f24 100644 --- a/commands/ci/trace/trace.go +++ b/commands/ci/trace/trace.go @@ -9,9 +9,6 @@ import ( ) func NewCmdTrace(f *cmdutils.Factory) *cobra.Command { - opts := &ciutils.JobOptions{ - IO: f.IO, - } pipelineCITraceCmd := &cobra.Command{ Use: "trace [] [flags]", Short: `Trace a CI/CD job log in real time`, @@ -27,28 +24,30 @@ func NewCmdTrace(f *cmdutils.Factory) *cobra.Command { `), RunE: func(cmd *cobra.Command, args []string) error { var err error - - if len(args) != 0 { - opts.JobName = args[0] - } - repo, err := f.BaseRepo() if err != nil { return err } - opts.Repo = repo - apiClient, err := f.HttpClient() if err != nil { return err } - opts.ApiClient = apiClient - - opts.Branch, _ = cmd.Flags().GetString("branch") - opts.PipelineId, _ = cmd.Flags().GetInt("pipeline-id") - opts.IO = f.IO - - return ciutils.TraceJob(opts) + jobName := "" + if len(args) != 0 { + jobName = args[0] + } + branch, _ := cmd.Flags().GetString("branch") + pipelineId, _ := cmd.Flags().GetInt("pipeline-id") + + return ciutils.TraceJob(&ciutils.JobInputs{ + JobName: jobName, + Branch: branch, + PipelineId: pipelineId, + }, &ciutils.JobOptions{ + ApiClient: apiClient, + IO: f.IO, + Repo: repo, + }) }, } diff --git a/commands/ci/trace/trace_test.go b/commands/ci/trace/trace_test.go index a7895233b..7e2a53724 100644 --- a/commands/ci/trace/trace_test.go +++ b/commands/ci/trace/trace_test.go @@ -60,7 +60,8 @@ func TestCiTrace(t *testing.T) { }, }, expectedOut: "\nGetting job trace...\nShowing logs for lint job #1122\nLorem ipsum", - }, { + }, + { name: "when trace for job-name is requested", args: "lint -b main -p 123", httpMocks: []httpMock{ -- GitLab From 808ef809c6571ff85d4dcc47d7494d17f4a9aff6 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 21:04:19 +0100 Subject: [PATCH 05/12] feat: add error tests --- commands/ci/ciutils/utils_test.go | 195 ++++++++++++++++++-- commands/ci/retry/retry.go | 5 +- commands/ci/retry/retry_test.go | 71 +++++-- commands/ci/trace/trace_integration_test.go | 96 ---------- commands/ci/trace/trace_test.go | 72 ++++++-- 5 files changed, 297 insertions(+), 142 deletions(-) delete mode 100644 commands/ci/trace/trace_integration_test.go diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index f68a8918f..028a4d655 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -20,21 +20,23 @@ func TestGetJobId(t *testing.T) { } tests := []struct { - name string - jobName string - pipelineId int - httpMocks []httpMock - expectedOut int + name string + jobName string + pipelineId int + httpMocks []httpMock + expectedOut int + expectedError string }{ { name: "when getJobId with integer is requested", jobName: "1122", - httpMocks: []httpMock{}, expectedOut: 1122, + httpMocks: []httpMock{}, }, { - name: "when getJobId with name and pipelineId is requested", - jobName: "lint", - pipelineId: 123, + name: "when getJobId with name and pipelineId is requested", + jobName: "lint", + pipelineId: 123, + expectedOut: 1122, httpMocks: []httpMock{ { http.MethodGet, @@ -51,8 +53,20 @@ func TestGetJobId(t *testing.T) { }]`, }, }, - - expectedOut: 1122, + }, { + name: "when getJobId with name and pipelineId is requested and listJobs throws error", + jobName: "lint", + pipelineId: 123, + expectedError: "list pipeline jobs: GET https://gitlab.com/api/v4/projects/OWNER/REPO/pipelines/123/jobs: 403 ", + expectedOut: 0, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusForbidden, + `{}`, + }, + }, }, { name: "when getJobId with name and last pipeline is requested", jobName: "lint", @@ -84,6 +98,44 @@ func TestGetJobId(t *testing.T) { }, }, expectedOut: 1122, + }, { + name: "when getJobId with name and last pipeline is requested and getCommits throws error", + jobName: "lint", + pipelineId: 0, + expectedError: "get pipeline: get commit: GET https://gitlab.com/api/v4/projects/OWNER/REPO/repository/commits/main: 403 ", + expectedOut: 0, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/repository/commits/main", + http.StatusForbidden, + `{}`, + }, + }, + }, { + name: "when getJobId with name and last pipeline is requested and getJobs throws error", + jobName: "lint", + pipelineId: 0, + expectedError: "list pipeline jobs: GET https://gitlab.com/api/v4/projects/OWNER/REPO/pipelines/123/jobs: 403 ", + expectedOut: 0, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/repository/commits/main", + http.StatusOK, + `{ + "last_pipeline" : { + "id": 123 + } + }`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusForbidden, + `{}`, + }, + }, }, } @@ -116,9 +168,126 @@ func TestGetJobId(t *testing.T) { ApiClient: apiClient, }) - require.Nil(t, err) - + if tc.expectedError == "" { + require.Nil(t, err) + } else { + require.NotNil(t, err) + require.Equal(t, tc.expectedError, err.Error()) + } assert.Equal(t, tc.expectedOut, output) }) } } + +func TestTraceJob(t *testing.T) { + type httpMock struct { + method string + path string + status int + body string + } + + tests := []struct { + name string + jobName string + pipelineId int + httpMocks []httpMock + expectedError string + }{ + { + name: "when traceJob is requested", + jobName: "1122", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusOK, + `Lorem ipsum`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + }, + }, + { + name: "when traceJob is requested and getJob throws error", + jobName: "1122", + expectedError: "GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusForbidden, + `{}`, + }, + }, + }, { + name: "when traceJob is requested and getJob throws error", + jobName: "1122", + expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122/trace: 403 ", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusForbidden, + `{}`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fakeHTTP := &httpmock.Mocker{ + MatchURL: httpmock.PathAndQuerystring, + } + defer fakeHTTP.Verify(t) + + for _, mock := range tc.httpMocks { + fakeHTTP.RegisterResponder(mock.method, mock.path, httpmock.NewStringResponse(mock.status, mock.body)) + } + + ios, _, _, _ := iostreams.Test() + f := cmdtest.InitFactory(ios, fakeHTTP) + + _, _ = f.HttpClient() + + apiClient, _ := f.HttpClient() + repo, _ := f.BaseRepo() + + err := TraceJob(&JobInputs{ + JobName: tc.jobName, + PipelineId: tc.pipelineId, + Branch: "main", + }, &JobOptions{ + IO: f.IO, + Repo: repo, + ApiClient: apiClient, + }) + + if tc.expectedError == "" { + require.Nil(t, err) + } else { + require.NotNil(t, err) + require.Equal(t, tc.expectedError, err.Error()) + } + }) + } +} diff --git a/commands/ci/retry/retry.go b/commands/ci/retry/retry.go index d6399cad5..d383c50be 100644 --- a/commands/ci/retry/retry.go +++ b/commands/ci/retry/retry.go @@ -56,11 +56,8 @@ func NewCmdRetry(f *cmdutils.Factory) *cobra.Command { Repo: repo, }) if err != nil { - return err - } - if jobID < 1 { fmt.Fprintln(f.IO.StdErr, "invalid job ID:", args[0]) - return cmdutils.SilentError + return err } job, err := api.RetryPipelineJob(apiClient, jobID, repo.FullName()) diff --git a/commands/ci/retry/retry_test.go b/commands/ci/retry/retry_test.go index efceb9b1f..a4470748f 100644 --- a/commands/ci/retry/retry_test.go +++ b/commands/ci/retry/retry_test.go @@ -33,14 +33,17 @@ func TestCiRetry(t *testing.T) { } tests := []struct { - name string - args string - httpMocks []httpMock - expectedOut string + name string + args string + httpMocks []httpMock + expectedError string + expectedStderr string + expectedOut string }{ { - name: "when retry with job-id", - args: "1122", + name: "when retry with job-id", + args: "1122", + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", httpMocks: []httpMock{ { http.MethodPost, @@ -60,11 +63,25 @@ func TestCiRetry(t *testing.T) { }`, }, }, - expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", }, { - name: "when retry with job-name", - args: "lint -b main -p 123", + name: "when retry with job-id throws error", + args: "1122", + expectedError: "POST https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122/retry: 403 ", + expectedOut: "", + httpMocks: []httpMock{ + { + http.MethodPost, + "/api/v4/projects/OWNER/REPO/jobs/1122/retry", + http.StatusForbidden, + `{}`, + }, + }, + }, + { + name: "when retry with job-name", + args: "lint -b main -p 123", + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", httpMocks: []httpMock{ { http.MethodPost, @@ -98,11 +115,26 @@ func TestCiRetry(t *testing.T) { }]`, }, }, - expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", }, { - name: "when retry with job-name and last pipeline", - args: "lint -b main", + name: "when retry with job-name throws error", + args: "lint -b main -p 123", + expectedError: "list pipeline jobs: GET https://gitlab.com/api/v4/projects/OWNER/REPO/pipelines/123/jobs: 403 ", + expectedStderr: "invalid job ID: lint\n", + httpMocks: []httpMock{ + + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", + http.StatusForbidden, + `{}`, + }, + }, + }, + { + name: "when retry with job-name and last pipeline", + args: "lint -b main", + expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", httpMocks: []httpMock{ { http.MethodPost, @@ -146,7 +178,6 @@ func TestCiRetry(t *testing.T) { }]`, }, }, - expectedOut: "Retried job (ID: 1123 ), status: pending , ref: branch-name , weburl: https://gitlab.com/OWNER/REPO/-/jobs/1123 )\n", }, } @@ -162,10 +193,20 @@ func TestCiRetry(t *testing.T) { } output, err := runCommand(fakeHTTP, tc.args) - require.Nil(t, err) + + if tc.expectedError == "" { + require.Nil(t, err) + } else { + require.NotNil(t, err) + require.Equal(t, tc.expectedError, err.Error()) + } assert.Equal(t, tc.expectedOut, output.String()) - assert.Empty(t, output.Stderr()) + if tc.expectedStderr != "" { + assert.Equal(t, tc.expectedStderr, output.Stderr()) + } else { + assert.Empty(t, output.Stderr()) + } }) } } diff --git a/commands/ci/trace/trace_integration_test.go b/commands/ci/trace/trace_integration_test.go deleted file mode 100644 index 5e629ed51..000000000 --- a/commands/ci/trace/trace_integration_test.go +++ /dev/null @@ -1,96 +0,0 @@ -package trace - -import ( - "bytes" - "testing" - - "gitlab.com/gitlab-org/cli/test" - - "gitlab.com/gitlab-org/cli/pkg/iostreams" - - "github.com/google/shlex" - "github.com/stretchr/testify/assert" - "gitlab.com/gitlab-org/cli/pkg/prompt" - - "github.com/spf13/cobra" - "gitlab.com/gitlab-org/cli/commands/cmdutils" - - "gitlab.com/gitlab-org/cli/commands/cmdtest" -) - -var ( - stubFactory *cmdutils.Factory - cmd *cobra.Command - stdout *bytes.Buffer -) - -func TestMain(m *testing.M) { - cmdtest.InitTest(m, "ci_trace_test") -} - -func TestTraceRun(t *testing.T) { - glTestHost := test.GetHostOrSkip(t) - - var io *iostreams.IOStreams - io, _, stdout, _ = iostreams.Test() - stubFactory = cmdtest.StubFactory(glTestHost + "/cli-automated-testing/test.git") - stubFactory.IO = io - stubFactory.IO.IsaTTY = true - stubFactory.IO.IsErrTTY = true - - tests := []struct { - desc string - args string - assertContains func(t *testing.T, out string) - }{ - { - desc: "Has no arg", - args: ``, - assertContains: func(t *testing.T, out string) { - assert.Contains(t, out, "Getting job trace...") - assert.Contains(t, out, "Showing logs for ") - assert.Contains(t, out, `Preparing the "docker+machine"`) - assert.Contains(t, out, `$ echo "This is a after script section"`) - assert.Contains(t, out, "Job succeeded") - }, - }, - { - desc: "Has arg with job-id", - args: `3509632552`, - assertContains: func(t *testing.T, out string) { - assert.Contains(t, out, "Getting job trace...\n") - }, - }, - { - desc: "On a specified repo with job ID", - args: "3509632552 -X cli-automated-testing/test", - assertContains: func(t *testing.T, out string) { - assert.Contains(t, out, "Getting job trace...\n") - }, - }, - } - - cmd = NewCmdTrace(stubFactory) - cmd.Flags().StringP("repo", "X", "", "") - - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - if tt.args == "" { - as, teardown := prompt.InitAskStubber() - defer teardown() - - as.StubOne("cleanup4 (3509632552) - success") - } - argv, err := shlex.Split(tt.args) - if err != nil { - t.Fatal(err) - } - cmd.SetArgs(argv) - _, err = cmd.ExecuteC() - if err != nil { - t.Fatal(err) - } - tt.assertContains(t, stdout.String()) - }) - } -} diff --git a/commands/ci/trace/trace_test.go b/commands/ci/trace/trace_test.go index 7e2a53724..21370a039 100644 --- a/commands/ci/trace/trace_test.go +++ b/commands/ci/trace/trace_test.go @@ -33,14 +33,16 @@ func TestCiTrace(t *testing.T) { } tests := []struct { - name string - args string - httpMocks []httpMock - expectedOut string + name string + args string + httpMocks []httpMock + expectedOut string + expectedError string }{ { - name: "when trace for job-id is requested", - args: "1122", + name: "when trace for job-id is requested", + args: "1122", + expectedOut: "\nGetting job trace...\nShowing logs for lint job #1122\nLorem ipsum", httpMocks: []httpMock{ { http.MethodGet, @@ -59,11 +61,48 @@ func TestCiTrace(t *testing.T) { }`, }, }, - expectedOut: "\nGetting job trace...\nShowing logs for lint job #1122\nLorem ipsum", }, { - name: "when trace for job-name is requested", - args: "lint -b main -p 123", + name: "when trace for job-id is requested and getTrace throws error", + args: "1122", + expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122/trace: 403 ", + expectedOut: "\nGetting job trace...\n", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusForbidden, + `{}`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "success" + }`, + }, + }, + }, + { + name: "when trace for job-id is requested and getJob throws error", + args: "1122", + expectedError: "GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusForbidden, + `{}`, + }, + }, + }, + { + name: "when trace for job-name is requested", + args: "lint -b main -p 123", + expectedOut: "\nGetting job trace...\n", httpMocks: []httpMock{ { http.MethodGet, @@ -96,11 +135,11 @@ func TestCiTrace(t *testing.T) { }]`, }, }, - expectedOut: "\nGetting job trace...\n", }, { - name: "when trace for job-name and last pipeline is requested", - args: "lint -b main", + name: "when trace for job-name and last pipeline is requested", + args: "lint -b main", + expectedOut: "\nGetting job trace...\n", httpMocks: []httpMock{ { http.MethodGet, @@ -143,7 +182,6 @@ func TestCiTrace(t *testing.T) { }]`, }, }, - expectedOut: "\nGetting job trace...\n", }, } @@ -159,7 +197,13 @@ func TestCiTrace(t *testing.T) { } output, err := runCommand(fakeHTTP, tc.args) - require.Nil(t, err) + + if tc.expectedError == "" { + require.Nil(t, err) + } else { + require.NotNil(t, err) + require.Equal(t, tc.expectedError, err.Error()) + } assert.Equal(t, tc.expectedOut, output.String()) assert.Empty(t, output.Stderr()) -- GitLab From df27ec41d44105105146bc788f31f32b03697252 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 21:15:40 +0100 Subject: [PATCH 06/12] feat: use getPipelineId --- commands/ci/ciutils/utils.go | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 250f79a93..87c33983c 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -175,7 +175,6 @@ func getPipelineId(inputs *JobInputs, opts *JobOptions) (int, error) { if err != nil { return 0, fmt.Errorf("get commit: %w", err) } - return commit.LastPipeline.ID, nil } @@ -193,20 +192,14 @@ func getBranch(branch string, opts *JobOptions) (string, error) { } func getJobIdInteractive(inputs *JobInputs, opts *JobOptions) (int, error) { - branch, err := getBranch(inputs.Branch, opts) - if err != nil { - return 0, err - } - fmt.Fprintf(opts.IO.StdOut, "\nSearching for latest pipeline on %s...\n", branch) - - pipeline, err := api.GetLastPipeline(opts.ApiClient, opts.Repo.FullName(), branch) + pipelineId, err := getPipelineId(inputs, opts) if err != nil { return 0, err } - fmt.Fprintf(opts.IO.StdOut, "Getting jobs for pipeline %d...\n\n", pipeline.ID) + fmt.Fprintf(opts.IO.StdOut, "Getting jobs for pipeline %d...\n\n", pipelineId) - jobs, err := api.GetPipelineJobs(opts.ApiClient, pipeline.ID, opts.Repo.FullName()) + jobs, err := api.GetPipelineJobs(opts.ApiClient, pipelineId, opts.Repo.FullName()) if err != nil { return 0, err } @@ -239,6 +232,10 @@ func getJobIdInteractive(inputs *JobInputs, opts *JobOptions) (int, error) { } else if len(jobs) > 0 { return jobs[0].ID, nil } else { + pipeline, err := api.GetPipeline(opts.ApiClient, pipelineId, nil, opts.Repo.FullName()) + if err != nil { + return 0, err + } // use commit statuses to show external jobs cs, err := api.GetCommitStatuses(opts.ApiClient, opts.Repo.FullName(), pipeline.SHA) if err != nil { -- GitLab From b9c3892e7b3e3c5ba6ef77ea506eabd8c15a76a1 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 21:19:10 +0100 Subject: [PATCH 07/12] feat: gofumpt --- commands/ci/ciutils/utils_test.go | 3 ++- commands/ci/retry/retry_test.go | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index 028a4d655..5802a06c1 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -228,7 +228,8 @@ func TestTraceJob(t *testing.T) { `{}`, }, }, - }, { + }, + { name: "when traceJob is requested and getJob throws error", jobName: "1122", expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122/trace: 403 ", diff --git a/commands/ci/retry/retry_test.go b/commands/ci/retry/retry_test.go index a4470748f..71c9d6226 100644 --- a/commands/ci/retry/retry_test.go +++ b/commands/ci/retry/retry_test.go @@ -122,7 +122,6 @@ func TestCiRetry(t *testing.T) { expectedError: "list pipeline jobs: GET https://gitlab.com/api/v4/projects/OWNER/REPO/pipelines/123/jobs: 403 ", expectedStderr: "invalid job ID: lint\n", httpMocks: []httpMock{ - { http.MethodGet, "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs", -- GitLab From 2a7b7afe027122c3018ef0f4c70f6d3cd62c48f3 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 22:55:38 +0100 Subject: [PATCH 08/12] fix: check updated job status --- commands/ci/ciutils/utils.go | 23 +++++++++-------------- commands/ci/ciutils/utils_test.go | 2 +- commands/ci/trace/trace_test.go | 15 ++++++++------- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 87c33983c..35141da3d 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -85,17 +85,17 @@ func RunTraceSha(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid if err != nil || job == nil { return errors.Wrap(err, "failed to find job") } - return runTrace(ctx, apiClient, w, pid, job, name) + return runTrace(ctx, apiClient, w, pid, job.ID) } -func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid interface{}, job *gitlab.Job, name string) error { +func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid interface{}, jobId int) error { fmt.Fprintln(w, "Getting job trace...") for range time.NewTicker(time.Second * 3).C { if ctx.Err() == context.Canceled { break } - trace, _, err := apiClient.Jobs.GetTraceFile(pid, job.ID) - if err != nil || trace == nil { + job, _, err := apiClient.Jobs.GetJob(pid, jobId) + if err != nil { return errors.Wrap(err, "failed to find job") } switch job.Status { @@ -109,11 +109,12 @@ func runTrace(ctx context.Context, apiClient *gitlab.Client, w io.Writer, pid in fmt.Fprintf(w, "%s has been skipped\n", job.Name) } once.Do(func() { - if name == "" { - name = job.Name - } fmt.Fprintf(w, "Showing logs for %s job #%d\n", job.Name, job.ID) }) + trace, _, err := apiClient.Jobs.GetTraceFile(pid, jobId) + if err != nil || trace == nil { + return errors.Wrap(err, "failed to find job") + } _, _ = io.CopyN(io.Discard, trace, offset) lenT, err := io.Copy(w, trace) if err != nil { @@ -281,12 +282,6 @@ func TraceJob(inputs *JobInputs, opts *JobOptions) error { fmt.Fprintln(opts.IO.StdErr, "invalid job ID:", inputs.JobName) return err } - - job, err := api.GetPipelineJob(opts.ApiClient, jobID, opts.Repo.FullName()) - if err != nil { - return err - } fmt.Fprintln(opts.IO.StdOut) - - return runTrace(context.Background(), opts.ApiClient, opts.IO.StdOut, opts.Repo.FullName(), job, job.Name) + return runTrace(context.Background(), opts.ApiClient, opts.IO.StdOut, opts.Repo.FullName(), jobID) } diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index 5802a06c1..869e5801c 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -219,7 +219,7 @@ func TestTraceJob(t *testing.T) { { name: "when traceJob is requested and getJob throws error", jobName: "1122", - expectedError: "GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", + expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", httpMocks: []httpMock{ { http.MethodGet, diff --git a/commands/ci/trace/trace_test.go b/commands/ci/trace/trace_test.go index 21370a039..a4345e858 100644 --- a/commands/ci/trace/trace_test.go +++ b/commands/ci/trace/trace_test.go @@ -68,12 +68,6 @@ func TestCiTrace(t *testing.T) { expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122/trace: 403 ", expectedOut: "\nGetting job trace...\n", httpMocks: []httpMock{ - { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/jobs/1122/trace", - http.StatusForbidden, - `{}`, - }, { http.MethodGet, "/api/v4/projects/OWNER/REPO/jobs/1122", @@ -84,12 +78,19 @@ func TestCiTrace(t *testing.T) { "status": "success" }`, }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusForbidden, + `{}`, + }, }, }, { name: "when trace for job-id is requested and getJob throws error", args: "1122", - expectedError: "GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", + expectedError: "failed to find job: GET https://gitlab.com/api/v4/projects/OWNER/REPO/jobs/1122: 403 ", + expectedOut: "\nGetting job trace...\n", httpMocks: []httpMock{ { http.MethodGet, -- GitLab From 80ecd2c8cdf3bec9c71aab0eff77418b0a12e796 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Fri, 1 Dec 2023 22:55:54 +0100 Subject: [PATCH 09/12] fix: prevent fault if no pipeline exists --- commands/ci/ciutils/utils.go | 6 +++--- commands/ci/ciutils/utils_test.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/commands/ci/ciutils/utils.go b/commands/ci/ciutils/utils.go index 35141da3d..0a50f780e 100644 --- a/commands/ci/ciutils/utils.go +++ b/commands/ci/ciutils/utils.go @@ -172,11 +172,11 @@ func getPipelineId(inputs *JobInputs, opts *JobOptions) (int, error) { return 0, fmt.Errorf("get branch: %w", err) } - commit, err := api.GetCommit(opts.ApiClient, opts.Repo.FullName(), branch) + pipeline, err := api.GetLastPipeline(opts.ApiClient, opts.Repo.FullName(), branch) if err != nil { - return 0, fmt.Errorf("get commit: %w", err) + return 0, fmt.Errorf("get last pipeline: %w", err) } - return commit.LastPipeline.ID, nil + return pipeline.ID, err } func getBranch(branch string, opts *JobOptions) (string, error) { diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index 869e5801c..c9739f14b 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -102,7 +102,7 @@ func TestGetJobId(t *testing.T) { name: "when getJobId with name and last pipeline is requested and getCommits throws error", jobName: "lint", pipelineId: 0, - expectedError: "get pipeline: get commit: GET https://gitlab.com/api/v4/projects/OWNER/REPO/repository/commits/main: 403 ", + expectedError: "get pipeline: get last pipeline: GET https://gitlab.com/api/v4/projects/OWNER/REPO/repository/commits/main: 403 ", expectedOut: 0, httpMocks: []httpMock{ { -- GitLab From 72d8bb24c6e6c385674b3ab4b04b8b090e922bd7 Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Sun, 3 Dec 2023 22:31:08 +0100 Subject: [PATCH 10/12] feat: add test for interactive job selection --- commands/ci/ciutils/utils_test.go | 59 +++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index c9739f14b..f13f124a7 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -9,6 +9,7 @@ import ( "gitlab.com/gitlab-org/cli/commands/cmdtest" "gitlab.com/gitlab-org/cli/pkg/httpmock" "gitlab.com/gitlab-org/cli/pkg/iostreams" + "gitlab.com/gitlab-org/cli/pkg/prompt" ) func TestGetJobId(t *testing.T) { @@ -18,12 +19,17 @@ func TestGetJobId(t *testing.T) { status int body string } + type askStub struct { + question string + answer string + } tests := []struct { name string jobName string pipelineId int httpMocks []httpMock + askOneStubs []string expectedOut int expectedError string }{ @@ -136,6 +142,50 @@ func TestGetJobId(t *testing.T) { `{}`, }, }, + }, { + name: "when getJobId with pipelineId is requested, ask for job and answer", + jobName: "", + pipelineId: 123, + expectedOut: 1122, + askOneStubs: []string{"lint (1122) - failed"}, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs?per_page=100", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, + }, { + name: "when getJobId with pipelineId is requested, ask for job and give no answer", + jobName: "", + pipelineId: 123, + expectedOut: 1122, + askOneStubs: []string{""}, + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER%2FREPO/pipelines/123/jobs?per_page=100", + http.StatusOK, + `[{ + "id": 1122, + "name": "lint", + "status": "failed" + }, { + "id": 1124, + "name": "publish", + "status": "failed" + }]`, + }, + }, }, } @@ -144,6 +194,15 @@ func TestGetJobId(t *testing.T) { fakeHTTP := &httpmock.Mocker{ MatchURL: httpmock.PathAndQuerystring, } + + if tc.askOneStubs != nil { + as, restoreAsk := prompt.InitAskStubber() + defer restoreAsk() + for _, value := range tc.askOneStubs { + as.StubOne(value) + } + } + defer fakeHTTP.Verify(t) for _, mock := range tc.httpMocks { -- GitLab From af99dc6b20f73477f4ceecd5b82cd0c7612004fd Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Mon, 4 Dec 2023 18:49:40 +0100 Subject: [PATCH 11/12] fix: remove lint issue --- commands/ci/ciutils/utils_test.go | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index f13f124a7..540618fcd 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -19,10 +19,6 @@ func TestGetJobId(t *testing.T) { status int body string } - type askStub struct { - question string - answer string - } tests := []struct { name string @@ -275,6 +271,28 @@ func TestTraceJob(t *testing.T) { }, }, }, + { + name: "when traceJob is requested and is manual", + jobName: "1122", + httpMocks: []httpMock{ + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122/trace", + http.StatusOK, + `Lorem ipsum`, + }, + { + http.MethodGet, + "/api/v4/projects/OWNER/REPO/jobs/1122", + http.StatusOK, + `{ + "id": 1122, + "name": "lint", + "status": "manual" + }`, + }, + }, + }, { name: "when traceJob is requested and getJob throws error", jobName: "1122", -- GitLab From 5e78033e0f55ab1a1f044614f86c5af8bef2d40a Mon Sep 17 00:00:00 2001 From: Andreas Weber Date: Tue, 5 Dec 2023 20:29:59 +0100 Subject: [PATCH 12/12] feat: remove test for manuel --- commands/ci/ciutils/utils_test.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/commands/ci/ciutils/utils_test.go b/commands/ci/ciutils/utils_test.go index 540618fcd..c24d46c58 100644 --- a/commands/ci/ciutils/utils_test.go +++ b/commands/ci/ciutils/utils_test.go @@ -271,28 +271,6 @@ func TestTraceJob(t *testing.T) { }, }, }, - { - name: "when traceJob is requested and is manual", - jobName: "1122", - httpMocks: []httpMock{ - { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/jobs/1122/trace", - http.StatusOK, - `Lorem ipsum`, - }, - { - http.MethodGet, - "/api/v4/projects/OWNER/REPO/jobs/1122", - http.StatusOK, - `{ - "id": 1122, - "name": "lint", - "status": "manual" - }`, - }, - }, - }, { name: "when traceJob is requested and getJob throws error", jobName: "1122", -- GitLab