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

Fix failing ICU tests #35207

Merged
merged 5 commits into from
Nov 6, 2018
Merged

Fix failing ICU tests #35207

merged 5 commits into from
Nov 6, 2018

Conversation

romseygeek
Copy link
Contributor

Fixes #35173

@romseygeek romseygeek added :Search Relevance/Analysis How text is split into tokens >test-failure Triaged test failures from CI v7.0.0 labels Nov 2, 2018
@romseygeek romseygeek self-assigned this Nov 2, 2018
@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.

@romseygeek took a quick look, can you quickly comment on why the yaml changes are introduced? I saw the checkstyle stuff but wonder if the yaml update is fixing sth. or actually just changing the test to be more specific (from keyword -> standard tokenizer)

@romseygeek
Copy link
Contributor Author

The yaml tests were failing with 'keyword tokenizer not found', I guess due to dependency issues. The old tests were using standard tokenizer, so it seemed to make sense to use that here as well.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -38,8 +38,10 @@
* <p>The {@code unicodeSetFilter} attribute can be used to provide the UniCodeSet for filtering.</p>
*/
public class IcuNormalizerTokenFilterFactory extends AbstractTokenFilterFactory implements MultiTermAwareComponent {
private static final DeprecationLogger deprecationLogger =

private static final DeprecationLogger DEPRECATION_LOGGER =
Copy link
Member

Choose a reason for hiding this comment

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

We've been using deprecationLogger because the logger does things rather than is a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have changed it back

@romseygeek romseygeek merged commit 9f4b93f into elastic:master Nov 6, 2018
@romseygeek romseygeek deleted the icu-fixes branch November 6, 2018 09:02
matarrese added a commit to matarrese/elasticsearch that referenced this pull request Nov 6, 2018
…-agg

* master: (528 commits)
  Register Azure max_retries setting (elastic#35286)
  add version 6.4.4
  [Docs] Add painless context details for bucket_script (elastic#35142)
  Upgrade jline to 3.8.2 (elastic#35288)
  SQL: new SQL CLI logo (elastic#35261)
  Logger: Merge ESLoggerFactory into Loggers (elastic#35146)
  Docs: Add section about range query for range type (elastic#35222)
  [ILM] change remove-policy-from-index http method from DELETE to POST (elastic#35268)
  [CCR] Forgot missing return statement,
  SQL: Fix null handling for AND and OR in SELECT (elastic#35277)
  [TEST] Mute ChangePolicyForIndexIT#testChangePolicyForIndex
  Serialize ignore_throttled also to 6.6 after backport
  Check for java 11 in buildSrc (elastic#35260)
  [TEST] increase await timeout in RemoteClusterConnectionTests
  Add missing up-to-date configuration (elastic#35255)
  Adapt Lucene BWC version
  SQL: Introduce Coalesce function (elastic#35253)
  Upgrade to lucene-8.0.0-snapshot-31d7dfe6b1 (elastic#35224)
  Fix failing ICU tests (elastic#35207)
  Prevent throttled indices to be searched through wildcards by default (elastic#34354)
  ...
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Search Relevance/Analysis How text is split into tokens >test-failure Triaged test failures from CI v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants