Skip to content

Commit

Permalink
Fix broken behaviour and add some tests
Browse files Browse the repository at this point in the history 
The first iteration of this change produced a scenario where we'd drop
packages that have a different version which we previously were
retaining.  This change fixes that and includes differently versioned
packages while accounting for the repository priority otherwise.
  • Loading branch information
@kellyma2
kellyma2 committed Jan 17, 2025
1 parent 891c572 commit f00f2bd
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 31 deletions.
11 changes: 11 additions & 0 deletions pkg/reducer/helper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,19 @@ package reducer

import (
"github.com/rmohr/bazeldnf/pkg/api"
"github.com/rmohr/bazeldnf/pkg/api/bazeldnf"
)

func withRepository(packages []api.Package) []api.Package{
r := []api.Package{}
for _, p := range packages {
p.Repository = &bazeldnf.Repository{}
r = append(r, p)
}

return r
}

func newPackageList(names ...string) []api.Package {
r := []api.Package{}
for _, name := range names {
Expand Down
12 changes: 7 additions & 5 deletions pkg/reducer/reducer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ func (r *RepoReducer) Resolve(packages []string) (matched []string, involved []*
return nil, nil, fmt.Errorf("Package %s does not exist", req)
}

if len(candidates) > 0 {
selected := candidates[0]
for _, p := range candidates {
for i, p := range candidates {
if selected, ok := discovered[p.String()]; !ok {
discovered[p.String()] = candidates[i]
} else {
if selected.Repository.Priority > p.Repository.Priority {
selected = p
discovered[p.String()] = candidates[i]
}
}
}

discovered[selected.String()] = selected
if len(candidates) > 0 {
matched = append(matched, candidates[0].Name)
}
}
Expand Down
89 changes: 63 additions & 26 deletions pkg/reducer/reducer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

. "github.com/onsi/gomega"
"github.com/rmohr/bazeldnf/pkg/api"
"github.com/rmohr/bazeldnf/pkg/api/bazeldnf"
)

type MockPackageLoader struct {
Expand Down Expand Up @@ -59,7 +60,7 @@ func TestReducerOnlyImplicitRequires(t *testing.T) {
g := NewGomegaWithT(t)

packageInfo := packageInfo{
packages: newPackageList("foo"),
packages: withRepository(newPackageList("foo")),
}

matched, involved, err := resolve(&packageInfo, []string{}, []string{"foo"})
Expand All @@ -71,7 +72,7 @@ func TestReducerOnlyImplicitRequires(t *testing.T) {

func TestReducerSingleCandidate(t *testing.T) {
g := NewGomegaWithT(t)
packages := newPackageList("bar")
packages := withRepository(newPackageList("bar"))
packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"bar"}, []string{})
Expand All @@ -84,7 +85,7 @@ func TestReducerSingleCandidate(t *testing.T) {
func TestReducerMultipleCandidates(t *testing.T) {
g := NewGomegaWithT(t)
packageNames := []string{"foo", "bar", "baz"}
packages := newPackageList(packageNames...)
packages := withRepository(newPackageList(packageNames...))
packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, packageNames, []string{})
Expand All @@ -96,7 +97,7 @@ func TestReducerMultipleCandidates(t *testing.T) {

func TestReducerMultipleNameMatch(t *testing.T) {
g := NewGomegaWithT(t)
packages := newPackageList("foo", "foo", "bar")
packages := withRepository(newPackageList("foo", "foo", "bar"))
packages[0].Version = api.Version{Epoch: "1"}
packageInfo := packageInfo{packages: packages}

Expand All @@ -108,9 +109,9 @@ func TestReducerMultipleNameMatch(t *testing.T) {

func TestReducerRequiresMissingProvides(t *testing.T) {
g := NewGomegaWithT(t)
packages := []api.Package{
packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"bar"}, nil),
}
})
packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"foo"}, []string{})
Expand All @@ -121,10 +122,10 @@ func TestReducerRequiresMissingProvides(t *testing.T) {

func TestReducerRequiresFoundProvides(t *testing.T) {
g := NewGomegaWithT(t)
packages := []api.Package{
packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"bar"}, nil),
newPackage("bar"),
}
})
packageInfo := packageInfo{
packages: packages,
provides: map[string][]*api.Package{
Expand All @@ -140,12 +141,12 @@ func TestReducerRequiresFoundProvides(t *testing.T) {

func TestReducerRequiresFoundMultipleProvides(t *testing.T) {
g := NewGomegaWithT(t)
packages := []api.Package{
packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"baz", "bam"}, nil),
newPackage("baz"),
newPackage("bam"),
}
})

packageInfo := packageInfo{
packages: packages,
provides: map[string][]*api.Package{
Expand All @@ -162,11 +163,11 @@ func TestReducerRequiresFoundMultipleProvides(t *testing.T) {

func TestReducerRequiresFoundMultipleProvidesInOne(t *testing.T) {
g := NewGomegaWithT(t)
packages := []api.Package{
packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"baz", "bam"}, nil),
newPackage("baz"),
newPackage("bam"),
}
})

packageInfo := packageInfo{
packages: packages,
Expand All @@ -183,11 +184,11 @@ func TestReducerRequiresFoundMultipleProvidesInOne(t *testing.T) {

func TestReducerMultiLevelRequires(t *testing.T) {
g := NewGomegaWithT(t)
packages := []api.Package{
packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"baz"}, nil),
newPackageWithDeps("baz", []string{"bam"}, nil),
newPackage("bam"),
}
})

