Hi Hoss,
sorry did not see your message!
1) The test framework never randomly disables asserts (it cannot do this,
because asserts are enabled before the VM starts - you cannot do that at
runtime). So to disable asserts you have to pass this explicitly to the test
runner, and the default is asserts enabled. Currently Policeman Jenkins does
not disable asserts at the moment, but it could do that. For normal developers
running tests, they are always enabled (we also have a test that validates
this). In addition, our tests should not check the JVM for correctness, it is
just there to make an assumption about how WS should behave. So when this
assert fails, the test is still not wrong.
2) Now the answer: I changed it because of performance. The String.format /
String concatenation is executed on every single character and if the assert
does not fail. If you create a large whitespace string this takes endless (I
tried it), because it creates a new string, formats it,... A native Java assert
only calls the string part if the assert actually failed.
Based on (1) and (2) this is OK. If you still want to revert to
Assert.assertTrue like before, please use an if-condition instead:
if (not whitespace) Assert.fail(String.format(error message));
Uwe
-----
Uwe Schindler
H.-H.-Meier-Allee 63, D-28213 Bremen
http://www.thetaphi.de
eMail: u...@thetaphi.de
> -----Original Message-----
> From: Chris Hostetter [mailto:hossman_luc...@fucit.org]
> Sent: Thursday, August 27, 2015 12:15 AM
> To: u...@thetaphi.de; Lucene Dev
> Subject: Re: svn commit: r1697131 - in /lucene/dev/trunk/lucene:
> JRE_VERSION_MIGRATION.txt test-
> framework/src/java/org/apache/lucene/util/TestUtil.java
>
>
> Uwe: I'm still concerned about this change and the way it might result in
> confusing failure messages in the future (if the whitespace def of other
> characters changes) ... can you please explain your choice "Assert.assertTrue
> -> assert" ?
>
>
> On Mon, 24 Aug 2015, Chris Hostetter wrote:
>
> : Uwe: why did you change this from "Assert.assertTrue" to "assert" ?
> :
> : In the old code the test would fail every time with a clear explanation of
> : hte problem -- in your new code, if assertions are randomly disabled by
> : the test framework, then the sanity check won't run and instead we'll get
> : a strange failure from whatever test called this method.
> :
> :
> :
> : :
> ==========================================================
> ====================
> : : --- lucene/dev/trunk/lucene/test-
> framework/src/java/org/apache/lucene/util/TestUtil.java (original)
> : : +++ lucene/dev/trunk/lucene/test-
> framework/src/java/org/apache/lucene/util/TestUtil.java Sat Aug 22
> 21:33:47 2015
> : : @@ -35,6 +35,7 @@ import java.util.Collections;
> : : import java.util.HashMap;
> : : import java.util.Iterator;
> : : import java.util.List;
> : : +import java.util.Locale;
> : : import java.util.Map;
> : : import java.util.NoSuchElementException;
> : : import java.util.Random;
> : : @@ -1188,7 +1189,7 @@ public final class TestUtil {
> : : int offset = nextInt(r, 0, WHITESPACE_CHARACTERS.length-1);
> : : char c = WHITESPACE_CHARACTERS[offset];
> : : // sanity check
> : : - Assert.assertTrue("Not really whitespace? (@"+offset+"): " + c,
> Character.isWhitespace(c));
> : : + assert Character.isWhitespace(c) : String.format(Locale.ENGLISH,
> "Not
> really whitespace? WHITESPACE_CHARACTERS[%d] is '\\u%04X'", offset, (int)
> c);
> : : out.append(c);
> : : }
> : : return out.toString();
> : : @@ -1307,9 +1308,9 @@ public final class TestUtil {
> : : '\u001E',
> : : '\u001F',
> : : '\u0020',
> : : - // '\u0085', faild sanity check?
> : : + // '\u0085', failed sanity check?
> : : '\u1680',
> : : - '\u180E',
> : : + // '\u180E', no longer whitespace in Unicode 7.0 (Java 9)!
> : : '\u2000',
> : : '\u2001',
> : : '\u2002',
> : :
> : :
> : :
> :
> : -Hoss
> : http://www.lucidworks.com/
> :
>
> -Hoss
> http://www.lucidworks.com/
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org For additional
> commands, e-mail: dev-h...@lucene.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
