WIP: refactor labeler to allow for testing and... test

utilities/labeler/README.md

@@ -1,40 +1,102 @@
-## Label Config / General Notes
+# GitHub Labeler
 
-When a label configuration is supplied, they should be the only labels that exist unless we set auto-delete to FALSE
+A Go program that automatically labels GitHub issues and pull requests based on configurable rules defined in a `labels.yaml` file.
 
-When the label configuration is updated (name, color, description etc), it should update the label and all items the label was previously tagged with.
+## Rule Types Supported
 
-When a label that isn't defined is attempted to be applied, it should not create the label and prompt the user UNLESS we set auto-create to TRUE
+### 1. Match Rules (`kind: match`)
+Process slash commands in comments:
+```yaml
+- name: apply-triage
+  kind: match
+  spec:
+    command: "/triage"
+    matchList: ["valid", "duplicate", "needs-information", "not-planned"]
+  actions:
+  - kind: remove-label
+    spec:
+      match: needs-triage
+  - kind: apply-label
+    spec:
+      label: "triage/{{ argv.0 }}"
+```
 
-When there are multiple matching commands, it should it process all of them.
+### 2. Label Rules (`kind: label`)
+Apply labels based on existing label presence:
+```yaml
+- name: needs-triage
+  kind: label
+  spec:
+    match: "triage/*"
+    matchCondition: NOT
+  actions:
+  - kind: apply-label
+    spec:
+      label: "needs-triage"
+```
 
-When the labeler executes, it should only attempt to modify the label state IF the end state is different from the current state e.g. it should not remove and re-add a label if the end condition is the same.
+### 3. File Path Rules (`kind: filePath`)
+Apply labels based on changed file paths:
+```yaml
+- name: charter
+  kind: filePath
+  spec:
+    matchPath: "tags/*/charter.md"
+  actions:
+  - kind: apply-label
+    spec:
+      label: toc
+```
 
-A label should be able to be removed by some method e.g. /remove-<foo> <bar> would remove the label foo/bar or /foo -bar.
-No preference, just a method of removing a label needs to exist
-
-##  kind/match
-
-When the matchList rule is used, it should ONLY execute the actions if the text supplied by the user matches one of the items in the list
+## Action Types
 
-When the unique rule is used, only ONE of the defined labels should be present
+### Apply Label
+```yaml
+- kind: apply-label
+  spec:
+    label: "label-name"
+```
 
-- This can be renamed / adjusted - essentially need to restrict a set of labels to a 'namespace' and only one can be present in the final state. Maybe this should be processed as soemthing different? definine an end state condition vs matching whats there initially?
+### Remove Label
+```yaml
+- kind: remove-label
+  spec:
+    match: "label-pattern"  # Supports wildcards like "triage/*"
+```
 
+## Testing
 
-## kind/label
+Run tests:
+```bash
+go test -v
+```
 
-When the kind/label rule is used, it should ignore issue/PR bodies and check label state only for taking action.
+## Usage
 
-## kind/filePath
-
-Only applies to PRs
-
-When a commit changes anything that matches the filepath, the rules defined should execute
-
-
-## rules / actions
-
-When the remove-label action is present, it should remove the matching label if present
+### CLI
+```bash
+./labeler <labels_url> <owner> <repo> <issue_number> <comment_body> <changed_files>
+```
 
-When the apply-label action is used, it should ONLY apply a label if the label exists.
+### GitHub Actions Workflow
+The included workflow automatically runs the labeler on issue comments.
+
+## Configuration
+
+The labeler reads configuration from a `labels.yaml` file that defines:
+
+- **Label definitions** with colors and descriptions
+- **Rule sets** for automated labeling
+- **Global settings** for auto-creation/deletion
+
+## Development
+
+### Adding New Rule Types
+
+1. Add new rule processing function in `labeler.go`
+2. Update `processRule()` to handle the new rule type
+3. Add corresponding tests in test files
+
+### Mock Testing
+
+The `MockGitHubClient` provides comprehensive mocking for testing complex scenarios without hitting the GitHub API.

utilities/labeler/comprehensive_test.go

