kepctl query: add test coverage

Add some basic coverage for each "query" field in QueryOpts, aka ignore
IncludePRs and Output

Add some TODO-titled test cases that are skipped for things that seem
like opportunities to improve validation

Author: spiffxp

pkg/repo/query.go

@@ -63,7 +63,7 @@ func (r *Repo) PrepareQueryOpts(opts *QueryOpts) error {
 		}
 
 		if len(sigs) == 0 {
-			return fmt.Errorf("no SIG matches any of the passed regular expressions")
+			return fmt.Errorf("no SIGs match any of: %v", opts.Groups)
 		}
 
 		opts.Groups = sigs
@@ -81,7 +81,7 @@ func (r *Repo) Query(opts *QueryOpts) ([]*api.Proposal, error) {
 
 	err := r.PrepareQueryOpts(opts)
 	if err != nil {
-		return nil, fmt.Errorf("unable to prepare query opts: %w", err)
+		return nil, fmt.Errorf("invalid query options: %w", err)
 	}
 
 	if r.TokenPath != "" {

pkg/repo/query_test.go

@@ -46,7 +46,7 @@ func TestValidateQueryOpt(t *testing.T) {
 				IncludePRs: true,
 				Output:     "json",
 			},
-			err: fmt.Errorf("No SIG matches any of the passed regular expressions"),
+			err: fmt.Errorf("no SIGs match any of: [sig-does-not-exist]"),
 		},
 		{
 			name: "output: unsupported format",
@@ -74,3 +74,264 @@ func TestValidateQueryOpt(t *testing.T) {
 		})
 	}
 }
