Skip to content

Commit e5568fe

Browse files
mandremandre
authored
Acceptance: Fix IsCurrentBelow and IsCurrentAbove functions
Take into account numerical release names, after zed, and ensure that comparing the same version always yields false.
1 parent 2aab79d commit e5568fe

2 files changed

Lines changed: 115 additions & 23 deletions

File tree

‎acceptance/clients/conditions.go‎

Lines changed: 50 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package clients
22

33
import (
44
"os"
5+
"strconv"
6+
"strings"
57
"testing"
68
)
79

@@ -90,66 +92,91 @@ func RequireIronicHTTPBasic(t *testing.T) {
9092
}
9193

9294
func getReleaseFromEnv(t *testing.T) string {
93-
current_branch := os.Getenv("OS_BRANCH")
94-
if current_branch == "" {
95+
current := strings.TrimPrefix(os.Getenv("OS_BRANCH"), "stable/")
96+
if current == "" {
9597
t.Fatal("this test requires OS_BRANCH to be set but it wasn't")
9698
}
97-
return current_branch
99+
return current
98100
}
99101

100102
// SkipRelease will have the test be skipped on a certain
101103
// release. Releases are named such as 'stable/mitaka', master, etc.
102104
func SkipRelease(t *testing.T, release string) {
103-
current_branch := getReleaseFromEnv(t)
104-
if current_branch == release {
105+
current := getReleaseFromEnv(t)
106+
if current == release {
105107
t.Skipf("this is not supported in %s", release)
106108
}
107109
}
108110

109111
// SkipReleasesBelow will have the test be skipped on releases below a certain
110112
// one. Releases are named such as 'stable/mitaka', master, etc.
111113
func SkipReleasesBelow(t *testing.T, release string) {
112-
current_branch := getReleaseFromEnv(t)
114+
current := getReleaseFromEnv(t)
113115

114116
if IsCurrentBelow(t, release) {
115-
t.Skipf("this is not supported below %s, testing in %s", release, current_branch)
117+
t.Skipf("this is not supported below %s, testing in %s", release, current)
116118
}
117119
}
118120

119121
// SkipReleasesAbove will have the test be skipped on releases above a certain
120122
// one. The test is always skipped on master release. Releases are named such
121123
// as 'stable/mitaka', master, etc.
122124
func SkipReleasesAbove(t *testing.T, release string) {
123-
current_branch := getReleaseFromEnv(t)
125+
current := getReleaseFromEnv(t)
124126

125-
// Assume master is always too new
126127
if IsCurrentAbove(t, release) {
127-
t.Skipf("this is not supported above %s, testing in %s", release, current_branch)
128+
t.Skipf("this is not supported above %s, testing in %s", release, current)
128129
}
129130
}
130131

132+
func isReleaseNumeral(release string) bool {
133+
_, err := strconv.Atoi(release[0:1])
134+
return err == nil
135+
}
136+
131137
// IsCurrentAbove will return true on releases above a certain
132138
// one. The result is always true on master release. Releases are named such
133139
// as 'stable/mitaka', master, etc.
134140
func IsCurrentAbove(t *testing.T, release string) bool {
135-
current_branch := getReleaseFromEnv(t)
136-
137-
// Assume master is always too new
138-
if current_branch == "master" || current_branch > release {
139-
return true
140-
}
141-
t.Logf("Target release %s is below the current branch %s", release, current_branch)
141+
current := getReleaseFromEnv(t)
142+
release = strings.TrimPrefix(release, "stable/")
143+
144+
if release != "master" {
145+
// Assume master is always too new
146+
if current == "master" {
147+
return true
148+
}
149+
// Numeral releases are always newer than non-numeral ones
150+
if isReleaseNumeral(current) && !isReleaseNumeral(release) {
151+
return true
152+
}
153+
if current > release && !(!isReleaseNumeral(current) && isReleaseNumeral(release)) {
154+
return true
155+
}
156+
}
157+
t.Logf("Target release %s is below the current branch %s", release, current)
142158
return false
143159
}
144160

145161
// IsCurrentBelow will return true on releases below a certain
146162
// one. Releases are named such as 'stable/mitaka', master, etc.
147163
func IsCurrentBelow(t *testing.T, release string) bool {
148-
current_branch := getReleaseFromEnv(t)
149-
150-
if current_branch != "master" || current_branch < release {
151-
return true
152-
}
153-
t.Logf("Target release %s is above the current branch %s", release, current_branch)
164+
current := getReleaseFromEnv(t)
165+
release = strings.TrimPrefix(release, "stable/")
166+
167+
if current != "master" {
168+
// Assume master is always too new
169+
if release == "master" {
170+
return true
171+
}
172+
// Numeral releases are always newer than non-numeral ones
173+
if isReleaseNumeral(release) && !isReleaseNumeral(current) {
174+
return true
175+
}
176+
if release > current && !(!isReleaseNumeral(release) && isReleaseNumeral(current)) {
177+
return true
178+
}
179+
}
180+
t.Logf("Target release %s is above the current branch %s", release, current)
154181
return false
155182
}

‎acceptance/clients/testing/conditions_test.go‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
package testing
2+
3+
import (
4+
"fmt"
5+
"os"
6+
"testing"
7+
8+
"github.com/gophercloud/gophercloud/acceptance/clients"
9+
)
10+
11+
func TestIsCurrentAbove(t *testing.T) {
12+
cases := []struct {
13+
Current string
14+
Release string
15+
Result bool
16+
}{
17+
{Current: "master", Release: "zed", Result: true},
18+
{Current: "master", Release: "2023.1", Result: true},
19+
{Current: "master", Release: "master", Result: false},
20+
{Current: "zed", Release: "master", Result: false},
21+
{Current: "zed", Release: "yoga", Result: true},
22+
{Current: "zed", Release: "2023.1", Result: false},
23+
{Current: "2023.1", Release: "2023.1", Result: false},
24+
{Current: "2023.2", Release: "stable/2023.1", Result: true},
25+
}
26+
27+
for _, tt := range cases {
28+
t.Run(fmt.Sprintf("%s above %s", tt.Current, tt.Release), func(t *testing.T) {
29+
os.Setenv("OS_BRANCH", tt.Current)
30+
got := clients.IsCurrentAbove(t, tt.Release)
31+
if got != tt.Result {
32+
t.Errorf("got %v want %v", got, tt.Result)
33+
}
34+
})
35+
36+
}
37+
}
38+
39+
func TestIsCurrentBelow(t *testing.T) {
40+
cases := []struct {
41+
Current string
42+
Release string
43+
Result bool
44+
}{
45+
{Current: "master", Release: "zed", Result: false},
46+
{Current: "master", Release: "2023.1", Result: false},
47+
{Current: "master", Release: "master", Result: false},
48+
{Current: "zed", Release: "master", Result: true},
49+
{Current: "zed", Release: "yoga", Result: false},
50+
{Current: "zed", Release: "2023.1", Result: true},
51+
{Current: "2023.1", Release: "2023.1", Result: false},
52+
{Current: "2023.2", Release: "stable/2023.1", Result: false},
53+
}
54+
55+
for _, tt := range cases {
56+
t.Run(fmt.Sprintf("%s below %s", tt.Current, tt.Release), func(t *testing.T) {
57+
os.Setenv("OS_BRANCH", tt.Current)
58+
got := clients.IsCurrentBelow(t, tt.Release)
59+
if got != tt.Result {
60+
t.Errorf("got %v want %v", got, tt.Result)
61+
}
62+
})
63+
64+
}
65+
}

0 commit comments

Comments
 (0)