lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Uwe Schindler" <>
Subject RE: [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings
Date Thu, 25 Feb 2010 17:15:06 GMT
This discussion is out oft he scope of this issue. We can start the flamewar again. In IRC
we came to the conculsion, that our primary intent is to make the test runs faster, which
we achieved by patching lots of tests to not change static defaults and so be able to run
all tests in the same JVM without forking. More speed improvements can be done by moving read-only
index creation for search tests into static @BeforeClass and setting IndexReaders/-Searchers
to NULL in @AfterClass to allow GC of static fields holding RAMDirectory and so on.


The @Test annotation lead to more confusion and errors at our delevopers. E.g. we had a test
merged back from 3.0 (without Junit4) to trunk or even new tests were added, but nobody added
@Test to it, leading to the fact that the test were never run. So the most important change
to LuceneTestCaseJ4 would be to emulate the old test* method names as if they have @Test.
By that you could still disable them as mentioned, but it would reduce the burden of these
dumb import statements and useless annotations.


By the way, why does LuceneTestCaseJ4 extend TestWatchman and also a instance field extends
that class? I do not understand the whole magic behind, this is totally confusing to me –
annotating a field that is never used in code by an annotation is stupid and looks totally
incorrect (I mean the field holding the TestWatchman-subclass). - This is another thing why
I am against the migration of our already proven tests.


Because of that we don’t want to deprecate LuceneTestCase and instead only transform new
tests and such needing @BeforeClass/@AfterClass for more speed to the new API.



Uwe Schindler

H.-H.-Meier-Allee 63, D-28213 Bremen




From: Erick Erickson [] 
Sent: Thursday, February 25, 2010 5:27 PM
Subject: Re: [jira] Commented: (LUCENE-2285) Code cleanup from all sorts of (trivial) warnings




Well, simply disliking the @Test annotation seems like a poor reason to stay with Junit3,
although I admit it's a pain in the neck to change. Which is why I didn't try to change all
of them. The current system lends itself to the practice of mangling the test name as a way
of not running it, which far too easily allows the test case to be forever ignored. One concrete
advantage of  annotations in Junit4 is the ability to add another "stupid" annotation @Ignore,
which then gets reported and thus doesn't get lost.

As I remember, that last place we left localization what that Mike (?) saw some intermittent
problem that I couldn't reproduce. I could dust off that code and see what the current state
of affairs is since this has come up again. The other problem was that the implementation
I used lead to *increased* test run times. The localization tests basically spun through all
the Locales available and ran all the tests in the class against them. The current system
only runs *some* of the tests in a test class through the localization process. This can be
addressed by, at worst, splitting the test class up, but in my proof-of-concept that seemed
like too much detail...


My purpose in deprecating LuceneTestCase was to explicitly encourage migration to Junit4,
the deprecation warnings being the goad. I vote against removing it....





On Thu, Feb 25, 2010 at 10:54 AM, Uwe Schindler (JIRA) <> wrote:

&focusedCommentId=12838384#action_12838384 ]

Uwe Schindler commented on LUCENE-2285:

Hi Shai,

I applied the patch to my checkout, so it will not get out-of date. As mentioned before, I
have to review each change, as on my first diagonal look-around I found a removed cast in
TestCharArraySet/Map that is important to call the right method, without the cast the test
would pass, but the affected method is never called. I am also not want to remove some casts
in NumericRange and other parts, where the casts were added for more clearness in code. Especially
at some places without the cast it is not clear what javac will do, so the cast is for more
"security" even if not needed.

So please excuse by complaints, but two people looking over such a large patch is really needed.

Thanks for the work! Uwe

> Code cleanup from all sorts of (trivial) warnings
> -------------------------------------------------
>                 Key: LUCENE-2285
>                 URL:
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Shai Erera

>            Assignee: Uwe Schindler

>            Priority: Minor
>             Fix For: 3.1
>         Attachments: LUCENE-2285.patch
> I would like to do some code cleanup and remove all sorts of trivial warnings, like unnecessary
casts, problems w/ javadocs, unused variables, redundant null checks, unnecessary semicolon
etc. These are all very trivial and should not pose any problem.
> I'll create another issue for getting rid of deprecated code usage, like LuceneTestCase
and all sorts of deprecated constructors. That's also trivial because it only affects Lucene
code, but it's a different type of change.
> Another issue I'd like to create is about introducing more generics in the code, where
it's missing today - not changing existing API. There are many places in the code like that.
> So, with you permission, I'll start with the trivial ones first, and then move on to
the others.

This message is automatically generated by JIRA.
You can reply to this email to add a comment to the issue online.

To unsubscribe, e-mail:
For additional commands, e-mail:


View raw message