+
+func TestQuery(t *testing.T) {
+	testcases := map[string][]struct {
+		name      string
+		queryOpts repo.QueryOpts
+		err       error
+		kepNames  []string
+		skip      bool
+	}{
+		"all": {
+			{
+				name:      "defaults",
+				queryOpts: repo.QueryOpts{},
+				kepNames: []string{
+					"1-the-bare-minimum",
+					"123-newstyle",
+					"13-keps-as-crds",
+					"404-question-not-found",
+					"42-the-answer",
+				},
+			},
+			{
+				name: "everything",
+				queryOpts: repo.QueryOpts{
+					Groups: []string{"sig-architecture"},
+					Status: []string{"provisional"},
+					Stage:  []string{"alpha"},
+					// TODO: PRRApprover: []string{""},
+					Author:   []string{"@rjbez17"},
+					Approver: []string{"@liggitt"},
+				},
+				kepNames: []string{
+					"123-newstyle",
+				},
+			},
+		},
+		"groups": {
+			{
+				name: "results",
+				queryOpts: repo.QueryOpts{
+					Groups: []string{"sig-owns-participates"},
+				},
+				kepNames: []string{"1-the-bare-minimum", "13-keps-as-crds"},
+			},
+			{
+				name: "no results",
+				queryOpts: repo.QueryOpts{
+					Groups: []string{"sig-participates-only"},
+				},
+			},
+			{
+				name: "invalid",
+				queryOpts: repo.QueryOpts{
+					Groups: []string{"sig-does-not-exist"},
+				},
+				err: fmt.Errorf("invalid query options: no SIGs match any of: [sig-does-not-exist]"),
+			},
+		},
+		"status": {
+			{
+				name: "results",
+				queryOpts: repo.QueryOpts{
+					Status: []string{"provisional"},
+				},
+				kepNames: []string{
+					"123-newstyle",
+					"13-keps-as-crds",
+				},
+			},
+			{
+				name: "TODO: invalid should error, but instead returns nothing",
+				queryOpts: repo.QueryOpts{
+					Status: []string{"status-does-not-exist"},
+				},
+				err:  fmt.Errorf("something about invalid status"),
+				skip: true,
+			},
+		},
+		"stage": {
+			{
+				name: "empty-string",
+				queryOpts: repo.QueryOpts{
+					Stage: []string{""},
+				},
+				kepNames: []string{
+					"1-the-bare-minimum",
+				},
+			},
+			{
+				name: "alpha",
+				queryOpts: repo.QueryOpts{
+					Stage: []string{"alpha"},
+				},
+				kepNames: []string{
+					"123-newstyle",
+					"42-the-answer",
+				},
+			},
+			{
+				name: "beta",
+				queryOpts: repo.QueryOpts{
+					Stage: []string{"beta"},
+				},
+				kepNames: []string{
+					"404-question-not-found",
+				},
+			},
+			{
+				name: "stable",
+				queryOpts: repo.QueryOpts{
+					Stage: []string{"stable"},
+				},
+				kepNames: []string{
+					"13-keps-as-crds",
+				},
+			},
+			{
+				name: "TODO: invalid should error, but instead returns nothing",
+				queryOpts: repo.QueryOpts{
+					Stage: []string{"stage-does-not-exist"},
+				},
+				err:  fmt.Errorf("something about invalid stage"),
+				skip: true,
+			},
+		},
+		"prr-approver": {
+			{
+				name: "results",
+				queryOpts: repo.QueryOpts{
+					PRRApprover: []string{"@dorothy"},
+				},
+				kepNames: []string{
+					"404-question-not-found",
+					"42-the-answer",
+					"13-keps-as-crds",
+				},
+			},
+			{
+				name: "no results",
+				queryOpts: repo.QueryOpts{
+					PRRApprover: []string{"prr-approves-nothing"},
+				},
+			},
+			{
+				name: "TODO: invalid should error but instead returns nothing",
+				queryOpts: repo.QueryOpts{
+					PRRApprover: []string{"prr-approver-does-not-exist"},
+				},
+				err:  fmt.Errorf("something about invalid prr-approver"),
+				skip: true,
+			},
+		},
+		"author": {
+			{
+				name: "results",
+				queryOpts: repo.QueryOpts{
+					Author: []string{"@alice"},
+				},
+				kepNames: []string{
+					"404-question-not-found",
+					"42-the-answer",
+					"13-keps-as-crds",
+				},
+			},
+			{
+				name: "no results",
+				queryOpts: repo.QueryOpts{
+					Author: []string{"authors-nothing"},
+				},
+			},
+			{
+				name: "TODO: empty-string should error but instead returns nothing",
+				queryOpts: repo.QueryOpts{
+					Author: []string{""},
+				},
+				err:  fmt.Errorf("something about invalid author"),
+				skip: true,
+			},
+		},
+		"approver": {
+			{
+				name: "results",
+				queryOpts: repo.QueryOpts{
+					Approver: []string{"@carolyn"},
+				},
+				kepNames: []string{
+					"404-question-not-found",
+					"42-the-answer",
+					"13-keps-as-crds",
+				},
+			},
+			{
+				name: "no results",
+				queryOpts: repo.QueryOpts{
+					Approver: []string{"approves-nothing"},
+				},
+			},
+			{
+				name: "TODO: empty-string should error but instead returns nothing",
+				queryOpts: repo.QueryOpts{
+					Author: []string{""},
+				},
+				err:  fmt.Errorf("something about invalid approver"),
+				skip: true,
+			},
+		},
+	}
+
+	r := fixture.validRepo
+
+	for group, groupTestcases := range testcases {
+		for _, tc := range groupTestcases {
+			t.Run(fmt.Sprintf("%s/%s", group, tc.name), func(t *testing.T) {
+				if tc.skip {
+					t.Skip("TODO")
+				}
+				// TODO(spiffxp): uh, Query shouldn't care about Output
+				queryOpts := tc.queryOpts
+				queryOpts.Output = "json"
+
+				results, err := r.Query(&queryOpts)
+
+				if tc.err == nil {
+					require.Nil(t, err)
+				} else {
+					// TODO(spiffxp): does this verify not nil, or the actual error?
+					require.Error(t, err, tc.err.Error())
+				}
+
+				if len(tc.kepNames) != len(results) {
+					t.Errorf("expected %v results, found: %v", len(tc.kepNames), len(results))
+				}
+
+				expectedKEPs := make(map[string]bool)
+				for _, name := range tc.kepNames {
+					expectedKEPs[name] = false
+				}
+
+				errs := []error{}
+
+				for _, kep := range results {
+					if _, expected := expectedKEPs[kep.Name]; expected {
+						expectedKEPs[kep.Name] = true
+					} else {
+						errs = append(errs, fmt.Errorf("query returned unexpected kep: %+v", kep))
+					}
+				}
+
+				for _, name := range tc.kepNames {
+					if found := expectedKEPs[name]; !found {
+						errs = append(errs, fmt.Errorf("query did not return expected kep: %v", name))
+					}
+				}
+
+				for _, err := range errs {
+					t.Error(err)
+				}
+			})
+		}
+	}
+}

