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

Remove easy uses of setAccessible in tests. #13537

Merged
merged 2 commits into from Sep 12, 2015

Conversation

rmuir
Copy link
Contributor

@rmuir rmuir commented Sep 12, 2015

Don't worry, I will fix the rest. But some of those remaining will need a lucene upgrade,
we need to add a getter or two for tests to do things cleanly.

Don't worry, I will fix the rest. But some of those remaining will need a lucene upgrade,
we need to add a getter or two for tests to do things cleanly.
@rmuir rmuir added >bug >test Issues or PRs that are addressing/adding tests v5.0.0-alpha1 labels Sep 12, 2015
@@ -293,6 +293,8 @@
<include>org/elasticsearch/common/util/MockBigArrays$*.class</include>
<include>org/elasticsearch/node/NodeMocksPlugin.class</include>
<include>org/elasticsearch/node/MockNode.class</include>
<include>org/elasticsearch/node/MockNode.class</include>
Copy link
Member

Choose a reason for hiding this comment

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

My is mock node duplicated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I committed a fix for this

// the close() method of a lucene analyzer sets the storedValue field to null
// we simply check this via reflection - ugly but works
private void assertLuceneAnalyzersAreNotClosed(Map<PreBuiltAnalyzers, List<Version>> loadedAnalyzers) throws IllegalAccessException, NoSuchFieldException {
// ensure analyzers are still open by checking there is no ACE
Copy link
Member

Choose a reason for hiding this comment

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

nice simplification

@rjernst
Copy link
Member

rjernst commented Sep 12, 2015

LGTM

rmuir added a commit that referenced this pull request Sep 12, 2015
Remove easy uses of setAccessible in tests.
@rmuir rmuir merged commit 3a0dd59 into elastic:master Sep 12, 2015
@rmuir rmuir added the v2.2.0 label Sep 30, 2015
@rmuir
Copy link
Contributor Author

rmuir commented Sep 30, 2015

I just backported this to 2.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug >test Issues or PRs that are addressing/adding tests v2.2.0 v5.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants