Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release-4.6] Bug 1891064: Check prerelease version for semver-skippatch mode #488

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/registry/bundlegraphloader.go
Expand Up @@ -153,6 +153,9 @@ func (g *BundleGraphLoader) AddBundleToGraph(bundle *Bundle, graph *Package, ann
return graph, nil
}

// isSkipPatchCandidate returns true if version is equal to toCompare
// in major and minor positions and strictly greater in all others,
// indicating that toCompare can be skipped over to version.
func isSkipPatchCandidate(version, toCompare semver.Version) bool {
return (version.Major == toCompare.Major) && (version.Minor == toCompare.Minor) && (version.Patch > toCompare.Patch)
return (version.Major == toCompare.Major) && (version.Minor == toCompare.Minor) && version.GT(toCompare)
}
71 changes: 71 additions & 0 deletions pkg/registry/bundlegraphloader_test.go
Expand Up @@ -2,6 +2,7 @@ package registry

import (
"encoding/json"
"github.com/blang/semver"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: import order

remember, even with goimports, we still need to organize the imports into the correct groups; e.g.

import (
    <standard>
    
    <external orgs>

    <same org>
)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice if go fmt did this for free

"testing"

"github.com/stretchr/testify/assert"
Expand Down Expand Up @@ -312,3 +313,73 @@ func TestBundleGraphLoader(t *testing.T) {
})
}
}

func TestIsSkipPatchCandidate(t *testing.T) {
tests := []struct{
name string
added string
compare string
expected bool
commutative bool
}{
{
name: "equal versions",
added: "0.0.0",
compare: "0.0.0",
expected: false,
commutative: true,
},
{
name: "do not accept different major/minor version",
added: "0.1.0",
compare: "0.2.0",
expected: false,
commutative: true,
},
{
name: "accept larger patch version",
added: "0.0.1",
compare: "0.0.0",
expected: true,
},
{
name: "accept patch version without pre-release",
added: "0.0.0",
compare: "0.0.0-1",
expected: true,
},
{
name: "accept longer pre-release with same prefix",
added: "0.0.1-1.2",
compare: "0.0.1-1",
expected: true,
},
{
name: "accept numerically larger pre-release",
added: "0.0.1-11",
compare: "0.0.1-2",
expected: true,
},
{
name: "accept lexicographically larger pre-release",
added: "0.0.1-beta.1",
compare: "0.0.1-alpha.1",
expected: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
added, err := semver.Make(tt.added)
require.NoError(t, err)
compare, err := semver.Make(tt.compare)
require.NoError(t, err)
actual := isSkipPatchCandidate(added, compare)
assert.Equal(t, tt.expected, actual)

if !tt.commutative {
reverse := isSkipPatchCandidate(compare, added)
assert.Equal(t, !tt.expected, reverse)
}
})
}
}