packageInfo := packageInfo{
packages: packages,
Expand All @@ -206,13 +207,13 @@ func TestReducerMultiLevelRequires(t *testing.T) {
func TestReducerExcludePinnedDependency(t *testing.T) {
g := NewGomegaWithT(t)
pinned := newPackage("bar")
pinned.Version = api.Version{Epoch:"1"}
packages := []api.Package{
pinned.Version = api.Version{Epoch: "1"}

packages := withRepository([]api.Package{
newPackageWithDeps("foo", []string{"bar"}, nil),
newPackage("bar"),
}
})

packageInfo := packageInfo{
packages: packages,
provides: map[string][]*api.Package{
Expand All @@ -227,11 +228,12 @@ func TestReducerExcludePinnedDependency(t *testing.T) {
}

func TestInvolvedProvidesIsNotRequiredOrSelf(t *testing.T) {
g:= NewGomegaWithT(t)
packages := []api.Package{
g := NewGomegaWithT(t)
packages := withRepository([]api.Package{
newPackageWithDeps("foo", nil, []string{"bar"}),
}
})
expectedPackage := newPackageWithDeps("foo", nil, []string{})
expectedPackage.Repository = &bazeldnf.Repository{}
packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"foo"}, []string{})
Expand All @@ -241,15 +243,50 @@ func TestInvolvedProvidesIsNotRequiredOrSelf(t *testing.T) {
}

func TestInvolvedProvidesIsSelf(t *testing.T) {
g:= NewGomegaWithT(t)
packages := []api.Package{
g := NewGomegaWithT(t)
packages := withRepository([]api.Package{
newPackageWithDeps("foo", nil, []string{"foo"}),
}
})
expectedPackage := newPackageWithDeps("foo", nil, []string{"foo"})
expectedPackage.Repository = &bazeldnf.Repository{}

packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"foo"}, []string{})
g.Expect(err).Should(BeNil())
g.Expect(matched).Should(ConsistOf("foo"))
g.Expect(involved).Should(ConsistOf(&expectedPackage))
}

func TestRepositoryPriority(t *testing.T) {
g := NewGomegaWithT(t)
packages := withRepository(newPackageList("bar", "bar"))
packages[0].Repository.Priority = 2
packages[1].Repository.Priority = 1
packages[1].Summary = "I'm the one"

packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"bar"}, []string{})

g.Expect(err).Should(BeNil())
g.Expect(matched).Should(ConsistOf("bar"))
g.Expect(involved).Should(ConsistOf(&packages[1]))
}

func TestRepositoryPriorityWithVersion(t *testing.T) {
g := NewGomegaWithT(t)
packages := withRepository(newPackageList("bar", "bar", "bar"))
packages[0].Repository.Priority = 2
packages[1].Repository.Priority = 1
packages[1].Summary = "I'm the one"
packages[2].Version = api.Version{Epoch: "3"}

packageInfo := packageInfo{packages: packages}

matched, involved, err := resolve(&packageInfo, []string{"bar"}, []string{})

g.Expect(err).Should(BeNil())
g.Expect(matched).Should(ConsistOf("bar"))
g.Expect(involved).Should(ConsistOf(&packages[1], &packages[2]))
}

0 comments on commit f00f2bd

Please sign in to comment.