From c4579a26c0c69d827a2e4f25df0b435ea4dad77f Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Tue, 11 Aug 2026 20:13:22 -0700 Subject: [PATCH] refactor(conflict)!: key path overlap by file or by directory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? `fileoverlap` serialized two batches only when they changed the exact same file. That is the narrowest useful reading of target overlap, and for a queue whose directories are tightly coupled it is too narrow: two changes to sibling files in one package can break each other without either one touching the path the other did, and the analyzer will happily speculate them in parallel. The right granularity is not a property of the analyzer. It follows from how tightly coupled a directory's contents are in the repository behind the queue, which is something only the integrator wiring that queue up can know. So it belongs at construction, next to the resolver, rather than baked into the package. ### What? Overlap is measured on a key projected from each changed path. `PathKey` is that projection, chosen at construction and applied to every path before the two batches' sets are intersected; `Analyze` is otherwise unchanged. Two projections ship with the package. `ByFile` keys on the whole path and reproduces the previous behaviour. `ByDirectory` keys on the immediate parent, so batches touching sibling files conflict too — strictly coarser, since every file overlap is also a directory overlap. It buys protection against semantic conflicts between neighbouring files and pays for it in parallelism, which is the trade the integrator is choosing between. Paths at the repository root key on `.` under `ByDirectory`, so a batch touching `README.md` conflicts with one touching `go.mod`. Root files are usually build configuration and usually do interact, so this is deliberate rather than incidental. The package is renamed `fileoverlap` → `pathoverlap`, because the unit of overlap is now a path-derived key rather than a file. `New` takes the key as a third argument and panics on nil, mirroring `heuristic.New`. `conflict.Analyzer`, `conflict.Config` and `ConflictTypeTargetOverlap` are untouched — a folder is a coarser target, not a different kind of one. The only caller, `file-overlap-queue` in the orchestrator profiles, passes `ByFile` and keeps its behaviour and its name. One incidental behaviour change: `ByFile` runs `path.Clean`, where paths were previously compared verbatim. A provider emitting an unclean path used to produce a missed conflict. ## Test Plan ✅ `make test` — 98 pass ✅ `make lint`, `make check-gazelle`, `make check-tidy` New coverage in `pathoverlap_test.go`: - `TestPathKey` — both projections over a nested path, a repository-root file, and an unclean path. - Sibling files in one directory: no conflict under `ByFile`, conflict under `ByDirectory`; files in sibling directories conflict under neither; the same file still conflicts under both. - Two root-level files conflict under `ByDirectory` while a nested file in the same batch set does not. - `New` panics when the key is nil. --- doc/rfc/submitqueue/modular-queue-wiring.md | 4 +- .../orchestrator/server/BUILD.bazel | 2 +- .../orchestrator/server/profiles.go | 5 +- .../extension/conflict/fileoverlap/README.md | 9 -- .../conflict/fileoverlap/fileoverlap.go | 107 -------------- .../{fileoverlap => pathoverlap}/BUILD.bazel | 6 +- .../extension/conflict/pathoverlap/README.md | 18 +++ .../conflict/pathoverlap/pathoverlap.go | 138 ++++++++++++++++++ .../pathoverlap_test.go} | 105 ++++++++++++- 9 files changed, 266 insertions(+), 128 deletions(-) delete mode 100644 submitqueue/extension/conflict/fileoverlap/README.md delete mode 100644 submitqueue/extension/conflict/fileoverlap/fileoverlap.go rename submitqueue/extension/conflict/{fileoverlap => pathoverlap}/BUILD.bazel (89%) create mode 100644 submitqueue/extension/conflict/pathoverlap/README.md create mode 100644 submitqueue/extension/conflict/pathoverlap/pathoverlap.go rename submitqueue/extension/conflict/{fileoverlap/fileoverlap_test.go => pathoverlap/pathoverlap_test.go} (55%) diff --git a/doc/rfc/submitqueue/modular-queue-wiring.md b/doc/rfc/submitqueue/modular-queue-wiring.md index 0e7e828f7..c63a59479 100644 --- a/doc/rfc/submitqueue/modular-queue-wiring.md +++ b/doc/rfc/submitqueue/modular-queue-wiring.md @@ -466,7 +466,7 @@ func run(ctx context.Context) error { ChangeProvider(github.New(cfg.GitHub)). BuildRunner(local.New()). Scorer(heuristic.New()). - ConflictAnalyzer(fileoverlap.New()), + ConflictAnalyzer(pathoverlap.New()), ). Option(pipeline.TopicNames(cfg.TopicNames)). Option(pipeline.Classifiers(backendClassifiers())). @@ -497,7 +497,7 @@ app, err := submitqueue.New(). ). Queue(base.Named("monorepo/exp"). BuildRunner(local.New()). - ConflictAnalyzer(fileoverlap.New()), + ConflictAnalyzer(pathoverlap.New()), ). Queue(base.Named("monorepo/test"). BuildRunner(noop.New()). diff --git a/service/submitqueue/orchestrator/server/BUILD.bazel b/service/submitqueue/orchestrator/server/BUILD.bazel index 436453a2e..bd113c1e9 100644 --- a/service/submitqueue/orchestrator/server/BUILD.bazel +++ b/service/submitqueue/orchestrator/server/BUILD.bazel @@ -37,8 +37,8 @@ go_library( "//submitqueue/extension/conflict:go_default_library", "//submitqueue/extension/conflict/all:go_default_library", "//submitqueue/extension/conflict/fake:go_default_library", - "//submitqueue/extension/conflict/fileoverlap:go_default_library", "//submitqueue/extension/conflict/none:go_default_library", + "//submitqueue/extension/conflict/pathoverlap:go_default_library", "//submitqueue/extension/scorer:go_default_library", "//submitqueue/extension/scorer/composite:go_default_library", "//submitqueue/extension/scorer/fake:go_default_library", diff --git a/service/submitqueue/orchestrator/server/profiles.go b/service/submitqueue/orchestrator/server/profiles.go index a59d25459..02aeac713 100644 --- a/service/submitqueue/orchestrator/server/profiles.go +++ b/service/submitqueue/orchestrator/server/profiles.go @@ -27,8 +27,8 @@ import ( "github.com/uber/submitqueue/submitqueue/extension/conflict" "github.com/uber/submitqueue/submitqueue/extension/conflict/all" conflictfake "github.com/uber/submitqueue/submitqueue/extension/conflict/fake" - "github.com/uber/submitqueue/submitqueue/extension/conflict/fileoverlap" "github.com/uber/submitqueue/submitqueue/extension/conflict/none" + "github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap" "github.com/uber/submitqueue/submitqueue/extension/scorer" "github.com/uber/submitqueue/submitqueue/extension/scorer/composite" scorerfake "github.com/uber/submitqueue/submitqueue/extension/scorer/fake" @@ -256,9 +256,10 @@ func newProfiles(logger *zap.Logger, scope tally.Scope, resolver changeset.Resol // file-overlap-queue: a real analyzer that serializes only batches sharing // a changed file, resolving each batch's files itself via the resolver. + // pathoverlap.ByDirectory would coarsen this to whole directories. fileOverlapQueue := base fileOverlapQueue.Analyzer = analyzerFunc(func(c conflict.Config) (conflict.Analyzer, error) { - return fileoverlap.New(c, resolver), nil + return pathoverlap.New(c, resolver, pathoverlap.ByFile), nil }) // e2e-test-queue: composite scorer; no conflicts (maximum parallelism). diff --git a/submitqueue/extension/conflict/fileoverlap/README.md b/submitqueue/extension/conflict/fileoverlap/README.md deleted file mode 100644 index d63cf6d39..000000000 --- a/submitqueue/extension/conflict/fileoverlap/README.md +++ /dev/null @@ -1,9 +0,0 @@ -# fileoverlap - -`fileoverlap` is a `conflict.Analyzer` that reports a conflict between two batches when they change one or more of the same files. - -## Behavior - -The files a batch changes are drawn from each change's provider-supplied details. The candidate batch conflicts with an in-flight batch when their changed-file sets intersect; each such in-flight batch is reported once, preserving the in-flight order. A shared file is the concrete notion of *target overlap*, so conflicts are classified as `ConflictTypeTargetOverlap`. A batch that changes no files conflicts with nothing, and an empty in-flight list yields no conflicts. A failure to resolve a batch's changes is returned as a (retryable) error. - -File-path intersection is a deliberately simple notion of overlap. A richer one (build targets, ownership boundaries) would be a separate analyzer rather than a change to this one. diff --git a/submitqueue/extension/conflict/fileoverlap/fileoverlap.go b/submitqueue/extension/conflict/fileoverlap/fileoverlap.go deleted file mode 100644 index cc19f1fbc..000000000 --- a/submitqueue/extension/conflict/fileoverlap/fileoverlap.go +++ /dev/null @@ -1,107 +0,0 @@ -// Copyright (c) 2025 Uber Technologies, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -// Package fileoverlap provides a conflict.Analyzer that reports a conflict -// between two batches when they change one or more of the same files. It is the -// first analyzer to use the capability the extension contract unblocks: it takes -// only batch identity and resolves each batch's changed files itself through an -// injected changeset resolver, rather than depending on the controller to -// pre-compute them. A shared file is the concrete notion of target overlap, so -// it reports entity.ConflictTypeTargetOverlap. -package fileoverlap - -import ( - "context" - "fmt" - - "github.com/uber/submitqueue/submitqueue/core/changeset" - "github.com/uber/submitqueue/submitqueue/entity" - "github.com/uber/submitqueue/submitqueue/extension/conflict" -) - -// analyzer reports a conflict between batches that change a common file. The -// files a batch changes are resolved from each batch's change details. -type analyzer struct { - // cfg is the per-queue identity this analyzer was built for. - cfg conflict.Config - resolver changeset.Resolver -} - -// New returns a conflict.Analyzer that flags an in-flight batch as conflicting -// when it changes a file the candidate batch also changes, bound to the queue -// named in cfg. The resolver resolves each batch's changed files. -func New(cfg conflict.Config, resolver changeset.Resolver) conflict.Analyzer { - return analyzer{cfg: cfg, resolver: resolver} -} - -// Analyze returns one ConflictTypeTargetOverlap Conflict per in-flight batch -// that shares a changed file with batch, preserving the in-flight order. A batch -// that changes no files conflicts with nothing. -func (a analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]entity.Conflict, error) { - if len(inFlight) == 0 { - return nil, nil - } - - candidate, err := a.files(ctx, batch) - if err != nil { - return nil, fmt.Errorf("failed to resolve files for batch %s: %w", batch.ID, err) - } - if len(candidate) == 0 { - return nil, nil - } - - var conflicts []entity.Conflict - for _, other := range inFlight { - files, err := a.files(ctx, other) - if err != nil { - return nil, fmt.Errorf("failed to resolve files for batch %s: %w", other.ID, err) - } - if intersects(candidate, files) { - conflicts = append(conflicts, entity.Conflict{ - BatchID: other.ID, - Type: entity.ConflictTypeTargetOverlap, - }) - } - } - return conflicts, nil -} - -// files resolves the set of file paths the batch changes. -func (a analyzer) files(ctx context.Context, batch entity.Batch) (map[string]struct{}, error) { - changes, err := a.resolver.DetailedForBatch(ctx, batch) - if err != nil { - return nil, err - } - files := make(map[string]struct{}) - for _, change := range changes.Changes { - for _, file := range change.Details.ChangedFiles { - files[file.Path] = struct{}{} - } - } - return files, nil -} - -// intersects reports whether the two sets share any element. -func intersects(a, b map[string]struct{}) bool { - // Iterate the smaller set for fewer lookups. - if len(b) < len(a) { - a, b = b, a - } - for k := range a { - if _, ok := b[k]; ok { - return true - } - } - return false -} diff --git a/submitqueue/extension/conflict/fileoverlap/BUILD.bazel b/submitqueue/extension/conflict/pathoverlap/BUILD.bazel similarity index 89% rename from submitqueue/extension/conflict/fileoverlap/BUILD.bazel rename to submitqueue/extension/conflict/pathoverlap/BUILD.bazel index a32b277a5..be667b4cb 100644 --- a/submitqueue/extension/conflict/fileoverlap/BUILD.bazel +++ b/submitqueue/extension/conflict/pathoverlap/BUILD.bazel @@ -2,8 +2,8 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", - srcs = ["fileoverlap.go"], - importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/fileoverlap", + srcs = ["pathoverlap.go"], + importpath = "github.com/uber/submitqueue/submitqueue/extension/conflict/pathoverlap", visibility = ["//visibility:public"], deps = [ "//submitqueue/core/changeset:go_default_library", @@ -14,7 +14,7 @@ go_library( go_test( name = "go_default_test", - srcs = ["fileoverlap_test.go"], + srcs = ["pathoverlap_test.go"], embed = [":go_default_library"], deps = [ "//submitqueue/core/changeset/fake:go_default_library", diff --git a/submitqueue/extension/conflict/pathoverlap/README.md b/submitqueue/extension/conflict/pathoverlap/README.md new file mode 100644 index 000000000..6f9086efa --- /dev/null +++ b/submitqueue/extension/conflict/pathoverlap/README.md @@ -0,0 +1,18 @@ +# pathoverlap + +`pathoverlap` is a `conflict.Analyzer` that reports a conflict between two batches when the paths they change share a key. + +## Behavior + +The files a batch changes are drawn from each change's provider-supplied details, and each path is projected onto a key by the `PathKey` chosen at construction. The candidate batch conflicts with an in-flight batch when their key sets intersect; each such in-flight batch is reported once, preserving the in-flight order. A shared path key is the concrete notion of *target overlap*, so conflicts are classified as `ConflictTypeTargetOverlap`. A batch that changes no files conflicts with nothing, and an empty in-flight list yields no conflicts. A failure to resolve a batch's changes is returned as a (retryable) error. + +## Granularity + +Two projections ship with the package, selected per queue in the wiring layer: + +- **`ByFile`** keys on the whole path, so only batches touching the same file conflict. +- **`ByDirectory`** keys on the path's immediate parent directory, so batches touching sibling files conflict too. Paths at the repository root share the key `.`, which serializes batches that touch any two root-level files. + +`ByDirectory` is strictly coarser: every file overlap is also a directory overlap. It trades parallelism for protection against semantic conflicts between neighbouring files — edits that break each other without touching the same file. Which trade is right is a per-queue judgement about how tightly coupled a directory's contents are, so the choice is a construction parameter rather than a property of the analyzer. + +Path-key intersection is a deliberately simple notion of overlap. A richer one that needs inputs beyond the changed paths — build targets, ownership boundaries — would be a separate analyzer rather than another `PathKey`. diff --git a/submitqueue/extension/conflict/pathoverlap/pathoverlap.go b/submitqueue/extension/conflict/pathoverlap/pathoverlap.go new file mode 100644 index 000000000..a206854e3 --- /dev/null +++ b/submitqueue/extension/conflict/pathoverlap/pathoverlap.go @@ -0,0 +1,138 @@ +// Copyright (c) 2025 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package pathoverlap provides a conflict.Analyzer that reports a conflict +// between two batches when the paths they change share a key. The key is a +// projection of the path chosen at construction: ByFile compares whole paths, +// so only batches touching the same file conflict; ByDirectory compares parent +// directories, so batches touching sibling files conflict too. It is the first +// analyzer to use the capability the extension contract unblocks: it takes only +// batch identity and resolves each batch's changed files itself through an +// injected changeset resolver, rather than depending on the controller to +// pre-compute them. A shared path key is the concrete notion of target overlap, +// so it reports entity.ConflictTypeTargetOverlap. +package pathoverlap + +import ( + "context" + "fmt" + "path" + + "github.com/uber/submitqueue/submitqueue/core/changeset" + "github.com/uber/submitqueue/submitqueue/entity" + "github.com/uber/submitqueue/submitqueue/extension/conflict" +) + +// PathKey projects a changed file path onto the key overlap is measured on. Two +// batches conflict when any of their changed paths yield the same key, so a +// coarser projection serializes more batches. +type PathKey func(path string) string + +// ByFile keys on the whole path: batches conflict only when they change the +// same file. +func ByFile(p string) string { + return path.Clean(p) +} + +// ByDirectory keys on the path's immediate parent directory: batches conflict +// when they change any files in the same directory, whether or not the files +// themselves are the same. Overlap by directory is strictly coarser than +// overlap by file — every file overlap is also a directory overlap. Paths at +// the repository root share the key ".". +func ByDirectory(p string) string { + return path.Dir(p) +} + +// analyzer reports a conflict between batches whose changed paths share a key. +// The paths a batch changes are resolved from each batch's change details. +type analyzer struct { + // cfg is the per-queue identity this analyzer was built for. + cfg conflict.Config + resolver changeset.Resolver + // key projects each changed path onto the key overlap is measured on. + key PathKey +} + +// New returns a conflict.Analyzer that flags an in-flight batch as conflicting +// when it changes a path whose key matches one the candidate batch changes, +// bound to the queue named in cfg. The resolver resolves each batch's changed +// files, and key selects the granularity of overlap. +// Panics if key is nil. +func New(cfg conflict.Config, resolver changeset.Resolver, key PathKey) conflict.Analyzer { + if key == nil { + panic("pathoverlap.New: key must not be nil") + } + return analyzer{cfg: cfg, resolver: resolver, key: key} +} + +// Analyze returns one ConflictTypeTargetOverlap Conflict per in-flight batch +// that shares a path key with batch, preserving the in-flight order. A batch +// that changes no files conflicts with nothing. +func (a analyzer) Analyze(ctx context.Context, batch entity.Batch, inFlight []entity.Batch) ([]entity.Conflict, error) { + if len(inFlight) == 0 { + return nil, nil + } + + candidate, err := a.keys(ctx, batch) + if err != nil { + return nil, fmt.Errorf("failed to resolve files for batch %s: %w", batch.ID, err) + } + if len(candidate) == 0 { + return nil, nil + } + + var conflicts []entity.Conflict + for _, other := range inFlight { + keys, err := a.keys(ctx, other) + if err != nil { + return nil, fmt.Errorf("failed to resolve files for batch %s: %w", other.ID, err) + } + if intersects(candidate, keys) { + conflicts = append(conflicts, entity.Conflict{ + BatchID: other.ID, + Type: entity.ConflictTypeTargetOverlap, + }) + } + } + return conflicts, nil +} + +// keys resolves the set of path keys the batch changes. +func (a analyzer) keys(ctx context.Context, batch entity.Batch) (map[string]struct{}, error) { + changes, err := a.resolver.DetailedForBatch(ctx, batch) + if err != nil { + return nil, err + } + keys := make(map[string]struct{}) + for _, change := range changes.Changes { + for _, file := range change.Details.ChangedFiles { + keys[a.key(file.Path)] = struct{}{} + } + } + return keys, nil +} + +// intersects reports whether the two sets share any element. +func intersects(a, b map[string]struct{}) bool { + // Iterate the smaller set for fewer lookups. + if len(b) < len(a) { + a, b = b, a + } + for k := range a { + if _, ok := b[k]; ok { + return true + } + } + return false +} diff --git a/submitqueue/extension/conflict/fileoverlap/fileoverlap_test.go b/submitqueue/extension/conflict/pathoverlap/pathoverlap_test.go similarity index 55% rename from submitqueue/extension/conflict/fileoverlap/fileoverlap_test.go rename to submitqueue/extension/conflict/pathoverlap/pathoverlap_test.go index 6363ac74d..28cb9d0a4 100644 --- a/submitqueue/extension/conflict/fileoverlap/fileoverlap_test.go +++ b/submitqueue/extension/conflict/pathoverlap/pathoverlap_test.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package fileoverlap +package pathoverlap import ( "context" @@ -39,9 +39,45 @@ func detailed(batchID string, files ...string) entity.BatchChanges { } } +func TestPathKey(t *testing.T) { + tests := []struct { + name string + path string + wantFile string + wantDirectory string + }{ + { + name: "nested path", + path: "src/pkg/a.go", + wantFile: "src/pkg/a.go", + wantDirectory: "src/pkg", + }, + { + name: "repository root file keys on the root directory", + path: "README.md", + wantFile: "README.md", + wantDirectory: ".", + }, + { + name: "unclean path is normalized", + path: "./src/pkg/../pkg/a.go", + wantFile: "src/pkg/a.go", + wantDirectory: "src/pkg", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.wantFile, ByFile(tt.path)) + assert.Equal(t, tt.wantDirectory, ByDirectory(tt.path)) + }) + } +} + func TestAnalyze(t *testing.T) { tests := []struct { name string + key PathKey candidate entity.BatchChanges inFlight map[string]entity.BatchChanges inFlightIDs []string @@ -49,6 +85,7 @@ func TestAnalyze(t *testing.T) { }{ { name: "overlap on a shared file conflicts", + key: ByFile, candidate: detailed("cand", "a.go", "b.go"), inFlight: map[string]entity.BatchChanges{ "x": detailed("x", "b.go", "c.go"), @@ -58,6 +95,7 @@ func TestAnalyze(t *testing.T) { }, { name: "disjoint files do not conflict", + key: ByFile, candidate: detailed("cand", "a.go"), inFlight: map[string]entity.BatchChanges{ "x": detailed("x", "z.go"), @@ -67,6 +105,7 @@ func TestAnalyze(t *testing.T) { }, { name: "only overlapping in-flight batches are reported, in order", + key: ByFile, candidate: detailed("cand", "a.go"), inFlight: map[string]entity.BatchChanges{ "x": detailed("x", "a.go"), @@ -78,6 +117,7 @@ func TestAnalyze(t *testing.T) { }, { name: "candidate with no targets conflicts with nothing", + key: ByFile, candidate: detailed("cand"), inFlight: map[string]entity.BatchChanges{ "x": detailed("x", "a.go"), @@ -85,6 +125,57 @@ func TestAnalyze(t *testing.T) { inFlightIDs: []string{"x"}, wantBatches: nil, }, + { + name: "sibling files in one directory do not conflict by file", + key: ByFile, + candidate: detailed("cand", "src/pkg/a.go"), + inFlight: map[string]entity.BatchChanges{ + "x": detailed("x", "src/pkg/b.go"), + }, + inFlightIDs: []string{"x"}, + wantBatches: nil, + }, + { + name: "sibling files in one directory conflict by directory", + key: ByDirectory, + candidate: detailed("cand", "src/pkg/a.go"), + inFlight: map[string]entity.BatchChanges{ + "x": detailed("x", "src/pkg/b.go"), + }, + inFlightIDs: []string{"x"}, + wantBatches: []string{"x"}, + }, + { + name: "files in sibling directories do not conflict by directory", + key: ByDirectory, + candidate: detailed("cand", "src/pkg/a.go"), + inFlight: map[string]entity.BatchChanges{ + "x": detailed("x", "src/other/a.go"), + }, + inFlightIDs: []string{"x"}, + wantBatches: nil, + }, + { + name: "the same file still conflicts by directory", + key: ByDirectory, + candidate: detailed("cand", "src/pkg/a.go"), + inFlight: map[string]entity.BatchChanges{ + "x": detailed("x", "src/pkg/a.go"), + }, + inFlightIDs: []string{"x"}, + wantBatches: []string{"x"}, + }, + { + name: "repository root files conflict with each other by directory", + key: ByDirectory, + candidate: detailed("cand", "README.md"), + inFlight: map[string]entity.BatchChanges{ + "x": detailed("x", "go.mod"), + "y": detailed("y", "src/pkg/a.go"), + }, + inFlightIDs: []string{"x", "y"}, + wantBatches: []string{"x"}, + }, } for _, tt := range tests { @@ -96,7 +187,7 @@ func TestAnalyze(t *testing.T) { inFlight = append(inFlight, entity.Batch{ID: id}) } - got, err := New(conflict.Config{QueueName: "test-queue"}, resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, inFlight) + got, err := New(conflict.Config{QueueName: "test-queue"}, resolver, tt.key).Analyze(context.Background(), entity.Batch{ID: "cand"}, inFlight) require.NoError(t, err) var ids []string @@ -110,7 +201,7 @@ func TestAnalyze(t *testing.T) { } func TestAnalyze_EmptyInFlight(t *testing.T) { - got, err := New(conflict.Config{QueueName: "test-queue"}, changesetfake.New()).Analyze(context.Background(), entity.Batch{ID: "cand"}, nil) + got, err := New(conflict.Config{QueueName: "test-queue"}, changesetfake.New(), ByFile).Analyze(context.Background(), entity.Batch{ID: "cand"}, nil) require.NoError(t, err) assert.Empty(t, got) } @@ -119,6 +210,12 @@ func TestAnalyze_ResolverError(t *testing.T) { sentinel := errors.New("resolve failed") resolver := changesetfake.New().FailWith(sentinel) - _, err := New(conflict.Config{QueueName: "test-queue"}, resolver).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) + _, err := New(conflict.Config{QueueName: "test-queue"}, resolver, ByFile).Analyze(context.Background(), entity.Batch{ID: "cand"}, []entity.Batch{{ID: "x"}}) require.ErrorIs(t, err, sentinel) } + +func TestNew_NilKeyPanics(t *testing.T) { + assert.Panics(t, func() { + New(conflict.Config{QueueName: "test-queue"}, changesetfake.New(), nil) + }) +}