Remove easy uses of setAccessible in tests.

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.
This commit is contained in:
Robert Muir 2015-09-12 11:57:30 -04:00
parent e1efd5f6ba
commit a7418cd9bb
9 changed files with 68 additions and 96 deletions

View File

@ -293,6 +293,8 @@
<include>org/elasticsearch/common/util/MockBigArrays$*.class</include> <include>org/elasticsearch/common/util/MockBigArrays$*.class</include>
<include>org/elasticsearch/node/NodeMocksPlugin.class</include> <include>org/elasticsearch/node/NodeMocksPlugin.class</include>
<include>org/elasticsearch/node/MockNode.class</include> <include>org/elasticsearch/node/MockNode.class</include>
<include>org/elasticsearch/node/MockNode.class</include>
<include>org/elasticsearch/common/io/PathUtilsForTesting.class</include>
</includes> </includes>
<excludes> <excludes>
<!-- unit tests for yaml suite parser & rest spec parser need to be excluded --> <!-- unit tests for yaml suite parser & rest spec parser need to be excluded -->

View File

@ -43,8 +43,8 @@ public final class PathUtils {
/** the actual JDK default */ /** the actual JDK default */
static final FileSystem ACTUAL_DEFAULT = FileSystems.getDefault(); static final FileSystem ACTUAL_DEFAULT = FileSystems.getDefault();
/** can be changed by tests (via reflection) */ /** can be changed by tests */
private static volatile FileSystem DEFAULT = ACTUAL_DEFAULT; static volatile FileSystem DEFAULT = ACTUAL_DEFAULT;
/** /**
* Returns a {@code Path} from name components. * Returns a {@code Path} from name components.

View File

@ -147,6 +147,13 @@ public class MultiMatchQueryBuilder extends QueryBuilder implements BoostableQue
} }
} }
/**
* Returns the type (for testing)
*/
public MultiMatchQueryBuilder.Type getType() {
return type;
}
/** /**
* Constructs a new text query. * Constructs a new text query.
*/ */

View File

@ -0,0 +1,41 @@
/*
* Licensed to Elasticsearch under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package org.elasticsearch.common.io;
import org.apache.lucene.util.LuceneTestCase;
import java.nio.file.FileSystem;
/**
* Exposes some package private stuff in PathUtils for framework purposes only!
*/
public class PathUtilsForTesting {
/** Sets a new default filesystem for testing */
public static void setup() {
FileSystem mock = LuceneTestCase.getBaseTempDirForTestClass().getFileSystem();
PathUtils.DEFAULT = mock;
}
/** Resets filesystem back to the real system default */
public static void teardown() {
PathUtils.DEFAULT = PathUtils.ACTUAL_DEFAULT;
}
}

View File

@ -20,6 +20,7 @@
package org.elasticsearch.indices.analysis; package org.elasticsearch.indices.analysis;
import org.apache.lucene.analysis.Analyzer; import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.TokenStream;
import org.elasticsearch.Version; import org.elasticsearch.Version;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentBuilder;
@ -28,7 +29,7 @@ import org.elasticsearch.test.ESBackcompatTestCase;
import org.elasticsearch.test.ESIntegTestCase; import org.elasticsearch.test.ESIntegTestCase;
import org.junit.Test; import org.junit.Test;
import java.lang.reflect.Field; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.HashMap; import java.util.HashMap;
@ -161,44 +162,18 @@ public class PreBuiltAnalyzerIntegrationIT extends ESIntegTestCase {
} }
} }
// the close() method of a lucene analyzer sets the storedValue field to null // ensure analyzers are still open by checking there is no ACE
// we simply check this via reflection - ugly but works private void assertLuceneAnalyzersAreNotClosed(Map<PreBuiltAnalyzers, List<Version>> loadedAnalyzers) throws IOException {
private void assertLuceneAnalyzersAreNotClosed(Map<PreBuiltAnalyzers, List<Version>> loadedAnalyzers) throws IllegalAccessException, NoSuchFieldException {
for (Map.Entry<PreBuiltAnalyzers, List<Version>> preBuiltAnalyzerEntry : loadedAnalyzers.entrySet()) { for (Map.Entry<PreBuiltAnalyzers, List<Version>> preBuiltAnalyzerEntry : loadedAnalyzers.entrySet()) {
PreBuiltAnalyzers preBuiltAnalyzer = preBuiltAnalyzerEntry.getKey();
for (Version version : preBuiltAnalyzerEntry.getValue()) { for (Version version : preBuiltAnalyzerEntry.getValue()) {
Analyzer analyzer = preBuiltAnalyzerEntry.getKey().getCache().get(version); Analyzer analyzer = preBuiltAnalyzerEntry.getKey().getCache().get(version);
try (TokenStream stream = analyzer.tokenStream("foo", "bar")) {
Field field = getFieldFromClass("storedValue", analyzer); stream.reset();
boolean currentAccessible = field.isAccessible(); while (stream.incrementToken()) {
field.setAccessible(true); }
Object storedValue = field.get(analyzer); stream.end();
field.setAccessible(currentAccessible);
assertThat(String.format(Locale.ROOT, "Analyzer %s in version %s seems to be closed", preBuiltAnalyzer.name(), version), storedValue, is(notNullValue()));
} }
} }
} }
/**
* Searches for a field until it finds, loops through all superclasses
*/
private Field getFieldFromClass(String fieldName, Object obj) {
Field field = null;
boolean storedValueFieldFound = false;
Class clazz = obj.getClass();
while (!storedValueFieldFound) {
try {
field = clazz.getDeclaredField(fieldName);
storedValueFieldFound = true;
} catch (NoSuchFieldException e) {
clazz = clazz.getSuperclass();
} }
if (Object.class.equals(clazz)) throw new RuntimeException("Could not find storedValue field in class" + clazz);
}
return field;
}
} }

