From 8390ae9b110477919b9d3d2be3cc16c0a5bca646 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Fri, 19 Dec 2014 21:29:56 +0000 Subject: [PATCH] LUCENE-6127: fix mockanalyzer to not use asserts git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1646878 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/lucene/analysis/Tokenizer.java | 8 +- .../analysis/TestCachingTokenFilter.java | 9 +- .../analysis/BaseTokenStreamTestCase.java | 15 +-- .../apache/lucene/analysis/MockTokenizer.java | 106 ++++++++++++------ 4 files changed, 86 insertions(+), 52 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/analysis/Tokenizer.java b/lucene/core/src/java/org/apache/lucene/analysis/Tokenizer.java index 6facc45c871..5b41e6e8ced 100644 --- a/lucene/core/src/java/org/apache/lucene/analysis/Tokenizer.java +++ b/lucene/core/src/java/org/apache/lucene/analysis/Tokenizer.java @@ -90,7 +90,7 @@ public abstract class Tokenizer extends TokenStream { throw new IllegalStateException("TokenStream contract violation: close() call missing"); } this.inputPending = input; - assert setReaderTestPoint(); + setReaderTestPoint(); } @Override @@ -100,10 +100,8 @@ public abstract class Tokenizer extends TokenStream { inputPending = ILLEGAL_STATE_READER; } - // only used by assert, for testing - boolean setReaderTestPoint() { - return true; - } + // only used for testing + void setReaderTestPoint() {} private static final Reader ILLEGAL_STATE_READER = new Reader() { @Override diff --git a/lucene/core/src/test/org/apache/lucene/analysis/TestCachingTokenFilter.java b/lucene/core/src/test/org/apache/lucene/analysis/TestCachingTokenFilter.java index e548d5b0ae1..a4310c4e2aa 100644 --- a/lucene/core/src/test/org/apache/lucene/analysis/TestCachingTokenFilter.java +++ b/lucene/core/src/test/org/apache/lucene/analysis/TestCachingTokenFilter.java @@ -120,19 +120,16 @@ public class TestCachingTokenFilter extends BaseTokenStreamTestCase { } public void testDoubleResetFails() throws IOException { - assumeTrue("We want MockAnalyzer to detect double-reset", TEST_ASSERTS_ENABLED); Analyzer analyzer = new MockAnalyzer(random()); final TokenStream input = analyzer.tokenStream("field", "abc"); CachingTokenFilter buffer = new CachingTokenFilter(input); buffer.reset();//ok - boolean madeIt = false; try { buffer.reset();//bad (this used to work which we don't want) - madeIt = true; - } catch (Throwable e) { - //ignore + fail("didn't get expected exception"); + } catch (IllegalStateException e) { + assertEquals("double reset()", e.getMessage()); } - assertFalse(madeIt); } private void checkTokens(TokenStream stream) throws IOException { diff --git a/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java b/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java index f5f42d2827c..e1b2f877abc 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java +++ b/lucene/test-framework/src/java/org/apache/lucene/analysis/BaseTokenStreamTestCase.java @@ -391,9 +391,6 @@ public abstract class BaseTokenStreamTestCase extends LuceneTestCase { } } catch (IllegalStateException expected) { // ok - } catch (AssertionError expected) { - // ok: MockTokenizer - assertTrue(expected.getMessage(), expected.getMessage() != null && expected.getMessage().contains("wrong state")); } catch (Exception unexpected) { unexpected.printStackTrace(System.err); fail("got wrong exception when reset() not called: " + unexpected); @@ -752,13 +749,13 @@ public abstract class BaseTokenStreamTestCase extends LuceneTestCase { } try { ts.end(); - } catch (AssertionError ae) { + } catch (IllegalStateException ise) { // Catch & ignore MockTokenizer's // anger... - if ("end() called before incrementToken() returned false!".equals(ae.getMessage())) { + if ("end() called before incrementToken() returned false!".equals(ise.getMessage())) { // OK } else { - throw ae; + throw ise; } } ts.close(); @@ -777,13 +774,13 @@ public abstract class BaseTokenStreamTestCase extends LuceneTestCase { } try { ts.end(); - } catch (AssertionError ae) { + } catch (IllegalStateException ise) { // Catch & ignore MockTokenizer's // anger... - if ("end() called before incrementToken() returned false!".equals(ae.getMessage())) { + if ("end() called before incrementToken() returned false!".equals(ise.getMessage())) { // OK } else { - throw ae; + throw ise; } } ts.close(); diff --git a/lucene/test-framework/src/java/org/apache/lucene/analysis/MockTokenizer.java b/lucene/test-framework/src/java/org/apache/lucene/analysis/MockTokenizer.java index e85729f3ef9..3714c1af93c 100644 --- a/lucene/test-framework/src/java/org/apache/lucene/analysis/MockTokenizer.java +++ b/lucene/test-framework/src/java/org/apache/lucene/analysis/MockTokenizer.java @@ -83,7 +83,7 @@ public class MockTokenizer extends Tokenizer { }; private State streamState = State.CLOSE; - private int lastOffset = 0; // only for asserting + private int lastOffset = 0; // only for checks private boolean enableChecks = true; // evil: but we don't change the behavior with this random, we only switch up how we read @@ -118,11 +118,25 @@ public class MockTokenizer extends Tokenizer { public MockTokenizer(AttributeFactory factory) { this(factory, WHITESPACE, true); } + + // we allow some checks (e.g. state machine) to be turned off. + // turning off checks just means we suppress exceptions from them + private void fail(String message) { + if (enableChecks) { + throw new IllegalStateException(message); + } + } + + private void failAlways(String message) { + throw new IllegalStateException(message); + } @Override public final boolean incrementToken() throws IOException { - assert !enableChecks || (streamState == State.RESET || streamState == State.INCREMENT) - : "incrementToken() called while in wrong state: " + streamState; + if (streamState != State.RESET && streamState != State.INCREMENT) { + fail("incrementToken() called while in wrong state: " + streamState); + } + clearAttributes(); for (;;) { int startOffset; @@ -160,11 +174,19 @@ public class MockTokenizer extends Tokenizer { } int correctedStartOffset = correctOffset(startOffset); int correctedEndOffset = correctOffset(endOffset); - assert correctedStartOffset >= 0; - assert correctedEndOffset >= 0; - assert correctedStartOffset >= lastOffset; + if (correctedStartOffset < 0) { + failAlways("invalid start offset: " + correctedStartOffset + ", before correction: " + startOffset); + } + if (correctedEndOffset < 0) { + failAlways("invalid end offset: " + correctedEndOffset + ", before correction: " + endOffset); + } + if (correctedStartOffset < lastOffset) { + failAlways("start offset went backwards: " + correctedStartOffset + ", before correction: " + startOffset + ", lastOffset: " + lastOffset); + } lastOffset = correctedStartOffset; - assert correctedEndOffset >= correctedStartOffset; + if (correctedEndOffset < correctedStartOffset) { + failAlways("end offset: " + correctedEndOffset + " is before start offset: " + correctedStartOffset); + } offsetAtt.setOffset(correctedStartOffset, correctedEndOffset); if (state == -1 || runAutomaton.isAccept(state)) { // either we hit a reject state (longest match), or end-of-text, but in an accept state @@ -182,16 +204,20 @@ public class MockTokenizer extends Tokenizer { if (ch < 0) { return ch; } else { - assert !Character.isLowSurrogate((char) ch) : "unpaired low surrogate: " + Integer.toHexString(ch); + if (Character.isLowSurrogate((char) ch)) { + failAlways("unpaired low surrogate: " + Integer.toHexString(ch)); + } off++; if (Character.isHighSurrogate((char) ch)) { int ch2 = readChar(); if (ch2 >= 0) { off++; - assert Character.isLowSurrogate((char) ch2) : "unpaired high surrogate: " + Integer.toHexString(ch) + ", followed by: " + Integer.toHexString(ch2); + if (!Character.isLowSurrogate((char) ch2)) { + failAlways("unpaired high surrogate: " + Integer.toHexString(ch) + ", followed by: " + Integer.toHexString(ch2)); + } return Character.toCodePoint((char) ch, (char) ch2); } else { - assert false : "stream ends with unpaired high surrogate: " + Integer.toHexString(ch); + failAlways("stream ends with unpaired high surrogate: " + Integer.toHexString(ch)); } } return ch; @@ -243,40 +269,56 @@ public class MockTokenizer extends Tokenizer { @Override public void reset() throws IOException { - super.reset(); - state = runAutomaton.getInitialState(); - lastOffset = off = 0; - bufferedCodePoint = -1; - assert !enableChecks || streamState != State.RESET : "double reset()"; - streamState = State.RESET; + try { + super.reset(); + state = runAutomaton.getInitialState(); + lastOffset = off = 0; + bufferedCodePoint = -1; + if (streamState == State.RESET) { + fail("double reset()"); + } + } finally { + streamState = State.RESET; + } } @Override public void close() throws IOException { - super.close(); - // in some exceptional cases (e.g. TestIndexWriterExceptions) a test can prematurely close() - // these tests should disable this check, by default we check the normal workflow. - // TODO: investigate the CachingTokenFilter "double-close"... for now we ignore this - assert !enableChecks || streamState == State.END || streamState == State.CLOSE : "close() called in wrong state: " + streamState; - streamState = State.CLOSE; + try { + super.close(); + // in some exceptional cases (e.g. TestIndexWriterExceptions) a test can prematurely close() + // these tests should disable this check, by default we check the normal workflow. + // TODO: investigate the CachingTokenFilter "double-close"... for now we ignore this + if (!(streamState == State.END || streamState == State.CLOSE)) { + fail("close() called in wrong state: " + streamState); + } + } finally { + streamState = State.CLOSE; + } } @Override - boolean setReaderTestPoint() { - assert !enableChecks || streamState == State.CLOSE : "setReader() called in wrong state: " + streamState; - streamState = State.SETREADER; - return true; + void setReaderTestPoint() { + try { + if (streamState != State.CLOSE) { + fail("setReader() called in wrong state: " + streamState); + } + } finally { + streamState = State.SETREADER; + } } @Override public void end() throws IOException { - super.end(); - int finalOffset = correctOffset(off); - offsetAtt.setOffset(finalOffset, finalOffset); - // some tokenizers, such as limiting tokenizers, call end() before incrementToken() returns false. - // these tests should disable this check (in general you should consume the entire stream) try { - assert !enableChecks || streamState == State.INCREMENT_FALSE : "end() called before incrementToken() returned false!"; + super.end(); + int finalOffset = correctOffset(off); + offsetAtt.setOffset(finalOffset, finalOffset); + // some tokenizers, such as limiting tokenizers, call end() before incrementToken() returns false. + // these tests should disable this check (in general you should consume the entire stream) + if (streamState != State.INCREMENT_FALSE) { + fail("end() called before incrementToken() returned false!"); + } } finally { streamState = State.END; }