Skip to content

Conversation

@uschindler
Copy link
Contributor

@uschindler uschindler commented Apr 29, 2017

Now that Java 9 is stable and method signatures should no longer change, we should add tests to check for existence and correct detection of the special Java 9 optimizations in painless:

  • Indified String concat: This adds a test that only runs on java 9 to check the generated bytecode to have INVOKEDYNAMIC.
  • MethodHandles#arrayLength() should be used. This makes the static constant in Def pkg private and adds a test for non-nullness on Java 9

This PR should not to test Java 9 compatibility at runtime (we always fallback to Java 8 behaviour if something breaks), it is intended to break the build instead if we are on Java 9 and painless does not use the better features. This may happen if somebody breaks the reflective lookups.

… Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper()
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@uschindler uschindler changed the title Painless: Add tests for Java 9 features painless: Add tests for Java 9 features Apr 29, 2017
@jdconrad
Copy link
Contributor

jdconrad commented May 1, 2017

@elasticmachine please test this

@jdconrad jdconrad self-assigned this May 1, 2017
@jdconrad jdconrad added :Plugin Lang Painless v6.0.0-alpha1 v5.5.0 >test Issues or PRs that are addressing/adding tests labels May 1, 2017
@jdconrad
Copy link
Contributor

jdconrad commented May 2, 2017

Build failure looks unrelated. I'll test this locally.

@uschindler
Copy link
Contributor Author

Seems to be fine now!

@jdconrad jdconrad merged commit 62fa708 into elastic:master May 2, 2017
jdconrad pushed a commit that referenced this pull request May 2, 2017
…he special Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper() (#24405)
@jdconrad
Copy link
Contributor

jdconrad commented May 2, 2017

@uschindler Thanks for adding these tests! I committed this to master and 5.5.

@uschindler uschindler deleted the test/checkJava9Bytecodes branch May 2, 2017 16:25
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request May 2, 2017
* master: (27 commits)
  Check index sorting with no replica since we cannot ensure that the replica index is ready when forceMerge is called. Closes elastic#24416
  Docs: correct indentation on callout
  Build that java api docs from a test (elastic#24354)
  Move RemoteClusterService into TransportService (elastic#24424)
  Fix license header in WildflyIT.java
  Try not to lose stacktraces (elastic#24426)
  [DOCS] Update XPack Reference URL for 5.4 (elastic#24425)
  Painless: Add tests to check for existence and correct detection of the special Java 9 optimizations: Indified String concat and MethodHandles#ArrayLengthHelper() (elastic#24405)
  Extract a common base class to allow services to listen to remote cluster config updates (elastic#24367)
  Adds check to snapshot repository incompatible-snapshots blob to delete a pre-existing one before attempting to overwrite it.
  Added docs for batched_reduce_size
  Fixes checkstyle errors
  Allow scripted metric agg to access `_score` (elastic#24295)
  [Test] Add unit tests for HDR/TDigest PercentilesAggregators (elastic#24245)
  Fix FieldCaps documentation
  Upgrade to JUnit 4.12 (elastic#23877)
  Set available processors for Netty
  Painless: Fix method references to ctor with the new LambdaBootstrap and cleanup code (elastic#24406)
  Doc test: use propery regex for file size
  [DOCS] Tweak doc test to sync_flush
  ...
@clintongormley clintongormley added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed :Plugin Lang Painless labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >test Issues or PRs that are addressing/adding tests v5.5.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants