From 0f1ad52aa06aded8068333562a296524baef46c7 Mon Sep 17 00:00:00 2001 From: Michael Basnight Date: Mon, 12 Feb 2018 12:25:34 -0600 Subject: [PATCH] Fix build.snapshot bug in version collection The build.snapshot was mistakenly passed in to every snapshot version, so when release tests were run, these versions were mistaken as released entities and could not be found in maven, because they do not exist. This fix removes that bug in logic, and always makes them proper snapshots. This has a benefit of cleaning up the VersionUtilsTests because they no longer rely on different sets of versions to check against, which was also a bug. --- .../gradle/VersionCollection.groovy | 6 +- .../org/elasticsearch/test/VersionUtils.java | 4 - .../elasticsearch/test/VersionUtilsTests.java | 239 +++++------------- 3 files changed, 68 insertions(+), 181 deletions(-) diff --git a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionCollection.groovy b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionCollection.groovy index 6e14777b7c639..7d5b793254fe4 100644 --- a/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionCollection.groovy +++ b/buildSrc/src/main/groovy/org/elasticsearch/gradle/VersionCollection.groovy @@ -64,10 +64,7 @@ class VersionCollection { 'next-bugfix-snapshot', 'maintenance-bugfix-snapshot'] - - // When we roll 8.0 its very likely these will need to be extracted from this class - private final boolean buildSnapshot = System.getProperty("build.snapshot", "true") == "true" private final boolean isReleasableBranch = true /** @@ -76,6 +73,7 @@ class VersionCollection { * @param versionLines The lines of the Version.java file. */ VersionCollection(List versionLines) { + final boolean buildSnapshot = System.getProperty("build.snapshot", "true") == "true" List versions = [] // This class should be converted wholesale to use the treeset @@ -303,7 +301,7 @@ class VersionCollection { */ private Version replaceAsSnapshot(Version version) { versionSet.remove(version) - Version snapshotVersion = new Version(version.major, version.minor, version.revision, version.suffix, buildSnapshot) + Version snapshotVersion = new Version(version.major, version.minor, version.revision, version.suffix, true) safeAddToSet(snapshotVersion) return snapshotVersion } diff --git a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java index 0e57031459a4a..bca13f6018203 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java +++ b/test/framework/src/main/java/org/elasticsearch/test/VersionUtils.java @@ -52,10 +52,6 @@ public class VersionUtils { static Tuple, List> resolveReleasedVersions(Version current, Class versionClass) { List versions = Version.getDeclaredVersions(versionClass); - if (!Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - return Tuple.tuple(versions, Collections.emptyList()); - } - 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 + "]"; diff --git a/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java b/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java index e1c3ac82f7865..95941554c00ce 100644 --- a/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java +++ b/test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java @@ -119,27 +119,12 @@ public void testResolveReleasedVersionsForReleaseBranch() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList( - TestReleaseBranch.V_5_3_0, - TestReleaseBranch.V_5_3_1, - TestReleaseBranch.V_5_3_2, - TestReleaseBranch.V_5_4_0); - expectedUnreleased = Collections.singletonList(TestReleaseBranch.V_5_4_1); - } else { - expectedReleased = Arrays.asList( - TestReleaseBranch.V_5_3_0, - TestReleaseBranch.V_5_3_1, - TestReleaseBranch.V_5_3_2, - TestReleaseBranch.V_5_4_0, - TestReleaseBranch.V_5_4_1); - expectedUnreleased = Collections.emptyList(); - } - - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + 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))); } public static class TestStableBranch { @@ -155,19 +140,12 @@ public void testResolveReleasedVersionsForUnreleasedStableBranch() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList(TestStableBranch.V_5_3_0, TestStableBranch.V_5_3_1); - expectedUnreleased = Arrays.asList(TestStableBranch.V_5_3_2, TestStableBranch.V_5_4_0); - } else { - expectedReleased = - Arrays.asList(TestStableBranch.V_5_3_0, TestStableBranch.V_5_3_1, TestStableBranch.V_5_3_2, TestStableBranch.V_5_4_0); - expectedUnreleased = Collections.emptyList(); - } - - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + TestStableBranch.V_5_3_0, + TestStableBranch.V_5_3_1))); + assertThat(unreleased, equalTo(Arrays.asList( + TestStableBranch.V_5_3_2, + TestStableBranch.V_5_4_0))); } public static class TestStableBranchBehindStableBranch { @@ -184,25 +162,13 @@ public void testResolveReleasedVersionsForStableBranchBehindStableBranch() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList(TestStableBranchBehindStableBranch.V_5_3_0, TestStableBranchBehindStableBranch.V_5_3_1); - expectedUnreleased = Arrays.asList( - TestStableBranchBehindStableBranch.V_5_3_2, - TestStableBranchBehindStableBranch.V_5_4_0, - TestStableBranchBehindStableBranch.V_5_5_0); - } else { - expectedReleased = Arrays.asList( - TestStableBranchBehindStableBranch.V_5_3_0, - TestStableBranchBehindStableBranch.V_5_3_1, - TestStableBranchBehindStableBranch.V_5_3_2, - TestStableBranchBehindStableBranch.V_5_4_0, - TestStableBranchBehindStableBranch.V_5_5_0); - expectedUnreleased = Collections.emptyList(); - } - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + TestStableBranchBehindStableBranch.V_5_3_0, + TestStableBranchBehindStableBranch.V_5_3_1))); + assertThat(unreleased, equalTo(Arrays.asList( + TestStableBranchBehindStableBranch.V_5_3_2, + TestStableBranchBehindStableBranch.V_5_4_0, + TestStableBranchBehindStableBranch.V_5_5_0))); } public static class TestUnstableBranch { @@ -222,28 +188,15 @@ public void testResolveReleasedVersionsForUnstableBranch() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList( - TestUnstableBranch.V_5_3_0, - TestUnstableBranch.V_5_3_1, - TestUnstableBranch.V_6_0_0_alpha1, - TestUnstableBranch.V_6_0_0_alpha2); - expectedUnreleased = Arrays.asList(TestUnstableBranch.V_5_3_2, TestUnstableBranch.V_5_4_0, TestUnstableBranch.V_6_0_0_beta1); - } else { - expectedReleased = Arrays.asList( - TestUnstableBranch.V_5_3_0, - TestUnstableBranch.V_5_3_1, - TestUnstableBranch.V_5_3_2, - TestUnstableBranch.V_5_4_0, - TestUnstableBranch.V_6_0_0_alpha1, - TestUnstableBranch.V_6_0_0_alpha2, - TestUnstableBranch.V_6_0_0_beta1); - expectedUnreleased = Collections.emptyList(); - } - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + TestUnstableBranch.V_5_3_0, + TestUnstableBranch.V_5_3_1, + TestUnstableBranch.V_6_0_0_alpha1, + TestUnstableBranch.V_6_0_0_alpha2))); + assertThat(unreleased, equalTo(Arrays.asList( + TestUnstableBranch.V_5_3_2, + TestUnstableBranch.V_5_4_0, + TestUnstableBranch.V_6_0_0_beta1))); } public static class TestNewMajorRelease { @@ -265,35 +218,17 @@ public void testResolveReleasedVersionsAtNewMajorRelease() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = 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); - expectedUnreleased = Arrays.asList(TestNewMajorRelease.V_6_0_1); - } else { - expectedReleased = 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, - TestNewMajorRelease.V_6_0_1); - expectedUnreleased = Collections.emptyList(); - } - - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + 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_6_0_1))); } public static class TestVersionBumpIn6x { @@ -316,37 +251,18 @@ public void testResolveReleasedVersionsAtVersionBumpIn6x() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList( - TestVersionBumpIn6x.V_5_6_0, - TestVersionBumpIn6x.V_5_6_1, - TestVersionBumpIn6x.V_5_6_2, - TestVersionBumpIn6x.V_6_0_0_alpha1, - TestVersionBumpIn6x.V_6_0_0_alpha2, - TestVersionBumpIn6x.V_6_0_0_beta1, - TestVersionBumpIn6x.V_6_0_0_beta2, - TestVersionBumpIn6x.V_6_0_0); - expectedUnreleased = Arrays.asList(TestVersionBumpIn6x.V_6_0_1, TestVersionBumpIn6x.V_6_1_0); - } else { - expectedReleased = Arrays.asList( - TestVersionBumpIn6x.V_5_6_0, - TestVersionBumpIn6x.V_5_6_1, - TestVersionBumpIn6x.V_5_6_2, - TestVersionBumpIn6x.V_6_0_0_alpha1, - TestVersionBumpIn6x.V_6_0_0_alpha2, - TestVersionBumpIn6x.V_6_0_0_beta1, - TestVersionBumpIn6x.V_6_0_0_beta2, - TestVersionBumpIn6x.V_6_0_0, - TestVersionBumpIn6x.V_6_0_1, - TestVersionBumpIn6x.V_6_1_0); - expectedUnreleased = Collections.emptyList(); - } - - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + TestVersionBumpIn6x.V_5_6_0, + TestVersionBumpIn6x.V_5_6_1, + TestVersionBumpIn6x.V_5_6_2, + TestVersionBumpIn6x.V_6_0_0_alpha1, + TestVersionBumpIn6x.V_6_0_0_alpha2, + TestVersionBumpIn6x.V_6_0_0_beta1, + TestVersionBumpIn6x.V_6_0_0_beta2, + TestVersionBumpIn6x.V_6_0_0))); + assertThat(unreleased, equalTo(Arrays.asList( + TestVersionBumpIn6x.V_6_0_1, + TestVersionBumpIn6x.V_6_1_0))); } public static class TestNewMinorBranchIn6x { @@ -372,44 +288,21 @@ public void testResolveReleasedVersionsAtNewMinorBranchIn6x() { List released = t.v1(); List unreleased = t.v2(); - final List expectedReleased; - final List expectedUnreleased; - if (Booleans.parseBoolean(System.getProperty("build.snapshot", "true"))) { - expectedReleased = Arrays.asList( - TestNewMinorBranchIn6x.V_5_6_0, - TestNewMinorBranchIn6x.V_5_6_1, - TestNewMinorBranchIn6x.V_5_6_2, - TestNewMinorBranchIn6x.V_6_0_0_alpha1, - TestNewMinorBranchIn6x.V_6_0_0_alpha2, - TestNewMinorBranchIn6x.V_6_0_0_beta1, - TestNewMinorBranchIn6x.V_6_0_0_beta2, - TestNewMinorBranchIn6x.V_6_0_0, - TestNewMinorBranchIn6x.V_6_0_1, - TestNewMinorBranchIn6x.V_6_1_0, - TestNewMinorBranchIn6x.V_6_1_1); - expectedUnreleased = Arrays.asList( - TestNewMinorBranchIn6x.V_6_1_2, - TestNewMinorBranchIn6x.V_6_2_0); - } else { - expectedReleased = Arrays.asList( - TestNewMinorBranchIn6x.V_5_6_0, - TestNewMinorBranchIn6x.V_5_6_1, - TestNewMinorBranchIn6x.V_5_6_2, - TestNewMinorBranchIn6x.V_6_0_0_alpha1, - TestNewMinorBranchIn6x.V_6_0_0_alpha2, - TestNewMinorBranchIn6x.V_6_0_0_beta1, - TestNewMinorBranchIn6x.V_6_0_0_beta2, - TestNewMinorBranchIn6x.V_6_0_0, - TestNewMinorBranchIn6x.V_6_0_1, - TestNewMinorBranchIn6x.V_6_1_0, - TestNewMinorBranchIn6x.V_6_1_1, - TestNewMinorBranchIn6x.V_6_1_2, - TestNewMinorBranchIn6x.V_6_2_0); - expectedUnreleased = Collections.emptyList(); - } - - assertThat(released, equalTo(expectedReleased)); - assertThat(unreleased, equalTo(expectedUnreleased)); + assertThat(released, equalTo(Arrays.asList( + TestNewMinorBranchIn6x.V_5_6_0, + TestNewMinorBranchIn6x.V_5_6_1, + TestNewMinorBranchIn6x.V_5_6_2, + TestNewMinorBranchIn6x.V_6_0_0_alpha1, + TestNewMinorBranchIn6x.V_6_0_0_alpha2, + TestNewMinorBranchIn6x.V_6_0_0_beta1, + TestNewMinorBranchIn6x.V_6_0_0_beta2, + TestNewMinorBranchIn6x.V_6_0_0, + TestNewMinorBranchIn6x.V_6_0_1, + TestNewMinorBranchIn6x.V_6_1_0, + TestNewMinorBranchIn6x.V_6_1_1))); + assertThat(unreleased, equalTo(Arrays.asList( + TestNewMinorBranchIn6x.V_6_1_2, + TestNewMinorBranchIn6x.V_6_2_0))); } /**