View File

@ -629,8 +629,6 @@ public class MultiMatchQueryIT extends ESIntegTestCase {
} }
private MultiMatchQueryBuilder.Type getType(MultiMatchQueryBuilder builder) throws NoSuchFieldException, IllegalAccessException { private MultiMatchQueryBuilder.Type getType(MultiMatchQueryBuilder builder) throws NoSuchFieldException, IllegalAccessException {
Field field = MultiMatchQueryBuilder.class.getDeclaredField("type"); return builder.getType();
field.setAccessible(true);
return (MultiMatchQueryBuilder.Type) field.get(builder);
} }
} }

View File

@ -29,6 +29,7 @@ import com.carrotsearch.randomizedtesting.generators.RandomInts;
import com.carrotsearch.randomizedtesting.generators.RandomPicks; import com.carrotsearch.randomizedtesting.generators.RandomPicks;
import com.carrotsearch.randomizedtesting.generators.RandomStrings; import com.carrotsearch.randomizedtesting.generators.RandomStrings;
import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter; import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter;
import org.apache.lucene.uninverting.UninvertingReader; import org.apache.lucene.uninverting.UninvertingReader;
import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase;
import org.apache.lucene.util.LuceneTestCase.SuppressCodecs; import org.apache.lucene.util.LuceneTestCase.SuppressCodecs;
@ -41,6 +42,7 @@ import org.elasticsearch.client.Requests;
import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.cluster.routing.DjbHashFunction; import org.elasticsearch.cluster.routing.DjbHashFunction;
import org.elasticsearch.common.io.PathUtils; import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.io.PathUtilsForTesting;
import org.elasticsearch.common.logging.ESLogger; import org.elasticsearch.common.logging.ESLogger;
import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.settings.Settings;
@ -62,9 +64,7 @@ import org.junit.Rule;
import org.junit.rules.RuleChain; import org.junit.rules.RuleChain;
import java.io.IOException; import java.io.IOException;
import java.lang.reflect.Field;
import java.nio.file.DirectoryStream; import java.nio.file.DirectoryStream;
import java.nio.file.FileSystem;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.*; import java.util.*;
@ -137,20 +137,12 @@ public abstract class ESTestCase extends LuceneTestCase {
@BeforeClass @BeforeClass
public static void setFileSystem() throws Exception { public static void setFileSystem() throws Exception {
Field field = PathUtils.class.getDeclaredField("DEFAULT"); PathUtilsForTesting.setup();
field.setAccessible(true);
FileSystem mock = LuceneTestCase.getBaseTempDirForTestClass().getFileSystem();
field.set(null, mock);
assertEquals(mock, PathUtils.getDefaultFileSystem());
} }
@AfterClass @AfterClass
public static void restoreFileSystem() throws Exception { public static void restoreFileSystem() throws Exception {
Field field1 = PathUtils.class.getDeclaredField("ACTUAL_DEFAULT"); PathUtilsForTesting.teardown();
field1.setAccessible(true);
Field field2 = PathUtils.class.getDeclaredField("DEFAULT");
field2.setAccessible(true);
field2.set(null, field1.get(null));
} }
// setup a default exception handler which knows when and how to print a stacktrace // setup a default exception handler which knows when and how to print a stacktrace

View File

@ -688,7 +688,7 @@ public class ElasticsearchAssertions {
ElasticsearchAssertions.assertVersionSerializable(version, new ThrowableWrapper(t)); ElasticsearchAssertions.assertVersionSerializable(version, new ThrowableWrapper(t));
} }
private static final class ThrowableWrapper implements Streamable { public static final class ThrowableWrapper implements Streamable {
Throwable throwable; Throwable throwable;
public ThrowableWrapper(Throwable t) { public ThrowableWrapper(Throwable t) {
throwable = t; throwable = t;
@ -716,7 +716,6 @@ public class ElasticsearchAssertions {
Class<? extends Streamable> clazz = streamable.getClass(); Class<? extends Streamable> clazz = streamable.getClass();
Constructor<? extends Streamable> constructor = clazz.getDeclaredConstructor(); Constructor<? extends Streamable> constructor = clazz.getDeclaredConstructor();
assertThat(constructor, Matchers.notNullValue()); assertThat(constructor, Matchers.notNullValue());
constructor.setAccessible(true);
Streamable newInstance = constructor.newInstance(); Streamable newInstance = constructor.newInstance();
return newInstance; return newInstance;
} catch (Throwable e) { } catch (Throwable e) {

View File

@ -217,52 +217,10 @@ public class MockFSDirectoryService extends FsDirectoryService {
public static final class ElasticsearchMockDirectoryWrapper extends MockDirectoryWrapper { public static final class ElasticsearchMockDirectoryWrapper extends MockDirectoryWrapper {
private final boolean crash; private final boolean crash;
private final Set<String> superUnSyncedFiles;
private final Random superRandomState;
public ElasticsearchMockDirectoryWrapper(Random random, Directory delegate, boolean crash) { public ElasticsearchMockDirectoryWrapper(Random random, Directory delegate, boolean crash) {
super(random, delegate); super(random, delegate);
this.crash = crash; this.crash = crash;
// TODO: remove all this and cutover to MockFS (DisableFsyncFS) instead
try {
Field field = MockDirectoryWrapper.class.getDeclaredField("unSyncedFiles");
field.setAccessible(true);
superUnSyncedFiles = (Set<String>) field.get(this);
field = MockDirectoryWrapper.class.getDeclaredField("randomState");
field.setAccessible(true);
superRandomState = (Random) field.get(this);
} catch (ReflectiveOperationException roe) {
throw new RuntimeException(roe);
}
}
/**
* Returns true if {@link #in} must sync its files.
* Currently, only {@link org.apache.lucene.store.NRTCachingDirectory} requires sync'ing its files
* because otherwise they are cached in an internal {@link org.apache.lucene.store.RAMDirectory}. If
* other directories require that too, they should be added to this method.
*/
private boolean mustSync() {
Directory delegate = in;
while (delegate instanceof FilterDirectory) {
if (delegate instanceof NRTCachingDirectory) {
return true;
}
delegate = ((FilterDirectory) delegate).getDelegate();
}
return delegate instanceof NRTCachingDirectory;
}
@Override
public synchronized void sync(Collection<String> names) throws IOException {
// don't wear out our hardware so much in tests.
if (superRandomState.nextInt(100) == 0 || mustSync()) {
super.sync(names);
} else {
superUnSyncedFiles.removeAll(names);
}
} }
@Override @Override