diff --git a/lucene/analysis/stempel/src/test/org/egothor/stemmer/TestCompile.java b/lucene/analysis/stempel/src/test/org/egothor/stemmer/TestCompile.java index dbf93fc2da4..4852bd8570f 100644 --- a/lucene/analysis/stempel/src/test/org/egothor/stemmer/TestCompile.java +++ b/lucene/analysis/stempel/src/test/org/egothor/stemmer/TestCompile.java @@ -72,7 +72,6 @@ import java.util.StringTokenizer; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; -@SuppressSysoutChecks(bugUrl = "External tool.") public class TestCompile extends LuceneTestCase { public void testCompile() throws Exception { diff --git a/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMABaseAnalyzerTest.java b/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMABaseAnalyzerTest.java index 4da3a9835b2..59210062932 100644 --- a/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMABaseAnalyzerTest.java +++ b/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMABaseAnalyzerTest.java @@ -42,7 +42,6 @@ import java.util.Map; /** * Testcase for {@link UIMABaseAnalyzer} */ -@SuppressSysoutChecks(bugUrl = "UIMA logs via ju.logging") public class UIMABaseAnalyzerTest extends BaseTokenStreamTestCase { private UIMABaseAnalyzer analyzer; diff --git a/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMATypeAwareAnalyzerTest.java b/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMATypeAwareAnalyzerTest.java index 0b9598c0228..454e45e9dd1 100644 --- a/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMATypeAwareAnalyzerTest.java +++ b/lucene/analysis/uima/src/test/org/apache/lucene/analysis/uima/UIMATypeAwareAnalyzerTest.java @@ -27,7 +27,6 @@ import org.junit.Test; /** * Testcase for {@link UIMATypeAwareAnalyzer} */ -@SuppressSysoutChecks(bugUrl = "UIMA logs via ju.logging") public class UIMATypeAwareAnalyzerTest extends BaseTokenStreamTestCase { private UIMATypeAwareAnalyzer analyzer; diff --git a/lucene/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java b/lucene/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java index edb7c8b1385..ae0ae9ca059 100644 --- a/lucene/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java +++ b/lucene/benchmark/src/test/org/apache/lucene/benchmark/BenchmarkTestCase.java @@ -31,7 +31,6 @@ import org.junit.AfterClass; import org.junit.BeforeClass; /** Base class for all Benchmark unit tests. */ -@SuppressSysoutChecks(bugUrl = "Output expected.") public abstract class BenchmarkTestCase extends LuceneTestCase { private static File WORKDIR; diff --git a/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksLogic.java b/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksLogic.java index 66dc846036f..5fc7f82e29d 100644 --- a/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksLogic.java +++ b/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksLogic.java @@ -67,7 +67,6 @@ import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; /** * Test very simply that perf tasks - simple algorithms - are doing what they should. */ -@SuppressSysoutChecks(bugUrl = "Output expected.") public class TestPerfTasksLogic extends BenchmarkTestCase { @Override diff --git a/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksParse.java b/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksParse.java index 426020eba60..675452274ab 100644 --- a/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksParse.java +++ b/lucene/benchmark/src/test/org/apache/lucene/benchmark/byTask/TestPerfTasksParse.java @@ -42,7 +42,6 @@ import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; import conf.ConfLoader; /** Test very simply that perf tasks are parses as expected. */ -@SuppressSysoutChecks(bugUrl = "Output expected.") public class TestPerfTasksParse extends LuceneTestCase { static final String NEW_LINE = System.getProperty("line.separator"); diff --git a/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/TestQualityRun.java b/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/TestQualityRun.java index 2574ff55342..5f101a69cf3 100644 --- a/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/TestQualityRun.java +++ b/lucene/benchmark/src/test/org/apache/lucene/benchmark/quality/TestQualityRun.java @@ -44,7 +44,6 @@ import java.nio.charset.StandardCharsets; * this test will not work correctly, as it does not dynamically * generate its test trec topics/qrels! */ -@SuppressSysoutChecks(bugUrl = "Output expected.") public class TestQualityRun extends BenchmarkTestCase { @Override diff --git a/lucene/common-build.xml b/lucene/common-build.xml index cd44cf997e8..71ff92b6957 100644 --- a/lucene/common-build.xml +++ b/lucene/common-build.xml @@ -1008,7 +1008,6 @@ - diff --git a/lucene/core/src/test/org/apache/lucene/codecs/compressing/TestFastCompressionMode.java b/lucene/core/src/test/org/apache/lucene/codecs/compressing/TestFastCompressionMode.java index ab1500f83b9..4fd3471f861 100644 --- a/lucene/core/src/test/org/apache/lucene/codecs/compressing/TestFastCompressionMode.java +++ b/lucene/core/src/test/org/apache/lucene/codecs/compressing/TestFastCompressionMode.java @@ -24,5 +24,4 @@ public class TestFastCompressionMode extends AbstractTestLZ4CompressionMode { super.setUp(); mode = CompressionMode.FAST; } - } diff --git a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfDirectoryNotClosed.java b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfDirectoryNotClosed.java index c9a2b02e844..88d6641d1d7 100644 --- a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfDirectoryNotClosed.java +++ b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestFailIfDirectoryNotClosed.java @@ -18,7 +18,6 @@ package org.apache.lucene.util.junitcompat; */ import org.apache.lucene.store.Directory; -import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; import org.junit.Assert; import org.junit.Test; import org.junit.runner.JUnitCore; @@ -30,8 +29,7 @@ public class TestFailIfDirectoryNotClosed extends WithNestedTests { public TestFailIfDirectoryNotClosed() { super(true); } - - @SuppressSysoutChecks(bugUrl = "Expected.") + public static class Nested1 extends WithNestedTests.AbstractNestedTest { public void testDummy() throws Exception { Directory dir = newDirectory(); diff --git a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSameRandomnessLocalePassedOrNot.java b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSameRandomnessLocalePassedOrNot.java index 0dbfc31e20d..459f344bd83 100644 --- a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSameRandomnessLocalePassedOrNot.java +++ b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSameRandomnessLocalePassedOrNot.java @@ -43,7 +43,6 @@ public class TestSameRandomnessLocalePassedOrNot extends WithNestedTests { super(true); } - @SuppressSysoutChecks(bugUrl = "Expected.") public static class Nested extends WithNestedTests.AbstractNestedTest { public static String pickString; public static Locale defaultLocale; diff --git a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSeedFromUncaught.java b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSeedFromUncaught.java index fcf71bf0605..361f10b646e 100644 --- a/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSeedFromUncaught.java +++ b/lucene/core/src/test/org/apache/lucene/util/junitcompat/TestSeedFromUncaught.java @@ -30,7 +30,6 @@ import org.junit.runner.notification.Failure; * console. */ public class TestSeedFromUncaught extends WithNestedTests { - @SuppressSysoutChecks(bugUrl = "Expected.") public static class ThrowInUncaught extends AbstractNestedTest { @Test public void testFoo() throws Exception { diff --git a/lucene/demo/src/test/org/apache/lucene/demo/TestDemo.java b/lucene/demo/src/test/org/apache/lucene/demo/TestDemo.java index 85604ac2890..b7590336fb6 100644 --- a/lucene/demo/src/test/org/apache/lucene/demo/TestDemo.java +++ b/lucene/demo/src/test/org/apache/lucene/demo/TestDemo.java @@ -25,7 +25,6 @@ import java.nio.charset.Charset; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; -@SuppressSysoutChecks(bugUrl = "Output expected.") public class TestDemo extends LuceneTestCase { private void testOneSearch(File indexPath, String query, int expectedHitCount) throws Exception { diff --git a/lucene/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java b/lucene/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java index 648d2af6b15..3db6c4c3d5b 100644 --- a/lucene/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java +++ b/lucene/misc/src/test/org/apache/lucene/index/TestMultiPassIndexSplitter.java @@ -24,7 +24,6 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.LuceneTestCase; import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; -@SuppressSysoutChecks(bugUrl = "Output expected (external tool).") public class TestMultiPassIndexSplitter extends LuceneTestCase { IndexReader input; int NUM_DOCS = 11; diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java index e22d7cfc4a8..055b370a703 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java @@ -230,6 +230,7 @@ import static com.carrotsearch.randomizedtesting.RandomizedTest.systemPropertyAs @ThreadLeakFilters(defaultFilters = true, filters = { QuickPatchThreadsFilter.class }) +@TestRuleLimitSysouts.Limit(bytes = TestRuleLimitSysouts.DEFAULT_SYSOUT_BYTES_THRESHOLD) public abstract class LuceneTestCase extends Assert { // -------------------------------------------------------------------- @@ -248,13 +249,6 @@ public abstract class LuceneTestCase extends Assert { /** @see #ignoreAfterMaxFailures*/ public static final String SYSPROP_FAILFAST = "tests.failfast"; - - /** - * If true, enables assertions on writing to system streams. - * - * @see TestRuleLimitSysouts - */ - public static final String SYSPROP_SYSOUTS = "tests.sysouts"; /** * Annotation for tests that should only be run during nightly builds. @@ -356,8 +350,8 @@ public abstract class LuceneTestCase extends Assert { } /** - * Marks any suite which is known to print to {@link System#out} or {@link System#err}, - * even when {@link #VERBOSE} is disabled. + * Ignore {@link TestRuleLimitSysouts} for any suite which is known to print + * over the default limit of bytes to {@link System#out} or {@link System#err}. * * @see TestRuleLimitSysouts */ @@ -369,7 +363,7 @@ public abstract class LuceneTestCase extends Assert { /** Point to JIRA entry. */ public String bugUrl(); } - + // ----------------------------------------------------------------- // Truly immutable fields and constants, initialized once and valid // for all suites ever since. diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/RunListenerPrintReproduceInfo.java b/lucene/test-framework/src/java/org/apache/lucene/util/RunListenerPrintReproduceInfo.java index 6a1624bddd0..6041d22e96a 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/RunListenerPrintReproduceInfo.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/RunListenerPrintReproduceInfo.java @@ -154,9 +154,6 @@ public final class RunListenerPrintReproduceInfo extends RunListener { addVmOpt(b, "testcase", RandomizedContext.current().getTargetClass().getSimpleName()); addVmOpt(b, "tests.method", testName); addVmOpt(b, "tests.seed", RandomizedContext.current().getRunnerSeedAsString()); - - // Misc switches. - addVmOpt(b, SYSPROP_SYSOUTS, System.getProperty(SYSPROP_SYSOUTS)); // Test groups and multipliers. if (RANDOM_MULTIPLIER > 1) addVmOpt(b, "tests.multiplier", RANDOM_MULTIPLIER); diff --git a/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleDisallowSysouts.java b/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleLimitSysouts.java similarity index 55% rename from lucene/test-framework/src/java/org/apache/lucene/util/TestRuleDisallowSysouts.java rename to lucene/test-framework/src/java/org/apache/lucene/util/TestRuleLimitSysouts.java index 7549ac0d669..5298d7d640d 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleDisallowSysouts.java +++ b/lucene/test-framework/src/java/org/apache/lucene/util/TestRuleLimitSysouts.java @@ -5,9 +5,16 @@ import java.io.IOException; import java.io.OutputStream; import java.io.PrintStream; import java.io.UnsupportedEncodingException; +import java.lang.annotation.Documented; +import java.lang.annotation.ElementType; +import java.lang.annotation.Inherited; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; import java.nio.charset.Charset; import java.util.List; -import java.util.concurrent.atomic.AtomicReference; +import java.util.Locale; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.lucene.util.LuceneTestCase.SuppressSysoutChecks; @@ -33,25 +40,48 @@ import com.carrotsearch.randomizedtesting.rules.TestRuleAdapter; */ /** - * Fails the suite if it prints anything to {@link System#out} or {@link System#err}, + * Fails the suite if it prints over the given limit of bytes to either + * {@link System#out} or {@link System#err}, * unless the condition is not enforced (see {@link #isEnforced()}). */ -public class TestRuleDisallowSysouts extends TestRuleAdapter { - /** - * Stack trace of any thread that wrote something to sysout or syserr. +public class TestRuleLimitSysouts extends TestRuleAdapter { + /** + * Max limit of bytes printed to either {@link System#out} or {@link System#err}. + * This limit is enforced per-class (suite). */ - private final static AtomicReference firstWriteStack = new AtomicReference(); + public final static int DEFAULT_SYSOUT_BYTES_THRESHOLD = 8 * 1024; + + /** + * An annotation specifying the limit of bytes per class. + */ + @Documented + @Inherited + @Retention(RetentionPolicy.RUNTIME) + @Target(ElementType.TYPE) + public static @interface Limit { + public int bytes(); + } + + private final static AtomicInteger bytesWritten = new AtomicInteger(); private final static DelegateStream capturedSystemOut; private final static DelegateStream capturedSystemErr; + /** + * We capture system output and error streams as early as possible because + * certain components (like the Java logging system) steal these references and + * never refresh them. + * + * Also, for this exact reason, we cannot change delegate streams for every suite. + * This isn't as elegant as it should be, but there's no workaround for this. + */ static { System.out.flush(); System.err.flush(); final String csn = Charset.defaultCharset().name(); - capturedSystemOut = new DelegateStream(System.out, csn, firstWriteStack); - capturedSystemErr = new DelegateStream(System.err, csn, firstWriteStack); + capturedSystemOut = new DelegateStream(System.out, csn, bytesWritten); + capturedSystemErr = new DelegateStream(System.err, csn, bytesWritten); System.setOut(capturedSystemOut.printStream); System.setErr(capturedSystemErr.printStream); @@ -63,18 +93,18 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { private final TestRuleMarkFailure failureMarker; /** - * Sets {@link #firstWriteStack} to the current stack trace upon the first actual write - * to an underlying stream. + * Tracks the number of bytes written to an underlying stream by + * incrementing an {@link AtomicInteger}. */ static class DelegateStream extends FilterOutputStream { - private final AtomicReference firstWriteStack; final PrintStream printStream; + final AtomicInteger bytesCounter; - public DelegateStream(OutputStream delegate, String charset, AtomicReference firstWriteStack) { + public DelegateStream(OutputStream delegate, String charset, AtomicInteger bytesCounter) { super(delegate); try { - this.firstWriteStack = firstWriteStack; this.printStream = new PrintStream(this, true, charset); + this.bytesCounter = bytesCounter; } catch (UnsupportedEncodingException e) { throw new RuntimeException(e); } @@ -85,7 +115,7 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { @Override public void write(byte[] b) throws IOException { if (b.length > 0) { - bytesWritten(); + bytesCounter.addAndGet(b.length); } super.write(b); } @@ -93,27 +123,19 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { @Override public void write(byte[] b, int off, int len) throws IOException { if (len > 0) { - bytesWritten(); + bytesCounter.addAndGet(len); } super.write(b, off, len); } @Override public void write(int b) throws IOException { - bytesWritten(); + bytesCounter.incrementAndGet(); super.write(b); } - - private void bytesWritten() { - // This check isn't really needed, but getting the stack is expensive and may involve - // jit deopts, so we'll do it anyway. - if (firstWriteStack.get() == null) { - firstWriteStack.compareAndSet(null, Thread.currentThread().getStackTrace()); - } - } } - - public TestRuleDisallowSysouts(TestRuleMarkFailure failureMarker) { + + public TestRuleLimitSysouts(TestRuleMarkFailure failureMarker) { this.failureMarker = failureMarker; } @@ -125,6 +147,19 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { checkCaptureStreams(); } resetCaptureState(); + validateClassAnnotations(); + } + + private void validateClassAnnotations() { + Class target = RandomizedTest.getContext().getTargetClass(); + if (target.isAnnotationPresent(Limit.class)) { + int bytes = target.getAnnotation(Limit.class).bytes(); + if (bytes < 0 || bytes > 1 * 1024 * 1024) { + throw new AssertionError("The sysout limit is insane. Did you want to use " + + "@" + LuceneTestCase.SuppressSysoutChecks.class.getName() + " annotation to " + + "avoid sysout checks entirely?"); + } + } } /** @@ -141,13 +176,19 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { } protected boolean isEnforced() { + Class target = RandomizedTest.getContext().getTargetClass(); + if (LuceneTestCase.VERBOSE || LuceneTestCase.INFOSTREAM || - RandomizedTest.getContext().getTargetClass().isAnnotationPresent(SuppressSysoutChecks.class)) { + target.isAnnotationPresent(SuppressSysoutChecks.class)) { return false; } - return !RandomizedTest.systemPropertyAsBoolean(LuceneTestCase.SYSPROP_SYSOUTS, true); + if (!target.isAnnotationPresent(Limit.class)) { + return false; + } + + return true; } /** @@ -163,15 +204,17 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { capturedSystemOut.printStream.flush(); capturedSystemErr.printStream.flush(); - // And check for offenders, but only if everything was successful so far. - StackTraceElement[] offenderStack = firstWriteStack.get(); - if (offenderStack != null && failureMarker.wasSuccessful()) { - AssertionError e = new AssertionError("The test or suite printed information to stdout or stderr," + - " even though verbose mode is turned off and it's not annotated with @" + - SuppressSysoutChecks.class.getSimpleName() + ". This exception contains the stack" + - " trace of the first offending call."); - e.setStackTrace(offenderStack); - throw e; + // Check for offenders, but only if everything was successful so far. + int limit = RandomizedTest.getContext().getTargetClass().getAnnotation(Limit.class).bytes(); + if (bytesWritten.get() >= limit && failureMarker.wasSuccessful()) { + throw new AssertionError(String.format(Locale.ENGLISH, + "The test or suite printed %d bytes to stdout and stderr," + + " even though the limit was set to %d bytes. Increase the limit with @%s, ignore it completely" + + " with @%s or run with -Dtests.verbose=true", + bytesWritten.get(), + limit, + Limit.class.getSimpleName(), + SuppressSysoutChecks.class.getSimpleName())); } } } @@ -187,7 +230,7 @@ public class TestRuleDisallowSysouts extends TestRuleAdapter { private void resetCaptureState() { capturedSystemOut.printStream.flush(); capturedSystemErr.printStream.flush(); - firstWriteStack.set(null); - } + bytesWritten.set(0); + } } diff --git a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java index a01f02d9616..cc2f4760ab2 100644 --- a/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java +++ b/solr/test-framework/src/java/org/apache/solr/SolrTestCaseJ4.java @@ -119,7 +119,7 @@ import com.carrotsearch.randomizedtesting.rules.SystemPropertiesRestoreRule; SolrIgnoredThreadsFilter.class, QuickPatchThreadsFilter.class }) -@SuppressSysoutChecks(bugUrl = "Solr dumps logs to console.") +@SuppressSysoutChecks(bugUrl = "Solr dumps tons of logs to console.") public abstract class SolrTestCaseJ4 extends LuceneTestCase { private static String coreName = ConfigSolrXmlOld.DEFAULT_DEFAULT_CORE_NAME; public static int DEFAULT_CONNECTION_TIMEOUT = 60000; // default socket connection timeout in ms