diff --git a/pkg/codeowners/reader.go b/pkg/codeowners/reader.go index b533bce..2e94319 100644 --- a/pkg/codeowners/reader.go +++ b/pkg/codeowners/reader.go @@ -134,13 +134,13 @@ func Read(path string, reviewerGroupManager ReviewerGroupManager, fileReader Fil } slices.Reverse(rules.OwnerTests) - sort.Sort(rules.OwnerTests) + sort.Stable(rules.OwnerTests) slices.Reverse(rules.AdditionalReviewerTests) - sort.Sort(rules.AdditionalReviewerTests) + sort.Stable(rules.AdditionalReviewerTests) slices.Reverse(rules.OptionalReviewerTests) - sort.Sort(rules.OptionalReviewerTests) + sort.Stable(rules.OptionalReviewerTests) return rules } diff --git a/pkg/codeowners/reader_test.go b/pkg/codeowners/reader_test.go index 5989965..9f6ed4e 100644 --- a/pkg/codeowners/reader_test.go +++ b/pkg/codeowners/reader_test.go @@ -1,10 +1,25 @@ package codeowners import ( + "fmt" "io" + "strings" "testing" ) +// inMemoryReader is an in-memory FileReader that serves a single .codeowners file. +type inMemoryReader struct { + content []byte +} + +func (r *inMemoryReader) ReadFile(string) ([]byte, error) { + return r.content, nil +} + +func (r *inMemoryReader) PathExists(string) bool { + return true +} + func TestRead(t *testing.T) { tt := []struct { name string @@ -78,3 +93,44 @@ func TestRead(t *testing.T) { }) } } + +// TestReadLastMatchWinsSamePriority verifies that within a single priority tier, +// the last-declared rule wins, per the documented "last declared wins" semantics. +// Since ownerTestRecursive returns the first match, same-tier rules must appear in +// reverse declaration order (last-declared first). In a large file the sort must be +// stable to preserve that order for equal-priority patterns — an unstable sort +// scrambles them. +func TestReadLastMatchWinsSamePriority(t *testing.T) { + rgMan := NewReviewerGroupMemo() + + const globstarRules = 200 + + var b strings.Builder + // Many globstar rules (service0/**, service1/**, ...) interleaved with rules from + // the other two priority tiers. The mix forces the sort to move elements, which + // surfaces reordering of the equal-priority globstar rules under an unstable sort. + for i := 0; i < globstarRules; i++ { + fmt.Fprintf(&b, "service%d/** @team%d\n", i, i) + fmt.Fprintf(&b, "file%d.go @file%d\n", i, i) // no-wildcard tier + fmt.Fprintf(&b, "lib%d/*.go @lib%d\n", i, i) // wildcard tier + } + + reader := &inMemoryReader{content: []byte(b.String())} + rules := Read("any/dir", rgMan, reader, io.Discard) + + // Extract the globstar rules in result order. They must be in reverse declaration + // order (highest index first): for any two same-tier rules, the later-declared one + // must precede the earlier one so that first-match-wins picks the last declaration. + prev := globstarRules + for _, test := range rules.OwnerTests { + var idx int + if n, _ := fmt.Sscanf(test.Match, "service%d/**", &idx); n != 1 { + continue + } + if idx >= prev { + t.Errorf("globstar rules out of last-declared-wins order: service%d/** appears after service%d/** (expected strictly decreasing)", idx, prev) + break + } + prev = idx + } +}