pkg/repo/repo_test.go

@@ -37,7 +37,7 @@ var fixture = struct {
 }{}
 
 func TestMain(m *testing.M) {
-	err := log.SetupGlobalLogger("debug")
+	err := log.SetupGlobalLogger("info")
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Unable to setup global logger: %v: ", err)
 	}
@@ -48,6 +48,10 @@ func TestMain(m *testing.M) {
 	fetcher := &api.MockGroupFetcher{
 		Groups: []string{
 			"sig-architecture",
+			"sig-does-nothing",
+			"sig-owns-only",
+			"sig-owns-participates",
+			"sig-participates-only",
 		},
 	}
 	r, err := repo.NewRepo(fixture.validRepoPath, fetcher)

pkg/repo/testdata/repos/valid/keps/sig-architecture/123-newstyle/kep.yaml

@@ -3,7 +3,7 @@ kep-number: 1687
 authors:
   - "@rjbez17"
   - "@adrianludwin"
-owning-sig: sig-auth
+owning-sig: sig-architecture
 participating-sigs:
   - sig-auth
 reviewers:

pkg/repo/testdata/repos/valid/keps/sig-owns-only/404-question-not-found/kep.yaml

@@ -0,0 +1,22 @@
+title: Using Deep Thought
+kep-number: 13
+authors:
+  - "@alice"
+owning-sig: sig-owns-only
+participating-sigs:
+  - sig-owns-participates
+  - sig-participates-only
+status: implemented
+# creation-date: yyyy-mm-dd
+reviewers:
+  - "@beth"
+approvers:
+  - "@carolyn"
+prr-approvers:
+  - "@dorothy"
+stage: beta
+latest-milestone: v1.4
+milestone:
+  alpha: v1.1
+  beta: v1.2
+  stable: v1.3

pkg/repo/testdata/repos/valid/keps/sig-owns-only/42-the-answer/kep.yaml

@@ -0,0 +1,22 @@
+title: Life, The Universe, And Everything
+kep-number: 42
+authors:
+  - "@alice"
+owning-sig: sig-owns-only
+participating-sigs:
+  - sig-owns-participates
+  - sig-participates-only
+status: implemented
+# creation-date: yyyy-mm-dd
+reviewers:
+  - "@beth"
+approvers:
+  - "@carolyn"
+prr-approvers:
+  - "@dorothy"
+stage: alpha
+latest-milestone: v1.4
+milestone:
+  alpha: v1.1
+  beta: v1.2
+  stable: v1.3

pkg/repo/testdata/repos/valid/keps/sig-owns-participates/1-the-bare-minimum/kep.yaml

@@ -0,0 +1,21 @@
+title: The Bare Minimum
+kep-number: a non-empty string
+# authors:
+#   - "@alice"
+# TODO: this should fail to load/validate unless owning-sig is sig-owns-participates
+owning-sig: a non-empty string
+# TODO: this should fail to load/validate unless status is one of...
+status: a non-empty string
+# creation-date: yyyy-mm-dd
+# reviewers:
+#   - "@beth"
+# approvers:
+#   - "@carolyn"
+# prr-approvers:
+#   - "@dorothy"
+# stage: stable
+# latest-milestone: v1.4
+# milestone:
+#   alpha: v1.1
+#   beta: v1.2
+#   stable: v1.3