Skip to content

Commit b4abdc5

Browse files
committed
Tests: Simplify VersionUtils released version splitting (#30322)
This commit refactors VersionUtils.resolveReleasedVersions to be simpler, and in the process fixes the behavior to match that of VersionCollection.groovy. closes #30133
1 parent 763769a commit b4abdc5

File tree

2 files changed

+93
-89
lines changed

2 files changed

+93
-89
lines changed

test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java

+62-72
Original file line numberDiff line numberDiff line change
@@ -19,26 +19,25 @@
1919

2020
package org.elasticsearch.test;
2121

22-
import org.elasticsearch.Version;
23-
import org.elasticsearch.common.Booleans;
24-
import org.elasticsearch.common.Nullable;
25-
import org.elasticsearch.common.collect.Tuple;
26-
27-
import java.lang.reflect.Field;
28-
import java.lang.reflect.Modifier;
2922
import java.util.ArrayList;
30-
import java.util.Arrays;
3123
import java.util.Collections;
3224
import java.util.List;
25+
import java.util.Locale;
26+
import java.util.Map;
3327
import java.util.Optional;
3428
import java.util.Random;
29+
import java.util.SortedSet;
30+
import java.util.TreeSet;
3531
import java.util.stream.Collectors;
32+
import java.util.stream.Stream;
3633

37-
import static java.util.Collections.singletonList;
38-
import static java.util.Collections.unmodifiableList;
34+
import org.elasticsearch.Version;
35+
import org.elasticsearch.common.Nullable;
36+
import org.elasticsearch.common.collect.Tuple;
3937

4038
/** Utilities for selecting versions in tests */
4139
public class VersionUtils {
40+
4241
/**
4342
* Sort versions that have backwards compatibility guarantees from
4443
* those that don't. Doesn't actually check whether or not the versions
@@ -50,73 +49,65 @@ public class VersionUtils {
5049
* guarantees in v1 and versions without the guranteees in v2
5150
*/
5251
static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version current, Class<?> versionClass) {
53-
List<Version> versions = Version.getDeclaredVersions(versionClass);
54-
55-
Version last = versions.remove(versions.size() - 1);
56-
assert last.equals(current) : "The highest version must be the current one "
57-
+ "but was [" + versions.get(versions.size() - 1) + "] and current was [" + current + "]";
58-
59-
if (current.revision != 0) {
60-
/* If we are in a stable branch there should be no unreleased version constants
61-
* because we don't expect to release any new versions in older branches. If there
62-
* are extra constants then gradle will yell about it. */
63-
return new Tuple<>(unmodifiableList(versions), singletonList(current));
52+
// group versions into major version
53+
Map<Integer, List<Version>> majorVersions = Version.getDeclaredVersions(versionClass).stream()
54+
.collect(Collectors.groupingBy(v -> (int)v.major));
55+
// this breaks b/c 5.x is still in version list but master doesn't care about it!
56+
//assert majorVersions.size() == 2;
57+
// TODO: remove oldVersions, we should only ever have 2 majors in Version
58+
List<Version> oldVersions = majorVersions.getOrDefault((int)current.major - 2, Collections.emptyList());
59+
List<List<Version>> previousMajor = splitByMinor(majorVersions.get((int)current.major - 1));
60+
List<List<Version>> currentMajor = splitByMinor(majorVersions.get((int)current.major));
61+
62+
List<Version> unreleasedVersions = new ArrayList<>();
63+
final List<List<Version>> stableVersions;
64+
if (currentMajor.size() == 1) {
65+
// on master branch
66+
stableVersions = previousMajor;
67+
// remove current
68+
moveLastToUnreleased(currentMajor, unreleasedVersions);
69+
} else {
70+
// on a stable or release branch, ie N.x
71+
stableVersions = currentMajor;
72+
// remove the next maintenance bugfix
73+
moveLastToUnreleased(previousMajor, unreleasedVersions);
6474
}
6575

66-
/* If we are on a patch release then we know that at least the version before the
67-
* current one is unreleased. If it is released then gradle would be complaining. */
68-
int unreleasedIndex = versions.size() - 1;
69-
while (true) {
70-
if (unreleasedIndex < 0) {
71-
throw new IllegalArgumentException("Couldn't find first non-alpha release");
72-
}
73-
/* We don't support backwards compatibility for alphas, betas, and rcs. But
74-
* they were released so we add them to the released list. Usually this doesn't
75-
* matter to consumers, but consumers that do care should filter non-release
76-
* versions. */
77-
if (versions.get(unreleasedIndex).isRelease()) {
78-
break;
76+
// remove next minor
77+
Version lastMinor = moveLastToUnreleased(stableVersions, unreleasedVersions);
78+
if (lastMinor.revision == 0) {
79+
if (stableVersions.get(stableVersions.size() - 1).size() == 1) {
80+
// a minor is being staged, which is also unreleased
81+
moveLastToUnreleased(stableVersions, unreleasedVersions);
7982
}
80-
unreleasedIndex--;
83+
// remove the next bugfix
84+
moveLastToUnreleased(stableVersions, unreleasedVersions);
8185
}
8286

83-
Version unreleased = versions.remove(unreleasedIndex);
84-
if (unreleased.revision == 0) {
85-
/*
86-
* If the last unreleased version is itself a patch release then Gradle enforces that there is yet another unreleased version
87-
* before that. However, we have to skip alpha/betas/RCs too (e.g., consider when the version constants are ..., 5.6.3, 5.6.4,
88-
* 6.0.0-alpha1, ..., 6.0.0-rc1, 6.0.0-rc2, 6.0.0, 6.1.0 on the 6.x branch. In this case, we will have pruned 6.0.0 and 6.1.0 as
89-
* unreleased versions, but we also need to prune 5.6.4. At this point though, unreleasedIndex will be pointing to 6.0.0-rc2, so
90-
* we have to skip backwards until we find a non-alpha/beta/RC again. Then we can prune that version as an unreleased version
91-
* too.
92-
*/
93-
do {
94-
unreleasedIndex--;
95-
} while (versions.get(unreleasedIndex).isRelease() == false);
96-
Version earlierUnreleased = versions.remove(unreleasedIndex);
97-
98-
// This earlierUnreleased is either the snapshot on the minor branch lower, or its possible its a staged release. If it is a
99-
// staged release, remove it and return it in unreleased as well.
100-
if (earlierUnreleased.revision == 0) {
101-
unreleasedIndex--;
102-
Version actualUnreleasedPreviousMinor = versions.remove(unreleasedIndex);
103-
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(actualUnreleasedPreviousMinor,
104-
earlierUnreleased, unreleased, current)));
105-
}
87+
List<Version> releasedVersions = Stream.concat(oldVersions.stream(),
88+
Stream.concat(previousMajor.stream(), currentMajor.stream()).flatMap(List::stream))
89+
.collect(Collectors.toList());
90+
Collections.sort(unreleasedVersions); // we add unreleased out of order, so need to sort here
91+
return new Tuple<>(Collections.unmodifiableList(releasedVersions), Collections.unmodifiableList(unreleasedVersions));
92+
}
10693

107-
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierUnreleased, unreleased, current)));
108-
} else if (unreleased.major == current.major) {
109-
// need to remove one more of the last major's minor set
110-
do {
111-
unreleasedIndex--;
112-
} while (unreleasedIndex > 0 && versions.get(unreleasedIndex).major == current.major);
113-
if (unreleasedIndex > 0) {
114-
// some of the test cases return very small lists, so its possible this is just the end of the list, if so, dont include it
115-
Version earlierMajorsMinor = versions.remove(unreleasedIndex);
116-
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierMajorsMinor, unreleased, current)));
117-
}
94+
// split the given versions into sub lists grouped by minor version
95+
private static List<List<Version>> splitByMinor(List<Version> versions) {
96+
Map<Integer, List<Version>> byMinor = versions.stream().collect(Collectors.groupingBy(v -> (int)v.minor));
97+
return byMinor.entrySet().stream().sorted(Map.Entry.comparingByKey()).map(Map.Entry::getValue).collect(Collectors.toList());
98+
}
99+
100+
// move the last version of the last minor in versions to the unreleased versions
101+
private static Version moveLastToUnreleased(List<List<Version>> versions, List<Version> unreleasedVersions) {
102+
List<Version> lastMinor = new ArrayList<>(versions.get(versions.size() - 1));
103+
Version lastVersion = lastMinor.remove(lastMinor.size() - 1);
104+
if (lastMinor.isEmpty()) {
105+
versions.remove(versions.size() - 1);
106+
} else {
107+
versions.set(versions.size() - 1, lastMinor);
118108
}
119-
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(unreleased, current)));
109+
unreleasedVersions.add(lastVersion);
110+
return lastVersion;
120111
}
121112

