Skip to content

Test that gradle and Java version types match#24943

Merged
nik9000 merged 4 commits intoelastic:masterfrom
nik9000:test_gradle_and_java_versions_logic
Jun 3, 2017
Merged

Test that gradle and Java version types match#24943
nik9000 merged 4 commits intoelastic:masterfrom
nik9000:test_gradle_and_java_versions_logic

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented May 29, 2017 •
edited
Loading

Both gradle and java code attempt to infer whether or not each
version in Version.java is released or not and whether or not it is
wire compatible or index compatible. It is super important that
they infer the same things about each version. If they disagree
we might accidentally not be testing backwards compatibility for
some version.

This adds a test to make sure that they agree, modulo known and
accepted differences (mostly around alphas). It also changes the
minimum wire compatible version from the released 5.4.0 to the
unreleased 5.5.0 as that lines up with the gradle logic.

Relates to #24798

@nik9000
Test that gradle and Java version type match
11eef80 
Both gradle and java code attempt to infer the type of a each
Version constant in Version.java. It is super important that
they infer that each constant has the same type. If they disagree
we might accidentally not be testing backwards compatibility for
some version.

This adds a test to make sure that they agree, modulo known and
accepted differences (mostly around alphas). It also changes the
minimum wire compatible version from the released 5.4.0 to the
unreleased 5.5.0 as that lines up with the gradle logic.

Relates to elastic#24798
@nik9000 nik9000 added review >test Issues or PRs that are addressing/adding tests v6.0.0 labels May 29, 2017
@nik9000 nik9000 requested a review from rjernst May 29, 2017 20:14
@nik9000 nik9000 changed the title Test that gradle and Java version type match Test that gradle and Java version types match May 30, 2017
@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented May 30, 2017

@rjernst and @jasontedor I reworked the description of the PR to make this make more sense. I think.

rjernst
rjernst approved these changes Jun 2, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, one minor suggestion.

test/framework/src/test/java/org/elasticsearch/test/VersionUtilsTests.java Outdated
public VersionsFromProperty(String property) {
String versions = System.getProperty(property);
assertNotNull("Couldn't find [" + property + "]. Gradle should set these before running the tests.", versions);
assertThat(versions, startsWith("["));
Copy link
Copy Markdown
Member

@rjernst rjernst Jun 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why pass them in as an array and have to do extra parsing here, instead of a comma separated list?

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Jun 2, 2017

Sigh. I ran all the tests and this exploded. I'm looking into it....

@nik9000
Add a hack to make the version line up
618063a 
Rather than change the minimum compatibility version. Changing
the minimum compatibility version changes too much.
@nik9000 nik9000 merged commit 190f5dc into elastic:master Jun 3, 2017
@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Jun 3, 2017

Thanks for reviewing @rjernst!

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jun 6, 2017
@jasontedor
Merge branch 'master' into fix-get-mapping-head
91d5602 
* master: (1210 commits)
  Add support for clear scroll to high level REST client (elastic#25038)
  Tiny correction in inner-hits.asciidoc (elastic#25066)
  Added release notes for 6.0.0-alpha2
  Expand index expressions against indices only when managing aliases (elastic#23997)
  Collapse inner hits rest test should not skip 5.x
  Settings: Fix secure settings by prefix (elastic#25064)
  add `exclude_keys` option to KeyValueProcessor (elastic#24876)
  Test: update missing body tests to run against versions >= 5.5.0
  Track EWMA[1] of task execution time in search threadpool executor
  Removes an invalid assert in resizing big arrays which does not always hold (resizing can result in a smaller size than the current size, while the assert attempted to verify the new size is always greater than the current).
  Fixed NPEs caused by requests without content. (elastic#23497)
  Plugins can register pre-configured char filters (elastic#25000)
  Build: Allow preserving shared dir (elastic#24962)
  Tests: Make secure settings available from settings builder for tests (elastic#25037)
  [TEST] Skip wildcard expansion test due to breaking change
  Test that gradle and Java version types match (elastic#24943)
  Include duplicate jar when jarhell check fails
  Change ScriptContexts to use needs instead of uses$. (elastic#25036)
  Change `has_child`, `has_parent` queries and `childen` aggregation to work with the new join field type and at the same time maintaining support for the `_parent` meta field type.
  Remove comma-separated feature parsing for GetIndicesAction
  ...
jasontedor added a commit to zeha/elasticsearch that referenced this pull request Jun 6, 2017
@jasontedor
Merge branch 'master' into honor-systemd-sysctl-masking
5e2de34 
* master: (619 commits)
  Add support for clear scroll to high level REST client (elastic#25038)
  Tiny correction in inner-hits.asciidoc (elastic#25066)
  Added release notes for 6.0.0-alpha2
  Expand index expressions against indices only when managing aliases (elastic#23997)
  Collapse inner hits rest test should not skip 5.x
  Settings: Fix secure settings by prefix (elastic#25064)
  add `exclude_keys` option to KeyValueProcessor (elastic#24876)
  Test: update missing body tests to run against versions >= 5.5.0
  Track EWMA[1] of task execution time in search threadpool executor
  Removes an invalid assert in resizing big arrays which does not always hold (resizing can result in a smaller size than the current size, while the assert attempted to verify the new size is always greater than the current).
  Fixed NPEs caused by requests without content. (elastic#23497)
  Plugins can register pre-configured char filters (elastic#25000)
  Build: Allow preserving shared dir (elastic#24962)
  Tests: Make secure settings available from settings builder for tests (elastic#25037)
  [TEST] Skip wildcard expansion test due to breaking change
  Test that gradle and Java version types match (elastic#24943)
  Include duplicate jar when jarhell check fails
  Change ScriptContexts to use needs instead of uses$. (elastic#25036)
  Change `has_child`, `has_parent` queries and `childen` aggregation to work with the new join field type and at the same time maintaining support for the `_parent` meta field type.
  Remove comma-separated feature parsing for GetIndicesAction
  ...
@nik9000 nik9000 deleted the test_gradle_and_java_versions_logic branch June 7, 2017 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rjernst rjernst rjernst approved these changes

Assignees

No one assigned

Labels

>test Issues or PRs that are addressing/adding tests v6.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@nik9000 @rjernst @clintongormley @colings86