@@ -0,0 +1,206 @@
+package main
+
+import (
+	"context"
+	"testing"
+
+	"github.com/google/go-github/v55/github"
+)
+
+// Additional comprehensive tests for all rule types
+
+func TestLabeler_ProcessMatchRule_RemoveCommand(t *testing.T) {
+	client := NewMockGitHubClient()
+	config := createTestConfigWithRemoveRules()
+	labeler := NewLabeler(client, config)
+
+	// Add some existing labels
+	triageLabel := &github.Label{Name: stringPtr("triage/valid")}
+	tagLabel := &github.Label{Name: stringPtr("tag/infrastructure")}
+	client.IssueLabels[1] = []*github.Label{triageLabel, tagLabel}
+
+	req := &LabelRequest{
+		Owner:        "test-owner",
+		Repo:         "test-repo",
+		IssueNumber:  1,
+		CommentBody:  "/remove-triage valid",
+		ChangedFiles: []string{},
+	}
+
+	ctx := context.Background()
+	err := labeler.ProcessRequest(ctx, req)
+	if err != nil {
+		t.Fatalf("ProcessRequest failed: %v", err)
+	}
+
+	// Check that triage/valid was removed
+	removedLabels := client.RemovedLabels[1]
+	if !sliceContains(removedLabels, "triage/valid") {
+		t.Errorf("Expected 'triage/valid' to be removed, got: %v", removedLabels)
+	}
+}
+
+func TestLabeler_ProcessFilePathRule_MultipleFiles(t *testing.T) {
+	client := NewMockGitHubClient()
+	config := createTestConfigWithFilePathRules()
+	labeler := NewLabeler(client, config)
+
+	req := &LabelRequest{
+		Owner:        "test-owner",
+		Repo:         "test-repo",
+		IssueNumber:  1,
+		CommentBody:  "",
+		ChangedFiles: []string{
+			"tags/tag-infrastructure/charter.md",
+			"tags/tag-developer-experience/README.md",
+		},
+	}
+
+	ctx := context.Background()
+	err := labeler.ProcessRequest(ctx, req)
+	if err != nil {
+		t.Fatalf("ProcessRequest failed: %v", err)
+	}
+
+	// Check that toc label was applied (for charter.md)
+	appliedLabels := client.AppliedLabels[1]
+	if !sliceContains(appliedLabels, "toc") {
+		t.Errorf("Expected 'toc' label to be applied, got: %v", appliedLabels)
+	}
+	if !sliceContains(appliedLabels, "tag/developer-experience") {
+		t.Errorf("Expected 'tag/developer-experience' label to be applied, got: %v", appliedLabels)
+	}
+}
+
+func TestLabeler_ProcessMatchRule_WildcardRemoval(t *testing.T) {
+	client := NewMockGitHubClient()
+	config := createTestConfigWithWildcardRules()
+	labeler := NewLabeler(client, config)
+
+	// Add multiple triage labels
+	triageValid := &github.Label{Name: stringPtr("triage/valid")}
+	triageDuplicate := &github.Label{Name: stringPtr("triage/duplicate")}
+	client.IssueLabels[1] = []*github.Label{triageValid, triageDuplicate}
+
+	req := &LabelRequest{
+		Owner:        "test-owner",
+		Repo:         "test-repo",
+		IssueNumber:  1,
+		CommentBody:  "/clear-triage",
+		ChangedFiles: []string{},
+	}
+
+	ctx := context.Background()
+	err := labeler.ProcessRequest(ctx, req)
+	if err != nil {
+		t.Fatalf("ProcessRequest failed: %v", err)
+	}
+
+	// Check that all triage/* labels were removed
+	removedLabels := client.RemovedLabels[1]
+	if !sliceContains(removedLabels, "triage/valid") {
+		t.Errorf("Expected 'triage/valid' to be removed, got: %v", removedLabels)
+	}
+	if !sliceContains(removedLabels, "triage/duplicate") {
+		t.Errorf("Expected 'triage/duplicate' to be removed, got: %v", removedLabels)
+	}
+}
+
+func TestLabeler_ProcessComplexScenario(t *testing.T) {
+	client := NewMockGitHubClient()
+	config := createTestConfig()
+	labeler := NewLabeler(client, config)
+
+	// Start with needs-triage label
+	needsTriageLabel := &github.Label{Name: stringPtr("needs-triage")}
+	client.IssueLabels[1] = []*github.Label{needsTriageLabel}
+
+	req := &LabelRequest{
+		Owner:       "test-owner",
+		Repo:        "test-repo",
+		IssueNumber: 1,
+		CommentBody: `This is a complex scenario.
+/triage valid
+/tag developer-experience
+Some more comments here.`,
+		ChangedFiles: []string{"tags/tag-developer-experience/some-file.md"},
+	}
+
+	ctx := context.Background()
+	err := labeler.ProcessRequest(ctx, req)
+	if err != nil {
+		t.Fatalf("ProcessRequest failed: %v", err)
+	}
+
+	appliedLabels := client.AppliedLabels[1]
+	removedLabels := client.RemovedLabels[1]
+
+	// Should remove needs-triage
+	if !sliceContains(removedLabels, "needs-triage") {
+		t.Errorf("Expected 'needs-triage' to be removed, got: %v", removedLabels)
+	}
+
+	// Should apply triage/valid and tag/developer-experience
+	if !sliceContains(appliedLabels, "triage/valid") {
+		t.Errorf("Expected 'triage/valid' to be applied, got: %v", appliedLabels)
+	}
+	if !sliceContains(appliedLabels, "tag/developer-experience") {
+		t.Errorf("Expected 'tag/developer-experience' to be applied, got: %v", appliedLabels)
+	}
+}
+
+// Helper functions for additional test configs
+
+func createTestConfigWithRemoveRules() *LabelsYAML {
+	config := createTestConfig()
+	config.Ruleset = append(config.Ruleset, Rule{
+		Name: "remove-triage",
+		Kind: "match",
+		Spec: RuleSpec{
+			Command: "/remove-triage",
+		},
+		Actions: []Action{
+			{
+				Kind: "remove-label",
+				Spec: ActionSpec{Match: "triage/{{ argv.0 }}"},
+			},
+		},
+	})
+	return config
+}
+
+func createTestConfigWithFilePathRules() *LabelsYAML {
+	config := createTestConfig()
+	config.Ruleset = append(config.Ruleset, Rule{
+		Name: "tag-developer-experience-dir",
+		Kind: "filePath",
+		Spec: RuleSpec{
+			MatchPath: "tags/tag-developer-experience/*",
+		},
+		Actions: []Action{
+			{
+				Kind: "apply-label",
+				Spec: ActionSpec{Label: "tag/developer-experience"},
+			},
+		},
+	})
+	return config
+}
+
+func createTestConfigWithWildcardRules() *LabelsYAML {
+	config := createTestConfig()
+	config.Ruleset = append(config.Ruleset, Rule{
+		Name: "clear-triage",
+		Kind: "match",
+		Spec: RuleSpec{
+			Command: "/clear-triage",
+		},
+		Actions: []Action{
+			{
+				Kind: "remove-label",
+				Spec: ActionSpec{Match: "triage/*"},
+			},
+		},
+	})
+	return config
+}

