Skip to content

Forbid test sources to use System.out.println and Throwable.printStackTrace #17112

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Mar 15, 2016
Merged

Forbid test sources to use System.out.println and Throwable.printStackTrace #17112

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d14ae5f
Remove Python and Javascript Benchmark classes
ywelsch Mar 15, 2016
e91fd09
Enable jdk-system-out Forbidden API checks on test sources
ywelsch Mar 15, 2016
f5e6db4
Remove System.out.println and Throwable.printStackTrace from tests
ywelsch Mar 15, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions buildSrc/src/main/groovy/org/elasticsearch/gradle/precommit/PrecommitTasks.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,14 @@ class PrecommitTasks {
project.forbiddenApis {
internalRuntimeForbidden = true
failOnUnsupportedJava = false
bundledSignatures = ['jdk-unsafe', 'jdk-deprecated']
bundledSignatures = ['jdk-unsafe', 'jdk-deprecated', 'jdk-system-out']
signaturesURLs = [getClass().getResource('/forbidden/jdk-signatures.txt'),
getClass().getResource('/forbidden/es-all-signatures.txt')]
suppressAnnotations = ['**.SuppressForbidden']
}
Task mainForbidden = project.tasks.findByName('forbiddenApisMain')
if (mainForbidden != null) {
mainForbidden.configure {
bundledSignatures += 'jdk-system-out'
signaturesURLs += getClass().getResource('/forbidden/es-core-signatures.txt')
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/common/xcontent/builder/XContentBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ public void testWritingBinaryToStream() throws Exception {

byte[] data = bos.bytes().toBytes();
String sData = new String(data, "UTF8");
System.out.println("DATA: " + sData);
assertThat(sData, equalTo("{\"name\":\"something\", source : { test : \"value\" },\"name2\":\"something2\"}"));
}

public void testFieldCaseConversion() throws Exception {
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/http/netty/NettyHttpServerPipeliningTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ public void messageReceived(final ChannelHandlerContext ctx, final MessageEvent

@Override
public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent e) {
e.getCause().printStackTrace();
logger.info("Caught exception", e.getCause());
e.getChannel().close();
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/index/translog/TranslogTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1491,7 +1491,7 @@ protected void afterAdd() throws IOException {
if (writtenOperations.size() != snapshot.totalOperations()) {
for (int i = 0; i < threadCount; i++) {
if (threadExceptions[i] != null) {
threadExceptions[i].printStackTrace();
logger.info("Translog exception", threadExceptions[i]);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/indices/IndexingMemoryControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ public void testThrottling() throws Exception {
controller.assertNotThrottled(shard0);
controller.assertThrottled(shard1);

System.out.println("TEST: now index more");
logger.info("--> Indexing more data");

// More indexing to shard0
controller.simulateIndexing(shard0);
Expand Down
5 changes: 2 additions & 3 deletions core/src/test/java/org/elasticsearch/percolator/ConcurrentPercolatorIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
*
*/
public class ConcurrentPercolatorIT extends ESIntegTestCase {
public void testSimpleConcurrentPercolator() throws Exception {
public void testSimpleConcurrentPercolator() throws Throwable {
// We need to index a document / define mapping, otherwise field1 doesn't get recognized as number field.
// If we don't do this, then 'test2' percolate query gets parsed as a TermQuery and not a RangeQuery.
// The percolate api doesn't parse the doc if no queries have registered, so it can't lazily create a mapping
Expand Down Expand Up @@ -143,9 +143,8 @@ public void run() {

Throwable assertionError = exceptionHolder.get();
if (assertionError != null) {
assertionError.printStackTrace();
throw assertionError;
}
assertThat(assertionError + " should be null", assertionError, nullValue());
}

public void testConcurrentAddingAndPercolating() throws Exception {
Expand Down
2 changes: 1 addition & 1 deletion .../src/test/java/org/elasticsearch/search/aggregations/BasePipelineAggregationTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public void testFromXContent() throws IOException {
AF testAgg = createTestAggregatorFactory();
AggregatorFactories.Builder factoriesBuilder = AggregatorFactories.builder().skipResolveOrder().addPipelineAggregator(testAgg);
String contentString = factoriesBuilder.toString();
System.out.println(contentString);
logger.info("Content string: {}", contentString);
XContentParser parser = XContentFactory.xContent(contentString).createParser(contentString);
QueryParseContext parseContext = new QueryParseContext(queriesRegistry);
parseContext.reset(parser);
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/org/elasticsearch/search/fetch/FetchSubPhasePluginIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.elasticsearch.action.termvectors.TermVectorsRequest;
import org.elasticsearch.action.termvectors.TermVectorsResponse;
import org.elasticsearch.common.Priority;
import org.elasticsearch.common.logging.ESLoggerFactory;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.index.termvectors.TermVectorsService;
Expand Down Expand Up @@ -173,7 +174,7 @@ public void hitExecute(SearchContext context, HitContext hitContext) {
}
hitField.values().add(tv);
} catch (IOException e) {
e.printStackTrace();
ESLoggerFactory.getLogger(FetchSubPhasePluginIT.class.getName()).info("Swallowed exception", e);
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion core/src/test/java/org/elasticsearch/search/geo/GeoFilterIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

package org.elasticsearch.search.geo;

import org.elasticsearch.common.logging.ESLoggerFactory;
import org.locationtech.spatial4j.context.SpatialContext;
import org.locationtech.spatial4j.distance.DistanceUtils;
import org.locationtech.spatial4j.exception.InvalidShapeException;
Expand Down Expand Up @@ -560,7 +561,7 @@ protected static boolean testRelationSupport(SpatialOperation relation) {
strategy.makeQuery(args);
return true;
} catch (UnsupportedSpatialOperation e) {
e.printStackTrace();
ESLoggerFactory.getLogger(GeoFilterIT.class.getName()).info("Unsupported spatial operation {}", e, relation);
return false;
}
}
Expand Down
1 change: 0 additions & 1 deletion core/src/test/java/org/elasticsearch/search/highlight/HighlightBuilderTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,6 @@ public void testParsingEmptyStructure() throws IOException {
context.reset(parser);
highlightBuilder = HighlightBuilder.PROTOTYPE.fromXContent(context);
assertEquals("expected HighlightBuilder with field", new HighlightBuilder().field(new Field("foo")), highlightBuilder);
System.out.println(Math.log(1/(double)(1+1)) + 1.0);
}

/**
Expand Down
56 changes: 29 additions & 27 deletions core/src/test/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,8 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann
try {
channel.sendResponse(new StringMessageResponse("hello " + request.message));
} catch (IOException e) {
e.printStackTrace();
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
}
});
Expand All @@ -162,8 +162,8 @@ public void handleResponse(StringMessageResponse response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
});

Expand Down Expand Up @@ -193,8 +193,8 @@ public void handleResponse(StringMessageResponse response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
});

Expand All @@ -218,7 +218,8 @@ public void testThreadContext() throws ExecutionException, InterruptedException
threadPool.getThreadContext().putHeader("test.pong.user", "pong_user");
channel.sendResponse(response);
} catch (IOException e) {
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
});
final Object context = new Object();
Expand All @@ -245,7 +246,8 @@ public void handleResponse(StringMessageResponse response) {

@Override
public void handleException(TransportException exp) {
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
};
StringMessageRequest ping = new StringMessageRequest("ping");
Expand Down Expand Up @@ -317,8 +319,8 @@ public void messageReceived(TransportRequest.Empty request, TransportChannel cha
try {
channel.sendResponse(TransportResponse.Empty.INSTANCE, TransportResponseOptions.builder().withCompress(true).build());
} catch (IOException e) {
e.printStackTrace();
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
}
});
Expand All @@ -341,8 +343,8 @@ public void handleResponse(TransportResponse.Empty response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
});

Expand All @@ -364,8 +366,8 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann
try {
channel.sendResponse(new StringMessageResponse("hello " + request.message), TransportResponseOptions.builder().withCompress(true).build());
} catch (IOException e) {
e.printStackTrace();
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
}
});
Expand All @@ -389,8 +391,8 @@ public void handleResponse(StringMessageResponse response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
});

Expand Down Expand Up @@ -552,8 +554,8 @@ public void messageReceived(StringMessageRequest request, TransportChannel chann
try {
channel.sendResponse(new StringMessageResponse("hello " + request.message));
} catch (IOException e) {
e.printStackTrace();
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
}
});
Expand Down Expand Up @@ -613,7 +615,7 @@ public void handleResponse(StringMessageResponse response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
logger.error("Unexpected failure", exp);
fail("got exception instead of a response for " + counter + ": " + exp.getDetailedMessage());
}
});
Expand Down Expand Up @@ -959,8 +961,8 @@ public void handleResponse(Version0Response response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
fail();
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}

@Override
Expand Down Expand Up @@ -1000,8 +1002,8 @@ public void handleResponse(Version1Response response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
fail();
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}

@Override
Expand Down Expand Up @@ -1044,8 +1046,8 @@ public void handleResponse(Version1Response response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
fail();
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}

@Override
Expand Down Expand Up @@ -1084,8 +1086,8 @@ public void handleResponse(Version0Response response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
fail();
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}

@Override
Expand Down
8 changes: 4 additions & 4 deletions core/src/test/java/org/elasticsearch/transport/netty/NettyScheduledPingTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ public void messageReceived(TransportRequest.Empty request, TransportChannel cha
try {
channel.sendResponse(TransportResponse.Empty.INSTANCE, TransportResponseOptions.EMPTY);
} catch (IOException e) {
e.printStackTrace();
assertThat(e.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", e);
fail(e.getMessage());
}
}
});
Expand All @@ -113,8 +113,8 @@ public void handleResponse(TransportResponse.Empty response) {

@Override
public void handleException(TransportException exp) {
exp.printStackTrace();
assertThat("got exception instead of a response: " + exp.getMessage(), false, equalTo(true));
logger.error("Unexpected failure", exp);
fail("got exception instead of a response: " + exp.getMessage());
}
}).txGet();
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/org/elasticsearch/versioning/ConcurrentDocumentOperationIT.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void onResponse(IndexResponse response) {

@Override
public void onFailure(Throwable e) {
e.printStackTrace();
logger.error("Unexpected exception while indexing", e);
failure.set(e);
latch.countDown();
}
Expand Down
Loading