Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEST] Fix failure due to exception message in java11 #32321

Merged
merged 2 commits into from
Jul 25, 2018

Conversation

polyfractal
Copy link
Contributor

@cbuescher looks like CI turned up another variation of #32036, this time in rollup tests :)

Changed the test to be less restrictive and only look for the classes we care about, so that we don't fail on java 11 with more verbose exception messages.

Java 11 uses more verbose exceptions messages, causing this assertion
to fail.  Changed the test to be less restrictive and only look
for the classes we care about.
@polyfractal polyfractal added >test Issues or PRs that are addressing/adding tests review :StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data labels Jul 24, 2018
@polyfractal polyfractal requested a review from cbuescher July 24, 2018 11:34
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search-aggs

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

Thanks, I left a small request.

@@ -510,8 +510,10 @@ public void testMismatch() throws IOException {
InternalAggregation.ReduceContext reduceContext = new InternalAggregation.ReduceContext(bigArrays, scriptService, true);
Exception e = expectThrows(RuntimeException.class,
Copy link
Member

Choose a reason for hiding this comment

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

Can this be more specific so that we at least check the expected exception type? By checking the exception message I think we at least used to expect a ClassCastException or something alike. Would be great if we can to that here now that the assertion on the exact message text needs to be relaxed a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, good catch. Should have been like that before anyway.

@cbuescher cbuescher self-assigned this Jul 24, 2018
Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM

@polyfractal polyfractal merged commit 6cf7588 into elastic:master Jul 25, 2018
@polyfractal polyfractal added v7.0.0 and removed review labels Jul 25, 2018
dnhatn added a commit that referenced this pull request Jul 26, 2018
* master:
  [DOCS] Fix formatting error in Slack action
  Painless: Fix documentation links to use existing refs (#32335)
  Painless: Decouple PainlessLookupBuilder and Whitelists (#32346)
  [DOCS] Adds recommendation for xpack.security.enabled (#32345)
  [TEST] Mute ConvertProcessortTests.testConvertIntHexError
  [TEST] Fix failure due to exception message in java11 (#32321)
  [DOCS] Fixes typo in ML aggregations page
  [DOCS] Adds link from bucket_span property to common time units
  [ML][DOCS] Add documentation for detector rules and filters (#32013)
  Add opaque_id to index audit logging (#32260)
  Add 6.5.0 version to master
  fixes broken build for third-party-tests (#32353)
jimczi pushed a commit that referenced this pull request Oct 4, 2018
Java 11 uses more verbose exceptions messages, causing this assertion
to fail.  Changed the test to be less restrictive and only look
for the classes we care about.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:StorageEngine/Rollup Turn fine-grained time-based data into coarser-grained data >test Issues or PRs that are addressing/adding tests v6.5.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants