From 6f2e25b3820c2b3cc542e4a52a40c785da615580 Mon Sep 17 00:00:00 2001 From: Ian Kerins Date: Tue, 7 Nov 2023 22:59:55 -0500 Subject: [PATCH] Expand Repository.MergeMutable to cover more fields There was no test coverage, so I added some. Closes #683. --- api.go | 19 +++++++- api_test.go | 131 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+), 2 deletions(-) diff --git a/api.go b/api.go index 1641fed1a..7fd6a318d 100644 --- a/api.go +++ b/api.go @@ -656,8 +656,6 @@ func (r *Repository) UnmarshalJSON(data []byte) error { // // Note: We ignore RawConfig fields which are duplicated into Repository: // name and id. -// -// Note: URL, *Template fields are ignored. They are not used by Sourcegraph. func (r *Repository) MergeMutable(x *Repository) (mutated bool, err error) { if r.ID != x.ID { // Sourcegraph: strange behaviour may occur if ID changes but names don't. @@ -688,6 +686,23 @@ func (r *Repository) MergeMutable(x *Repository) (mutated bool, err error) { } } + if r.URL != x.URL { + mutated = true + r.URL = x.URL + } + if r.CommitURLTemplate != x.CommitURLTemplate { + mutated = true + r.CommitURLTemplate = x.CommitURLTemplate + } + if r.FileURLTemplate != x.FileURLTemplate { + mutated = true + r.FileURLTemplate = x.FileURLTemplate + } + if r.LineFragmentTemplate != x.LineFragmentTemplate { + mutated = true + r.LineFragmentTemplate = x.LineFragmentTemplate + } + return mutated, nil } diff --git a/api_test.go b/api_test.go index c7492730d..87ad41671 100644 --- a/api_test.go +++ b/api_test.go @@ -237,3 +237,134 @@ func TestSearchOptions_String(t *testing.T) { } } } + +func TestRepositoryMergeMutable(t *testing.T) { + a := Repository{ + ID: 0, + Name: "name", + Branches: []RepositoryBranch{ + { + Name: "branchName", + Version: "branchVersion", + }, + }, + RawConfig: nil, + URL: "url", + CommitURLTemplate: "commitUrlTemplate", + FileURLTemplate: "fileUrlTemplate", + LineFragmentTemplate: "lineFragmentTemplate", + } + + t.Run("different ID", func(t *testing.T) { + b := a + b.ID = 1 + mutated, err := a.MergeMutable(&b) + if err == nil { + t.Fatalf("want err, got mutated=%t", mutated) + } + }) + t.Run("different Name", func(t *testing.T) { + b := a + b.Name = "otherName" + mutated, err := a.MergeMutable(&b) + if err == nil { + t.Fatalf("want err, got mutated=%t", mutated) + } + }) + t.Run("different Branches", func(t *testing.T) { + b := a + b.Branches = []RepositoryBranch{ + { + Name: "otherBranchName", + Version: "branchVersion", + }, + } + mutated, err := a.MergeMutable(&b) + if err == nil { + t.Fatalf("want err, got mutated=%t", mutated) + } + }) + t.Run("different RawConfig", func(t *testing.T) { + b := a + b.RawConfig = map[string]string{"foo": "bar"} + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if !mutated { + t.Fatalf("want mutated=true, got false") + } + if !reflect.DeepEqual(a.RawConfig, b.RawConfig) { + t.Fatalf("got different RawConfig, %v vs %v", a.RawConfig, b.RawConfig) + } + }) + t.Run("different URL", func(t *testing.T) { + b := a + b.URL = "otherURL" + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if !mutated { + t.Fatalf("want mutated=true, got false") + } + if a.URL != b.URL { + t.Fatalf("got different URL, %s vs %s", a.URL, b.URL) + } + }) + t.Run("different CommitURLTemplate", func(t *testing.T) { + b := a + b.CommitURLTemplate = "otherCommitUrlTemplate" + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if !mutated { + t.Fatalf("want mutated=true, got false") + } + if a.CommitURLTemplate != b.CommitURLTemplate { + t.Fatalf("got different CommitURLTemplate, %s vs %s", a.CommitURLTemplate, b.CommitURLTemplate) + } + }) + t.Run("different FileURLTemplate", func(t *testing.T) { + b := a + b.FileURLTemplate = "otherFileUrlTemplate" + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if !mutated { + t.Fatalf("want mutated=true, got false") + } + if a.FileURLTemplate != b.FileURLTemplate { + t.Fatalf("got different FileURLTemplate, %s vs %s", a.FileURLTemplate, b.FileURLTemplate) + } + }) + t.Run("different LineFragmentTemplate", func(t *testing.T) { + b := a + b.LineFragmentTemplate = "otherLineFragmentTemplate" + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if !mutated { + t.Fatalf("want mutated=true, got false") + } + if a.LineFragmentTemplate != b.LineFragmentTemplate { + t.Fatalf("got different LineFragmentTemplate, %s vs %s", a.LineFragmentTemplate, b.LineFragmentTemplate) + } + }) + t.Run("all same", func(t *testing.T) { + b := a + mutated, err := a.MergeMutable(&b) + if err != nil { + t.Fatalf("got err %v", err) + } + if mutated { + t.Fatalf("want mutated=false, got true") + } + if !reflect.DeepEqual(a, b) { + t.Fatalf("got different Repository, %v vs %v", a, b) + } + }) +}