Skip to content

Commit

Permalink
Tests: Simplify VersionUtils released version splitting (#30322)
Browse files Browse the repository at this point in the history 
This commit refactors VersionUtils.resolveReleasedVersions to be
simpler, and in the process fixes the behavior to match that of
VersionCollection.groovy.

closes #30133
  • Loading branch information
@rjernst
rjernst committed May 2, 2018
1 parent 9a4c483 commit 6f29c73
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 89 deletions.
134 changes: 62 additions & 72 deletions test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,26 +19,25 @@

package org.elasticsearch.test;

import org.elasticsearch.Version;
import org.elasticsearch.common.Booleans;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.Tuple;

import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Random;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static java.util.Collections.singletonList;
import static java.util.Collections.unmodifiableList;
import org.elasticsearch.Version;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.collect.Tuple;

/** Utilities for selecting versions in tests */
public class VersionUtils {

/**
* Sort versions that have backwards compatibility guarantees from
* those that don't. Doesn't actually check whether or not the versions
Expand All @@ -50,73 +49,65 @@ public class VersionUtils {
* guarantees in v1 and versions without the guranteees in v2
*/
static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version current, Class<?> versionClass) {
List<Version> versions = Version.getDeclaredVersions(versionClass);

Version last = versions.remove(versions.size() - 1);
assert last.equals(current) : "The highest version must be the current one "
+ "but was [" + versions.get(versions.size() - 1) + "] and current was [" + current + "]";

if (current.revision != 0) {
/* If we are in a stable branch there should be no unreleased version constants
* because we don't expect to release any new versions in older branches. If there
* are extra constants then gradle will yell about it. */
return new Tuple<>(unmodifiableList(versions), singletonList(current));
// group versions into major version
Map<Integer, List<Version>> majorVersions = Version.getDeclaredVersions(versionClass).stream()
.collect(Collectors.groupingBy(v -> (int)v.major));
// this breaks b/c 5.x is still in version list but master doesn't care about it!
//assert majorVersions.size() == 2;
// TODO: remove oldVersions, we should only ever have 2 majors in Version
List<Version> oldVersions = majorVersions.getOrDefault((int)current.major - 2, Collections.emptyList());
List<List<Version>> previousMajor = splitByMinor(majorVersions.get((int)current.major - 1));
List<List<Version>> currentMajor = splitByMinor(majorVersions.get((int)current.major));

List<Version> unreleasedVersions = new ArrayList<>();
final List<List<Version>> stableVersions;
if (currentMajor.size() == 1) {
// on master branch
stableVersions = previousMajor;
// remove current
moveLastToUnreleased(currentMajor, unreleasedVersions);
} else {
// on a stable or release branch, ie N.x
stableVersions = currentMajor;
// remove the next maintenance bugfix
moveLastToUnreleased(previousMajor, unreleasedVersions);
}

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

Version unreleased = versions.remove(unreleasedIndex);
if (unreleased.revision == 0) {
/*
* If the last unreleased version is itself a patch release then Gradle enforces that there is yet another unreleased version
* 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,
* 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
* 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
* we have to skip backwards until we find a non-alpha/beta/RC again. Then we can prune that version as an unreleased version
* too.
*/
do {
unreleasedIndex--;
} while (versions.get(unreleasedIndex).isRelease() == false);
Version earlierUnreleased = versions.remove(unreleasedIndex);

// This earlierUnreleased is either the snapshot on the minor branch lower, or its possible its a staged release. If it is a
// staged release, remove it and return it in unreleased as well.
if (earlierUnreleased.revision == 0) {
unreleasedIndex--;
Version actualUnreleasedPreviousMinor = versions.remove(unreleasedIndex);
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(actualUnreleasedPreviousMinor,
earlierUnreleased, unreleased, current)));
}
List<Version> releasedVersions = Stream.concat(oldVersions.stream(),
Stream.concat(previousMajor.stream(), currentMajor.stream()).flatMap(List::stream))
.collect(Collectors.toList());
Collections.sort(unreleasedVersions); // we add unreleased out of order, so need to sort here
return new Tuple<>(Collections.unmodifiableList(releasedVersions), Collections.unmodifiableList(unreleasedVersions));
}

return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierUnreleased, unreleased, current)));
} else if (unreleased.major == current.major) {
// need to remove one more of the last major's minor set
do {
unreleasedIndex--;
} while (unreleasedIndex > 0 && versions.get(unreleasedIndex).major == current.major);
if (unreleasedIndex > 0) {
// 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
Version earlierMajorsMinor = versions.remove(unreleasedIndex);
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(earlierMajorsMinor, unreleased, current)));
}
// split the given versions into sub lists grouped by minor version
private static List<List<Version>> splitByMinor(List<Version> versions) {
Map<Integer, List<Version>> byMinor = versions.stream().collect(Collectors.groupingBy(v -> (int)v.minor));
return byMinor.entrySet().stream().sorted(Map.Entry.comparingByKey()).map(Map.Entry::getValue).collect(Collectors.toList());
}

// move the last version of the last minor in versions to the unreleased versions
private static Version moveLastToUnreleased(List<List<Version>> versions, List<Version> unreleasedVersions) {
List<Version> lastMinor = new ArrayList<>(versions.get(versions.size() - 1));
Version lastVersion = lastMinor.remove(lastMinor.size() - 1);
if (lastMinor.isEmpty()) {
versions.remove(versions.size() - 1);
} else {
versions.set(versions.size() - 1, lastMinor);
}
return new Tuple<>(unmodifiableList(versions), unmodifiableList(Arrays.asList(unreleased, current)));
unreleasedVersions.add(lastVersion);
return lastVersion;
}

