From c35a2edf05629eab646b8b03654d0b44faa0281b Mon Sep 17 00:00:00 2001 From: Tim Heckman Date: Fri, 28 Jul 2017 04:59:02 -0700 Subject: [PATCH] Remove redundant version check and unit test case This change introduces some small fixes for the `util/version` package of `keel`. There should be no breaking changes introduced by the changes being proposed. The first change is around the version check when the policy is set to PolicyTypePatch. In this path, the major and minor versions are confirmed to be the same. It then checks to make sure the new patch is greater than the current one, which we already do here: * https://github.com/theckman/keel/blob/b3ba3a47f1c9e0f4fc93571645286aacf00b4d39/util/version/version.go#L129-L132 This change removes the check confirming that the patch is higher on the new version. An additional test was added to test for a patch decrease, with PolicyTypePatch, to make sure nothing was broken. Lastly, in the process of writing this test a duplicated test case was discovered in the test file so it was removed. Signed-off-by: Tim Heckman --- util/version/version.go | 2 +- util/version/version_test.go | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/util/version/version.go b/util/version/version.go index c60cdbfd..75d6b396 100644 --- a/util/version/version.go +++ b/util/version/version.go @@ -137,7 +137,7 @@ func ShouldUpdate(current *types.Version, new *types.Version, policy types.Polic case types.PolicyTypeMinor: return newVersion.Major() == currentVersion.Major(), nil case types.PolicyTypePatch: - return newVersion.Major() == currentVersion.Major() && newVersion.Minor() == currentVersion.Minor() && newVersion.Patch() > currentVersion.Patch(), nil + return newVersion.Major() == currentVersion.Major() && newVersion.Minor() == currentVersion.Minor(), nil } return false, nil } diff --git a/util/version/version_test.go b/util/version/version_test.go index f3c7d680..8ec1a1b5 100644 --- a/util/version/version_test.go +++ b/util/version/version_test.go @@ -132,6 +132,16 @@ func TestShouldUpdate(t *testing.T) { want: true, wantErr: false, }, + { + name: "patch decrease, policy patch", + args: args{ + current: &types.Version{Major: 1, Minor: 4, Patch: 5}, + new: &types.Version{Major: 1, Minor: 4, Patch: 4}, + policy: types.PolicyTypePatch, + }, + want: false, + wantErr: false, + }, { name: "patch AND major increase, policy patch", args: args{ @@ -172,16 +182,6 @@ func TestShouldUpdate(t *testing.T) { want: true, wantErr: false, }, - { - name: "patch increase, policy ptach", - args: args{ - current: &types.Version{Major: 1, Minor: 4, Patch: 5}, - new: &types.Version{Major: 1, Minor: 4, Patch: 6}, - policy: types.PolicyTypePatch, - }, - want: true, - wantErr: false, - }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {