Skip to content

Commit

Permalink
Test: Fix forbidden uses in test framework (#32824)
Browse files Browse the repository at this point in the history
This commit fixes existing uses of forbidden apis in the test framework
and re-enables the forbidden apis check. It was previously completely
disabled and had missed a rename of the forbidden apis signatures files.

closes #32772
  • Loading branch information
rjernst committed Aug 14, 2018
1 parent ff9c5dd commit 2077135
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 25 deletions.
9 changes: 0 additions & 9 deletions test/build.gradle
Expand Up @@ -28,19 +28,10 @@ subprojects {
apply plugin: 'nebula.maven-base-publish'
apply plugin: 'nebula.maven-scm'


// the main files are actually test files, so use the appropriate forbidden api sigs
forbiddenApisMain {
signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
PrecommitTasks.getResource('/forbidden/es-signatures.txt'),
PrecommitTasks.getResource('/forbidden/es-test-signatures.txt')]
}

// TODO: should we have licenses for our test deps?
dependencyLicenses.enabled = false
dependenciesInfo.enabled = false

// TODO: why is the test framework pulled in...
forbiddenApisMain.enabled = false
jarHell.enabled = false
}
5 changes: 5 additions & 0 deletions test/fixtures/build.gradle
@@ -0,0 +1,5 @@

subprojects {
// fixtures are mostly external and by default we don't want to check forbidden apis
forbiddenApisMain.enabled = false
}
5 changes: 3 additions & 2 deletions test/framework/build.gradle
Expand Up @@ -42,8 +42,9 @@ compileTestJava.options.compilerArgs << '-Xlint:-rawtypes'

// the main files are actually test files, so use the appropriate forbidden api sigs
forbiddenApisMain {
signaturesURLs = [PrecommitTasks.getResource('/forbidden/all-signatures.txt'),
PrecommitTasks.getResource('/forbidden/test-signatures.txt')]
signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt'),
PrecommitTasks.getResource('/forbidden/es-all-signatures.txt'),
PrecommitTasks.getResource('/forbidden/es-test-signatures.txt')]
}

// TODO: should we have licenses for our test deps?
Expand Down
Expand Up @@ -67,7 +67,7 @@ private static String toCamelCase(String s) {
Matcher m = UNDERSCORE_THEN_ANYTHING.matcher(s);
StringBuffer sb = new StringBuffer();
while (m.find()) {
m.appendReplacement(sb, m.group(1).toUpperCase());
m.appendReplacement(sb, m.group(1).toUpperCase(Locale.ROOT));
}
m.appendTail(sb);
sb.setCharAt(0, Character.toUpperCase(sb.charAt(0)));
Expand Down
Expand Up @@ -21,7 +21,6 @@

import com.carrotsearch.randomizedtesting.RandomizedTest;
import com.carrotsearch.randomizedtesting.SeedUtils;

import org.apache.lucene.index.IndexReader;
import org.apache.lucene.util.Accountable;
import org.elasticsearch.Version;
Expand Down
Expand Up @@ -53,6 +53,7 @@
import org.elasticsearch.cluster.metadata.IndexMetaData;
import org.elasticsearch.common.CheckedBiFunction;
import org.elasticsearch.common.CheckedRunnable;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.PathUtils;
import org.elasticsearch.common.io.PathUtilsForTesting;
Expand Down Expand Up @@ -197,13 +198,9 @@ public static void resetPortCounter() {
}

static {
System.setProperty("log4j.shutdownHookEnabled", "false");
System.setProperty("log4j2.disable.jmx", "true");

setTestSysProps();
LogConfigurator.loadLog4jPlugins();

// Enable Netty leak detection and monitor logger for logged leak errors
System.setProperty("io.netty.leakDetection.level", "paranoid");
String leakLoggerName = "io.netty.util.ResourceLeakDetector";
Logger leakLogger = LogManager.getLogger(leakLoggerName);
Appender leakAppender = new AbstractAppender(leakLoggerName, null,
Expand Down Expand Up @@ -242,6 +239,14 @@ public void append(LogEvent event) {
Collections.sort(javaZoneIds);
JAVA_ZONE_IDS = Collections.unmodifiableList(javaZoneIds);
}
@SuppressForbidden(reason = "force log4j and netty sysprops")
private static void setTestSysProps() {
System.setProperty("log4j.shutdownHookEnabled", "false");
System.setProperty("log4j2.disable.jmx", "true");

// Enable Netty leak detection and monitor logger for logged leak errors
System.setProperty("io.netty.leakDetection.level", "paranoid");
}

protected final Logger logger = Loggers.getLogger(getClass());
protected final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
Expand Down
Expand Up @@ -20,6 +20,8 @@
package org.elasticsearch.test.fixture;

import com.sun.net.httpserver.HttpServer;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.PathUtils;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
Expand All @@ -32,7 +34,6 @@
import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.nio.file.StandardCopyOption;
import java.util.HashMap;
import java.util.List;
Expand All @@ -48,6 +49,7 @@
/**
* Base class for test fixtures that requires a {@link HttpServer} to work.
*/
@SuppressForbidden(reason = "uses httpserver by design")
public abstract class AbstractHttpFixture {

protected static final Map<String, String> TEXT_PLAIN_CONTENT_TYPE = contentType("text/plain; charset=utf-8");
Expand All @@ -62,7 +64,7 @@ public abstract class AbstractHttpFixture {
private final Path workingDirectory;

protected AbstractHttpFixture(final String workingDir) {
this.workingDirectory = Paths.get(Objects.requireNonNull(workingDir));
this.workingDirectory = PathUtils.get(Objects.requireNonNull(workingDir));
}

/**
Expand Down
Expand Up @@ -22,6 +22,7 @@
import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.Constants;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.test.ESIntegTestCase;
import org.elasticsearch.test.ESTestCase;
Expand Down Expand Up @@ -86,7 +87,12 @@ public void testFailure(Failure failure) throws Exception {
gradleMessageBuilder.appendClientYamlSuiteProperties();
}

System.err.println(b.toString());
printToErr(b.toString());
}

@SuppressForbidden(reason = "printing repro info")
private static void printToErr(String s) {
System.err.println(s);
}

protected static class GradleMessageBuilder extends ReproduceErrorMessageBuilder {
Expand Down
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.action.ActionListenerResponseHandler;
import org.elasticsearch.action.support.PlainActionFuture;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.common.SuppressForbidden;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
Expand Down Expand Up @@ -62,6 +63,7 @@
import java.net.InetSocketAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.UnknownHostException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -1895,7 +1897,7 @@ public void testTimeoutPerConnection() throws IOException {
// means that once we received an ACK from the client we just drop the packet on the floor (which is what we want) and we run
// into a connection timeout quickly. Yet other implementations can for instance can terminate the connection within the 3 way
// handshake which I haven't tested yet.
socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
socket.bind(getLocalEphemeral(), 1);
socket.setReuseAddress(true);
DiscoveryNode first = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
socket.getLocalPort()), emptyMap(),
Expand Down Expand Up @@ -2018,7 +2020,7 @@ protected String handleRequest(TcpChannel mockChannel, String profileName, Strea

public void testTcpHandshakeTimeout() throws IOException {
try (ServerSocket socket = new MockServerSocket()) {
socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
socket.bind(getLocalEphemeral(), 1);
socket.setReuseAddress(true);
DiscoveryNode dummy = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
socket.getLocalPort()), emptyMap(),
Expand All @@ -2039,7 +2041,7 @@ public void testTcpHandshakeTimeout() throws IOException {

public void testTcpHandshakeConnectionReset() throws IOException, InterruptedException {
try (ServerSocket socket = new MockServerSocket()) {
socket.bind(new InetSocketAddress(InetAddress.getLocalHost(), 0), 1);
socket.bind(getLocalEphemeral(), 1);
socket.setReuseAddress(true);
DiscoveryNode dummy = new DiscoveryNode("TEST", new TransportAddress(socket.getInetAddress(),
socket.getLocalPort()), emptyMap(),
Expand Down Expand Up @@ -2669,4 +2671,8 @@ public void onConnectionOpened(final Transport.Connection connection) {

protected abstract void closeConnectionChannel(Transport transport, Transport.Connection connection) throws IOException;

@SuppressForbidden(reason = "need local ephemeral port")
private InetSocketAddress getLocalEphemeral() throws UnknownHostException {
return new InetSocketAddress(InetAddress.getLocalHost(), 0);
}
}
Expand Up @@ -18,6 +18,7 @@
*/
package org.elasticsearch.transport;

import org.elasticsearch.cli.SuppressForbidden;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionListener;
Expand Down Expand Up @@ -171,6 +172,7 @@ private void readMessage(MockChannel mockChannel, StreamInput input) throws IOEx
}

@Override
@SuppressForbidden(reason = "real socket for mocking remote connections")
protected MockChannel initiateChannel(DiscoveryNode node, TimeValue connectTimeout, ActionListener<Void> connectListener)
throws IOException {
InetSocketAddress address = node.getAddress().address();
Expand Down
Expand Up @@ -20,7 +20,7 @@
package org.elasticsearch.transport.nio;

import org.apache.logging.log4j.Logger;
import org.apache.lucene.util.IOUtils;
import org.elasticsearch.core.internal.io.IOUtils;
import org.elasticsearch.transport.nio.channel.ChannelFactory;
import org.elasticsearch.transport.nio.channel.NioServerSocketChannel;
import org.elasticsearch.transport.nio.channel.NioSocketChannel;
Expand Down

0 comments on commit 2077135

Please sign in to comment.