utilities/labeler/go.mod

@@ -2,13 +2,16 @@ module labeler
 
 go 1.24.5
 
+require (
+	github.com/google/go-github/v55 v55.0.0
+	golang.org/x/oauth2 v0.30.0
+	gopkg.in/yaml.v3 v3.0.1
+)
+
 require (
 	github.com/ProtonMail/go-crypto v0.0.0-20230217124315-7d5c6f04bbb8 // indirect
 	github.com/cloudflare/circl v1.3.3 // indirect
-	github.com/google/go-github/v55 v55.0.0 // indirect
 	github.com/google/go-querystring v1.1.0 // indirect
 	golang.org/x/crypto v0.12.0 // indirect
-	golang.org/x/oauth2 v0.30.0 // indirect
 	golang.org/x/sys v0.11.0 // indirect
-	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

utilities/labeler/go.sum

@@ -5,6 +5,8 @@ github.com/cloudflare/circl v1.1.0/go.mod h1:prBCrKB9DV4poKZY1l9zBXg2QJY7mvgRvtM
 github.com/cloudflare/circl v1.3.3 h1:fE/Qz0QdIGqeWfnwq0RE0R7MI51s0M2E4Ga9kq5AEMs=
 github.com/cloudflare/circl v1.3.3/go.mod h1:5XYMA4rFBvNIrhs50XuiBJ15vF2pZn4nnUKZrLbUZFA=
 github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
+github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
+github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
 github.com/google/go-github/v55 v55.0.0 h1:4pp/1tNMB9X/LuAhs5i0KQAE40NmiR/y6prLNb9x9cg=
 github.com/google/go-github/v55 v55.0.0/go.mod h1:JLahOTA1DnXzhxEymmFF5PP2tSS9JVNj68mSZNDwskA=
 github.com/google/go-querystring v1.1.0 h1:AnCroh3fv4ZBgVIf1Iwtovgjaw/GiKJo8M8yD/fhyJ8=
@@ -24,6 +26,7 @@ golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9sn
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
+gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=