From 51353f0068306b43e84c44c16f2ca969a5abf116 Mon Sep 17 00:00:00 2001 From: Jesse Tatasciore Date: Tue, 1 Feb 2022 12:36:24 -0500 Subject: [PATCH] fix: Use better function / variable names, use cmd.Use where possible, improve comments, implement leftover TODO --- pkg/aspect/aquery/aquery.go | 7 +++---- pkg/aspect/aquery/aquery_test.go | 12 +++++++----- pkg/aspect/cquery/cquery.go | 7 +++---- pkg/aspect/cquery/cquery_test.go | 12 +++++++----- pkg/aspect/query/query.go | 20 +++++++++++--------- pkg/aspect/query/query_test.go | 13 +++++++------ pkg/aspect/query/shared/query.go | 7 +++++-- 7 files changed, 43 insertions(+), 35 deletions(-) diff --git a/pkg/aspect/aquery/aquery.go b/pkg/aspect/aquery/aquery.go index 987cd85c8..43a691e4b 100644 --- a/pkg/aspect/aquery/aquery.go +++ b/pkg/aspect/aquery/aquery.go @@ -28,7 +28,7 @@ type AQuery struct { } func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *AQuery { - presets := shared.GetPrecannedQueries("aquery") + presets := shared.PrecannedQueries("aquery") return &AQuery{ Streams: streams, @@ -42,13 +42,12 @@ func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *AQuery { } func (q *AQuery) Run(cmd *cobra.Command, args []string) error { - verb := "aquery" presets, presetNames, err := shared.ProcessQueries(q.Presets) if err != nil { return shared.GetPrettyError(cmd, err) } - verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select) + presetVerb, query, runReplacements, err := shared.SelectQuery(cmd.Use, presets, q.Presets, presetNames, q.Streams, args, q.Select) if err != nil { return shared.GetPrettyError(cmd, err) @@ -62,5 +61,5 @@ func (q *AQuery) Run(cmd *cobra.Command, args []string) error { } } - return shared.RunQuery(q.Bzl, verb, query) + return shared.RunQuery(q.Bzl, presetVerb, query) } diff --git a/pkg/aspect/aquery/aquery_test.go b/pkg/aspect/aquery/aquery_test.go index 687b2f140..2f22f0a2a 100644 --- a/pkg/aspect/aquery/aquery_test.go +++ b/pkg/aspect/aquery/aquery_test.go @@ -46,7 +46,8 @@ func TestQuery(t *testing.T) { }, } - g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) + cmd := &cobra.Command{Use: "aquery"} + g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) }) t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) { @@ -93,7 +94,8 @@ func TestQuery(t *testing.T) { Verb: "aquery", }, } - err := q.Run(nil, []string{"why"}) + cmd := &cobra.Command{Use: "aquery"} + err := q.Run(cmd, []string{"why"}) g.Expect(err).To(BeNil()) }) @@ -109,8 +111,6 @@ func TestQuery(t *testing.T) { spawner := bazel_mock.NewMockBazel(ctrl) - cmd := &cobra.Command{Use: "fake"} - promptRunner := query_mock.NewMockPromptRunner(ctrl) gomock.InOrder( promptRunner. @@ -141,6 +141,7 @@ func TestQuery(t *testing.T) { Verb: "aquery", }, } + cmd := &cobra.Command{Use: "aquery"} err := q.Run(cmd, []string{"why"}) g.Expect(err).To(MatchError(expectedError)) }) @@ -212,7 +213,8 @@ func TestQuery(t *testing.T) { Verb: "aquery", }, } - err := q.Run(nil, []string{}) + cmd := &cobra.Command{Use: "aquery"} + err := q.Run(cmd, []string{}) g.Expect(err).To(BeNil()) }) } diff --git a/pkg/aspect/cquery/cquery.go b/pkg/aspect/cquery/cquery.go index e8e8a3942..9c11a9377 100644 --- a/pkg/aspect/cquery/cquery.go +++ b/pkg/aspect/cquery/cquery.go @@ -28,7 +28,7 @@ type CQuery struct { } func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *CQuery { - presets := shared.GetPrecannedQueries("cquery") + presets := shared.PrecannedQueries("cquery") return &CQuery{ Streams: streams, @@ -42,13 +42,12 @@ func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *CQuery { } func (q *CQuery) Run(cmd *cobra.Command, args []string) error { - verb := "cquery" presets, presetNames, err := shared.ProcessQueries(q.Presets) if err != nil { return shared.GetPrettyError(cmd, err) } - verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select) + presetVerb, query, runReplacements, err := shared.SelectQuery(cmd.Use, presets, q.Presets, presetNames, q.Streams, args, q.Select) if err != nil { return shared.GetPrettyError(cmd, err) @@ -62,5 +61,5 @@ func (q *CQuery) Run(cmd *cobra.Command, args []string) error { } } - return shared.RunQuery(q.Bzl, verb, query) + return shared.RunQuery(q.Bzl, presetVerb, query) } diff --git a/pkg/aspect/cquery/cquery_test.go b/pkg/aspect/cquery/cquery_test.go index a38c0dea6..8f0479465 100644 --- a/pkg/aspect/cquery/cquery_test.go +++ b/pkg/aspect/cquery/cquery_test.go @@ -46,7 +46,8 @@ func TestQuery(t *testing.T) { }, } - g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) + cmd := &cobra.Command{Use: "cquery"} + g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) }) t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) { @@ -93,7 +94,8 @@ func TestQuery(t *testing.T) { Verb: "cquery", }, } - err := q.Run(nil, []string{"why"}) + cmd := &cobra.Command{Use: "cquery"} + err := q.Run(cmd, []string{"why"}) g.Expect(err).To(BeNil()) }) @@ -109,8 +111,6 @@ func TestQuery(t *testing.T) { spawner := bazel_mock.NewMockBazel(ctrl) - cmd := &cobra.Command{Use: "fake"} - promptRunner := query_mock.NewMockPromptRunner(ctrl) gomock.InOrder( promptRunner. @@ -141,6 +141,7 @@ func TestQuery(t *testing.T) { Verb: "cquery", }, } + cmd := &cobra.Command{Use: "cquery"} err := q.Run(cmd, []string{"why"}) g.Expect(err).To(MatchError(expectedError)) }) @@ -212,7 +213,8 @@ func TestQuery(t *testing.T) { Verb: "cquery", }, } - err := q.Run(nil, []string{}) + cmd := &cobra.Command{Use: "cquery"} + err := q.Run(cmd, []string{}) g.Expect(err).To(BeNil()) }) } diff --git a/pkg/aspect/query/query.go b/pkg/aspect/query/query.go index 011df19d4..f6a53bc22 100644 --- a/pkg/aspect/query/query.go +++ b/pkg/aspect/query/query.go @@ -39,8 +39,10 @@ type Query struct { } func New(streams ioutils.Streams, bzl bazel.Bazel, isInteractive bool) *Query { - // will potentially be updated during Run() if the user requests that query also show aquery and cquery predefined queries - presets := shared.GetPrecannedQueries("query") + // the list of available preset queries will potentially be updated during the "Run" function. + // if the user requests that query also show aquery and cquery predefined queries then these + // will be added to the list of presets + presets := shared.PrecannedQueries("query") return &Query{ Streams: streams, @@ -73,14 +75,14 @@ func (q *Query) Run(cmd *cobra.Command, args []string) error { return err } - verb := "query" + verb := cmd.Use if q.Prefs.GetBool(useCQuery) { verb = "cquery" } if q.Prefs.GetBool(allowAllQueries) { - q.Presets = shared.GetPrecannedQueries("") + q.Presets = shared.PrecannedQueries("") } presets, presetNames, err := shared.ProcessQueries(q.Presets) @@ -88,7 +90,7 @@ func (q *Query) Run(cmd *cobra.Command, args []string) error { return shared.GetPrettyError(cmd, err) } - verb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select) + presetVerb, query, runReplacements, err := shared.SelectQuery(verb, presets, q.Presets, presetNames, q.Streams, args, q.Select) if err != nil { return shared.GetPrettyError(cmd, err) @@ -102,17 +104,17 @@ func (q *Query) Run(cmd *cobra.Command, args []string) error { } } - return shared.RunQuery(q.Bzl, verb, query) + return shared.RunQuery(q.Bzl, presetVerb, query) } func (q *Query) checkConfig(baseUseKey string, baseInquiredKey string, question string) error { if !q.Prefs.GetBool(baseInquiredKey) { q.Prefs.Set(baseInquiredKey, true) - // if user types in y or Y then err will be nil. Any other input will not result in nil - _, someErr := q.Confirmation(question).Run() + // Y = no error; N = error + _, err := q.Confirmation(question).Run() - q.Prefs.Set(baseUseKey, someErr == nil) + q.Prefs.Set(baseUseKey, err == nil) if err := q.Prefs.WriteConfig(); err != nil { return fmt.Errorf("failed to update config file: %w", err) diff --git a/pkg/aspect/query/query_test.go b/pkg/aspect/query/query_test.go index 7fda72847..15eaa3671 100644 --- a/pkg/aspect/query/query_test.go +++ b/pkg/aspect/query/query_test.go @@ -55,7 +55,8 @@ func TestQuery(t *testing.T) { viper.SetConfigFile(cfg.Name()) q.Prefs = viper - g.Expect(q.Run(nil, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) + cmd := &cobra.Command{Use: "query"} + g.Expect(q.Run(cmd, []string{"why", "//cmd/aspect/query:query", "@com_github_bazelbuild_bazelisk//core:go_default_library"})).Should(Succeed()) }) t.Run("query can be selected by default and will prompt for inputs", func(t *testing.T) { @@ -126,7 +127,8 @@ func TestQuery(t *testing.T) { viper.SetConfigFile(cfg.Name()) q.Prefs = viper - err = q.Run(nil, []string{"why"}) + cmd := &cobra.Command{Use: "query"} + err = q.Run(cmd, []string{"why"}) g.Expect(err).To(BeNil()) }) @@ -142,8 +144,6 @@ func TestQuery(t *testing.T) { spawner := bazel_mock.NewMockBazel(ctrl) - cmd := &cobra.Command{Use: "fake"} - promptRunner := query_mock.NewMockPromptRunner(ctrl) gomock.InOrder( promptRunner. @@ -199,6 +199,7 @@ func TestQuery(t *testing.T) { viper.SetConfigFile(cfg.Name()) q.Prefs = viper + cmd := &cobra.Command{Use: "query"} err = q.Run(cmd, []string{"why"}) g.Expect(err).To(MatchError(expectedError)) }) @@ -257,7 +258,6 @@ func TestQuery(t *testing.T) { Bzl: spawner, IsInteractive: true, Prompt: func(label string) shared.PromptRunner { - fmt.Println("label:", label) g.Expect(strings.Contains(label, "targettwo") || strings.Contains(label, "dependencytwo")).To(Equal(true)) return promptRunner }, @@ -295,7 +295,8 @@ func TestQuery(t *testing.T) { viper.SetConfigFile(cfg.Name()) q.Prefs = viper - err = q.Run(nil, []string{}) + cmd := &cobra.Command{Use: "query"} + err = q.Run(cmd, []string{}) g.Expect(err).To(BeNil()) }) } diff --git a/pkg/aspect/query/shared/query.go b/pkg/aspect/query/shared/query.go index df2f965b5..53e8aa13c 100644 --- a/pkg/aspect/query/shared/query.go +++ b/pkg/aspect/query/shared/query.go @@ -23,6 +23,8 @@ import ( "aspect.build/cli/pkg/ioutils" ) +var placeholderRegex = regexp.MustCompile(`(\?[a-zA-Z]*)`) + type PresetQuery struct { Name string Description string @@ -62,7 +64,7 @@ func Select(presetNames []string) SelectRunner { } } -func GetPrecannedQueries(verb string) []*PresetQuery { +func PrecannedQueries(verb string) []*PresetQuery { // TODO: Queries should be loadable from the plugin config // https://github.com/aspect-build/aspect-cli/issues/98 presets := []*PresetQuery{ @@ -149,10 +151,11 @@ func RunQuery(bzl bazel.Bazel, verb string, query string) error { } func ReplacePlaceholders(query string, args []string, p func(label string) PromptRunner) (string, error) { - placeholders := regexp.MustCompile(`(\?[a-zA-Z]*)`).FindAllString(query, -1) + placeholders := placeholderRegex.FindAllString(query, -1) if len(placeholders) == len(args)-1 { for i, placeholder := range placeholders { + fmt.Printf("%s set to %s\n", strings.Replace(placeholder, "?", "", 1), args[i+1]) // todo.... Print out targetA was set to //foo and targetB was set to //bar query = strings.ReplaceAll(query, placeholder, args[i+1]) }