private static final List<Version> RELEASED_VERSIONS;
Expand All @@ -131,7 +122,7 @@ static Tuple<List<Version>, List<Version>> resolveReleasedVersions(Version curre
allVersions.addAll(RELEASED_VERSIONS);
allVersions.addAll(UNRELEASED_VERSIONS);
Collections.sort(allVersions);
ALL_VERSIONS = unmodifiableList(allVersions);
ALL_VERSIONS = Collections.unmodifiableList(allVersions);
}

/**
Expand Down Expand Up @@ -244,5 +235,4 @@ public static Version maxCompatibleVersion(Version version) {
assert compatible.size() > 0;
return compatible.get(compatible.size() - 1);
}

}
48 changes: 31 additions & 17 deletions test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public void testAllVersionsSorted() {
}

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

// sub range
got = VersionUtils.randomVersionBetween(random(), Version.V_5_0_0,
Version.V_6_0_0_beta1);
assertTrue(got.onOrAfter(Version.V_5_0_0));
assertTrue(got.onOrBefore(Version.V_6_0_0_beta1));
got = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0_alpha1,
Version.V_6_2_4);
assertTrue(got.onOrAfter(Version.V_6_0_0_alpha1));
assertTrue(got.onOrBefore(Version.V_6_2_4));

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

// unbounded upper
got = VersionUtils.randomVersionBetween(random(), Version.V_5_0_0, null);
assertTrue(got.onOrAfter(Version.V_5_0_0));
got = VersionUtils.randomVersionBetween(random(), Version.V_6_0_0, null);
assertTrue(got.onOrAfter(Version.V_6_0_0));
assertTrue(got.onOrBefore(Version.CURRENT));
got = VersionUtils.randomVersionBetween(random(), VersionUtils.getPreviousVersion(), null);
assertTrue(got.onOrAfter(VersionUtils.getPreviousVersion()));
Expand Down Expand Up @@ -100,6 +101,8 @@ public void testRandomVersionBetween() {
}

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

assertThat(released, equalTo(Arrays.asList(
TestReleaseBranch.V_4_0_0,
TestReleaseBranch.V_5_3_0,
TestReleaseBranch.V_5_3_1,
TestReleaseBranch.V_5_3_2,
TestReleaseBranch.V_5_4_0)));
assertThat(unreleased, equalTo(Collections.singletonList(TestReleaseBranch.V_5_4_1)));
assertThat(unreleased, equalTo(Arrays.asList(
TestReleaseBranch.V_4_0_1,
TestReleaseBranch.V_5_4_1)));
}

public static class TestStableBranch {
public static final Version V_5_3_0 = Version.fromString("5.3.0");
public static final Version V_5_3_1 = Version.fromString("5.3.1");
public static final Version V_5_3_2 = Version.fromString("5.3.2");
public static final Version V_5_4_0 = Version.fromString("5.4.0");
public static final Version CURRENT = V_5_4_0;
public static final Version V_4_0_0 = Version.fromString("4.0.0");
public static final Version V_4_0_1 = Version.fromString("4.0.1");
public static final Version V_5_0_0 = Version.fromString("5.0.0");
public static final Version V_5_0_1 = Version.fromString("5.0.1");
public static final Version V_5_0_2 = Version.fromString("5.0.2");
public static final Version V_5_1_0 = Version.fromString("5.1.0");
public static final Version CURRENT = V_5_1_0;
}
public void testResolveReleasedVersionsForUnreleasedStableBranch() {
Tuple<List<Version>, List<Version>> t = VersionUtils.resolveReleasedVersions(TestStableBranch.CURRENT,
Expand All @@ -134,14 +142,18 @@ public void testResolveReleasedVersionsForUnreleasedStableBranch() {
List<Version> unreleased = t.v2();

assertThat(released, equalTo(Arrays.asList(
TestStableBranch.V_5_3_0,
TestStableBranch.V_5_3_1)));
TestStableBranch.V_4_0_0,
TestStableBranch.V_5_0_0,
TestStableBranch.V_5_0_1)));
assertThat(unreleased, equalTo(Arrays.asList(
TestStableBranch.V_5_3_2,
TestStableBranch.V_5_4_0)));
TestStableBranch.V_4_0_1,
TestStableBranch.V_5_0_2,
TestStableBranch.V_5_1_0)));
}

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

assertThat(released, equalTo(Arrays.asList(
TestStableBranchBehindStableBranch.V_4_0_0,
TestStableBranchBehindStableBranch.V_5_3_0,
TestStableBranchBehindStableBranch.V_5_3_1)));
assertThat(unreleased, equalTo(Arrays.asList(
TestStableBranchBehindStableBranch.V_4_0_1,
TestStableBranchBehindStableBranch.V_5_3_2,
TestStableBranchBehindStableBranch.V_5_4_0,
TestStableBranchBehindStableBranch.V_5_5_0)));
Expand Down Expand Up @@ -214,13 +228,13 @@ public void testResolveReleasedVersionsAtNewMajorRelease() {
assertThat(released, equalTo(Arrays.asList(
TestNewMajorRelease.V_5_6_0,
TestNewMajorRelease.V_5_6_1,
TestNewMajorRelease.V_5_6_2,
TestNewMajorRelease.V_6_0_0_alpha1,
TestNewMajorRelease.V_6_0_0_alpha2,
TestNewMajorRelease.V_6_0_0_beta1,
TestNewMajorRelease.V_6_0_0_beta2,
TestNewMajorRelease.V_6_0_0)));
assertThat(unreleased, equalTo(Arrays.asList(
TestNewMajorRelease.V_5_6_2,
TestNewMajorRelease.V_6_0_1)));
}

Expand Down

0 comments on commit 6f29c73

Please sign in to comment.