From 6e43d0a8f5574ecf883133ea834134b1737a5cb3 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Wed, 19 Feb 2025 09:19:07 +0300 Subject: [PATCH 1/4] refactor(ci status): handle global repo flag explicitly The handling of global repo flag was broken for `ci status` in https://gitlab.com/gitlab-org/cli/-/merge_requests/1849 while implementing repo lookup from tracked branch. The reason why everybody missed the global flag break is that it is handled implicitly in this helper function https://gitlab.com/gitlab-org/cli/-/blob/4170977c848880085099f1516d84ace0e6510fcb/commands/cmdutils/repo_override.go#L12 Even if that pathway was covered by tests, there is no way for an external contributor to know all the details and make changes, so my proposal is to simplify the code. Finally repo/branch detection logic can be moved into simple function to add tests without HTTP mocks. --- commands/ci/status/status.go | 57 +++++++++++++++++++------------ commands/cmdutils/global_flags.go | 32 +++++++++++++++++ 2 files changed, 68 insertions(+), 21 deletions(-) create mode 100644 commands/cmdutils/global_flags.go diff --git a/commands/ci/status/status.go b/commands/ci/status/status.go index 94e8f2eec..f37fc53ea 100644 --- a/commands/ci/status/status.go +++ b/commands/ci/status/status.go @@ -50,35 +50,48 @@ func NewCmdStatus(f *cmdutils.Factory) *cobra.Command { live, _ := cmd.Flags().GetBool("live") compact, _ := cmd.Flags().GetBool("compact") - if branch == "" { - branch, err = git.CurrentBranch() - if err != nil { - return err - } - dbg.Debug("Current branch:", branch) - } - var repo glrepo.Interface - branchConfig := git.ReadBranchConfig(branch) - if branchConfig.RemoteName == "" { - repo, err = f.BaseRepo() + repoGlobal, err := cmdutils.GlobalRepo(cmd) + if err != nil { + return err + } + if repoGlobal != "" { + repo, err = glrepo.FromFullName(repoGlobal) if err != nil { return err } } else { - remotes, err := f.Remotes() - if err != nil { - return err - } - repo, err = remotes.FindByName(branchConfig.RemoteName) - if err != nil { - redCheck := c.Red("x") - fmt.Fprintf(f.IO.StdOut, "%s Remote '%s' for branch '%s' is gone.\n", redCheck, branchConfig.RemoteName, branch) - return err + branchConfig := git.ReadBranchConfig(branch) + if branchConfig.RemoteName == "" { + repo, err = f.BaseRepo() + if err != nil { + return err + } + if branch == "" { + branch, err = git.CurrentBranch() + if err != nil { + return err + } + } + } else { + remotes, err := f.Remotes() + if err != nil { + return err + } + repo, err = remotes.FindByName(branchConfig.RemoteName) + if err != nil { + redCheck := c.Red("x") + fmt.Fprintf(f.IO.StdOut, "%s Remote '%s' for branch '%s' is gone.\n", redCheck, branchConfig.RemoteName, branch) + return err + } + // XXX uncomment and test this in another MR + // branch = branchConfig.MergeRef } } + repoName := repo.FullName() - dbg.Debug("Repository:", repoName) + dbg.Debug("Repo:", repoName) + dbg.Debug("Branch:", branch) runningPipeline, err := api.GetLatestPipeline(apiClient, repoName, branch) if err != nil { @@ -179,6 +192,8 @@ func NewCmdStatus(f *cmdutils.Factory) *cobra.Command { }, } + cmdutils.AddGlobalRepoFlag(pipelineStatusCmd) + pipelineStatusCmd.Flags().BoolP("live", "l", false, "Show status in real time until the pipeline ends.") pipelineStatusCmd.Flags().BoolP("compact", "c", false, "Show status in compact format.") pipelineStatusCmd.Flags().StringP("branch", "b", "", "Check pipeline status for a branch. Default: current branch.") diff --git a/commands/cmdutils/global_flags.go b/commands/cmdutils/global_flags.go new file mode 100644 index 000000000..0430705d3 --- /dev/null +++ b/commands/cmdutils/global_flags.go @@ -0,0 +1,32 @@ +package cmdutils + +import ( + "os" + + "github.com/spf13/cobra" + "gitlab.com/gitlab-org/cli/pkg/dbg" +) + +func AddGlobalRepoFlag(cmd *cobra.Command) { + cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository. Can use either `OWNER/REPO` or `GROUP/NAMESPACE/REPO` format. Also accepts full URL or Git URL.") +} + +func GlobalRepo(cmd *cobra.Command) (string, error) { + repo, err := cmd.PersistentFlags().GetString("repo") + if err != nil { + return "", err + } + if repo != "" { + dbg.Debug("Repo from global flag:", repo) + return repo, nil + } + + if repoFromEnv := os.Getenv("GITLAB_REPO"); repoFromEnv != "" { + dbg.Debug("Repo from ENV:", repo) + return repoFromEnv, nil + } + + // XXX cleanup f.RepoOverride() calls + // return f.RepoOverride(repoOverride) + return "", nil +} -- GitLab From 3a08b72a3ffe653441109c9f9e7cf9dda5664019 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Sun, 23 Feb 2025 19:57:25 +0300 Subject: [PATCH 2/4] fix: add global flags to inherited flags --- commands/cmdutils/global_flags.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/commands/cmdutils/global_flags.go b/commands/cmdutils/global_flags.go index 0430705d3..3ea72c011 100644 --- a/commands/cmdutils/global_flags.go +++ b/commands/cmdutils/global_flags.go @@ -8,11 +8,11 @@ import ( ) func AddGlobalRepoFlag(cmd *cobra.Command) { - cmd.PersistentFlags().StringP("repo", "R", "", "Select another repository. Can use either `OWNER/REPO` or `GROUP/NAMESPACE/REPO` format. Also accepts full URL or Git URL.") + cmd.InheritedFlags().StringP("repo", "R", "", "Select another repository. Can use either `OWNER/REPO` or `GROUP/NAMESPACE/REPO` format. Also accepts full URL or Git URL.") } func GlobalRepo(cmd *cobra.Command) (string, error) { - repo, err := cmd.PersistentFlags().GetString("repo") + repo, err := cmd.InheritedFlags().GetString("repo") if err != nil { return "", err } -- GitLab From 61e525fa19bdba278207e810b30922c3d1889a90 Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Wed, 5 Mar 2025 10:31:28 +0300 Subject: [PATCH 3/4] test(ci status): returns error when there is no repo Slowly working towards https://gitlab.com/gitlab-org/cli/-/issues/7747 --- commands/ci/status/status_test.go | 41 +++++++++++++++++++ .../agent/bootstrap/agent_bootstrap_test.go | 5 +-- commands/cmdtest/helper.go | 5 +++ 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 commands/ci/status/status_test.go diff --git a/commands/ci/status/status_test.go b/commands/ci/status/status_test.go new file mode 100644 index 000000000..bec4d73b2 --- /dev/null +++ b/commands/ci/status/status_test.go @@ -0,0 +1,41 @@ +package status + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "gitlab.com/gitlab-org/cli/commands/cmdtest" + "gitlab.com/gitlab-org/cli/commands/cmdutils" +) + +func TestStatus(t *testing.T) { + /* + create fixture + + - no repo + - repo with main + - repo with branch + - repo with branch that is tracking remote + + create tests + + - if -R is given, it overrides repo + - if branch is given, set branch + - if local repo exists, branch from local + - no repo, default branch + + - no -R, take current repo branch + - if branch param, use param + - if branch tracks remote, use remote + */ + tempDir := t.TempDir() + t.Chdir(tempDir) + + cmd := NewCmdStatus(cmdutils.NewFactory()) + cmdtest.CmdSetup(cmd) + // test - no repo + err := cmd.Execute() + assert.Error(t, err) + // t.Log(os.Getwd()) + +} diff --git a/commands/cluster/agent/bootstrap/agent_bootstrap_test.go b/commands/cluster/agent/bootstrap/agent_bootstrap_test.go index cd6073956..8fafc4d30 100644 --- a/commands/cluster/agent/bootstrap/agent_bootstrap_test.go +++ b/commands/cluster/agent/bootstrap/agent_bootstrap_test.go @@ -10,6 +10,7 @@ import ( "github.com/google/shlex" gitlab "gitlab.com/gitlab-org/api/client-go" glab_api "gitlab.com/gitlab-org/cli/api" + "gitlab.com/gitlab-org/cli/commands/cmdtest" "gitlab.com/gitlab-org/cli/commands/cmdutils" "gitlab.com/gitlab-org/cli/internal/glrepo" "gitlab.com/gitlab-org/cli/pkg/iostreams" @@ -863,9 +864,7 @@ func setupCmdExec(t *testing.T) (execFunc, *MockAPI, *MockWriter, *MockWriter, * cmd.SetOut(mockStdout) cmd.SetErr(mockStderr) - // Set on root cmd, thus we also need to set it here. - // cmd.SilenceErrors = true - cmd.SilenceUsage = true + cmdtest.CmdSetup(cmd) exec := func(cli string) error { argv, err := shlex.Split(cli) diff --git a/commands/cmdtest/helper.go b/commands/cmdtest/helper.go index f537b9fe4..fe3c8d16d 100644 --- a/commands/cmdtest/helper.go +++ b/commands/cmdtest/helper.go @@ -53,6 +53,11 @@ func init() { } } +// Sets root command defaults for tests +func CmdSetup(cmd *cobra.Command) { + cmd.SilenceUsage = true +} + func InitTest(m *testing.M, suffix string) { // Build a glab binary with test symbols. If the parent test binary was run // with coverage enabled, enable coverage on the child binary, too. -- GitLab From 94e4f0b57fa7f71e4c683baceee660340aea436f Mon Sep 17 00:00:00 2001 From: Anatoli Babenia Date: Thu, 6 Mar 2025 07:43:59 +0300 Subject: [PATCH 4/4] test(ci status): test global -R flag --- commands/ci/status/status_test.go | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/commands/ci/status/status_test.go b/commands/ci/status/status_test.go index bec4d73b2..c1cd26f36 100644 --- a/commands/ci/status/status_test.go +++ b/commands/ci/status/status_test.go @@ -33,9 +33,15 @@ func TestStatus(t *testing.T) { cmd := NewCmdStatus(cmdutils.NewFactory()) cmdtest.CmdSetup(cmd) - // test - no repo - err := cmd.Execute() - assert.Error(t, err) - // t.Log(os.Getwd()) + t.Run("no repo", func(t *testing.T) { + err := cmd.Execute() + assert.Error(t, err) + }) + t.Run("no repo with -R", func(t *testing.T) { + cmd.SetArgs([]string{"-R", "forkrepo"}) + err := cmd.Execute() + assert.NoError(t, err) + }) + // t.Log(os.Getwd()) } -- GitLab