122113
private static final List<Version> RELEASED_VERSIONS;
@@ -131,7 +122,7 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
131122
allVersions.addAll(RELEASED_VERSIONS);
132123
allVersions.addAll(UNRELEASED_VERSIONS);
133124
Collections.sort(allVersions);
134-
ALL_VERSIONS = unmodifiableList(allVersions);
125+
ALL_VERSIONS = Collections.unmodifiableList(allVersions);
135126
}
136127

137128
/**
@@ -244,5 +235,4 @@ public static Version maxCompatibleVersion(Version version) {
244235
assert compatible.size() > 0;
245236
return compatible.get(compatible.size() - 1);
246237
}
247-
248238
}

test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java

+31-17
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ public void testAllVersionsSorted() {
4242
}
4343

4444
public void testRandomVersionBetween() {
45+
// TODO: rework this test to use a dummy Version class so these don't need to change with each release
4546
// full range
4647
Version got = VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), Version.CURRENT);
4748
assertTrue(got.onOrAfter(VersionUtils.getFirstVersion()));
@@ -54,10 +55,10 @@ public void testRandomVersionBetween() {
5455
assertTrue(got.onOrBefore(Version.CURRENT));
5556

5657
// sub range
57-
got = VersionUtils.randomVersionBetween(random(), Version.V_5_0_0,
58-
Version.V_6_0_0_beta1);
59-
assertTrue(got.onOrAfter(Version.V_5_0_0));
60-
assertTrue(got.onOrBefore(Version.V_6_0_0_beta1));
58+
got = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0_alpha1,
59+
Version.V_6_2_4);
60+
assertTrue(got.onOrAfter(Version.V_6_0_0_alpha1));
61+
assertTrue(got.onOrBefore(Version.V_6_2_4));
6162

6263
// unbounded lower
6364
got = VersionUtils.randomVersionBetween(random(), null, Version.V_6_0_0_beta1);
@@ -68,8 +69,8 @@ public void testRandomVersionBetween() {
6869
assertTrue(got.onOrBefore(VersionUtils.allReleasedVersions().get(0)));
6970

7071
// unbounded upper
71-
got = VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, null);
72-
assertTrue(got.onOrAfter(Version.V_5_0_0));
72+
got = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, null);
73+
assertTrue(got.onOrAfter(Version.V_6_0_0));
7374
assertTrue(got.onOrBefore(Version.CURRENT));
7475
got = VersionUtils.randomVersionBetween(random(), VersionUtils.getPreviousVersion(), null);
7576
assertTrue(got.onOrAfter(VersionUtils.getPreviousVersion()));
@@ -100,6 +101,8 @@ public void testRandomVersionBetween() {
100101
}
101102

102103
public static class TestReleaseBranch {
104+
public static final Version V_4_0_0 = Version.fromString("4.0.0");
105+
public static final Version V_4_0_1 = Version.fromString("4.0.1");
103106
public static final Version V_5_3_0 = Version.fromString("5.3.0");
104107
public static final Version V_5_3_1 = Version.fromString("5.3.1");
105108
public static final Version V_5_3_2 = Version.fromString("5.3.2");
@@ -113,19 +116,24 @@ public void testResolveReleasedVersionsForReleaseBranch() {
113116
List<Version> unreleased = t.v2();
114117

115118
assertThat(released, equalTo(Arrays.asList(
119+
TestReleaseBranch.V_4_0_0,
116120
TestReleaseBranch.V_5_3_0,
117121
TestReleaseBranch.V_5_3_1,
118122
TestReleaseBranch.V_5_3_2,
119123
TestReleaseBranch.V_5_4_0)));
120-
assertThat(unreleased, equalTo(Collections.singletonList(TestReleaseBranch.V_5_4_1)));
124+
assertThat(unreleased, equalTo(Arrays.asList(
125+
TestReleaseBranch.V_4_0_1,
126+
TestReleaseBranch.V_5_4_1)));
121127
}
122128

123129
public static class TestStableBranch {
124-
public static final Version V_5_3_0 = Version.fromString("5.3.0");
125-
public static final Version V_5_3_1 = Version.fromString("5.3.1");
126-
public static final Version V_5_3_2 = Version.fromString("5.3.2");
127-
public static final Version V_5_4_0 = Version.fromString("5.4.0");
128-
public static final Version CURRENT = V_5_4_0;
130+
public static final Version V_4_0_0 = Version.fromString("4.0.0");
131+
public static final Version V_4_0_1 = Version.fromString("4.0.1");
132+
public static final Version V_5_0_0 = Version.fromString("5.0.0");
133+
public static final Version V_5_0_1 = Version.fromString("5.0.1");
134+
public static final Version V_5_0_2 = Version.fromString("5.0.2");
135+
public static final Version V_5_1_0 = Version.fromString("5.1.0");
136+
public static final Version CURRENT = V_5_1_0;
129137
}
130138
public void testResolveReleasedVersionsForUnreleasedStableBranch() {
131139
Tuple<List<Version>, List<Version>> t = VersionUtils.resolveReleasedVersions(TestStableBranch.CURRENT,
@@ -134,14 +142,18 @@ public void testResolveReleasedVersionsForUnreleasedStableBranch() {
134142
List<Version> unreleased = t.v2();
135143

136144
assertThat(released, equalTo(Arrays.asList(
137-
TestStableBranch.V_5_3_0,
138-
TestStableBranch.V_5_3_1)));
145+
TestStableBranch.V_4_0_0,
146+
TestStableBranch.V_5_0_0,
147+
TestStableBranch.V_5_0_1)));
139148
assertThat(unreleased, equalTo(Arrays.asList(
140-
TestStableBranch.V_5_3_2,
141-
TestStableBranch.V_5_4_0)));
149+
TestStableBranch.V_4_0_1,
150+
TestStableBranch.V_5_0_2,
151+
TestStableBranch.V_5_1_0)));
142152
}
143153

144154
public static class TestStableBranchBehindStableBranch {
155+
public static final Version V_4_0_0 = Version.fromString("4.0.0");
156+
public static final Version V_4_0_1 = Version.fromString("4.0.1");
145157
public static final Version V_5_3_0 = Version.fromString("5.3.0");
146158
public static final Version V_5_3_1 = Version.fromString("5.3.1");
147159
public static final Version V_5_3_2 = Version.fromString("5.3.2");
@@ -156,9 +168,11 @@ public void testResolveReleasedVersionsForStableBranchBehindStableBranch() {
156168
List<Version> unreleased = t.v2();
157169

158170
assertThat(released, equalTo(Arrays.asList(
171+
TestStableBranchBehindStableBranch.V_4_0_0,
159172
TestStableBranchBehindStableBranch.V_5_3_0,
160173
TestStableBranchBehindStableBranch.V_5_3_1)));
161174
assertThat(unreleased, equalTo(Arrays.asList(
175+
TestStableBranchBehindStableBranch.V_4_0_1,
162176
TestStableBranchBehindStableBranch.V_5_3_2,
163177
TestStableBranchBehindStableBranch.V_5_4_0,
164178
TestStableBranchBehindStableBranch.V_5_5_0)));
@@ -214,13 +228,13 @@ public void testResolveReleasedVersionsAtNewMajorRelease() {
214228
assertThat(released, equalTo(Arrays.asList(
215229
TestNewMajorRelease.V_5_6_0,
216230
TestNewMajorRelease.V_5_6_1,
217-
TestNewMajorRelease.V_5_6_2,
218231
TestNewMajorRelease.V_6_0_0_alpha1,
219232
TestNewMajorRelease.V_6_0_0_alpha2,
220233
TestNewMajorRelease.V_6_0_0_beta1,
221234
TestNewMajorRelease.V_6_0_0_beta2,
222235
TestNewMajorRelease.V_6_0_0)));
223236
assertThat(unreleased, equalTo(Arrays.asList(
237+
TestNewMajorRelease.V_5_6_2,
224238
TestNewMajorRelease.V_6_0_1)));
225239
}
226240

0 commit comments

Comments
 (0)