I don't have my heart set on keeping the deprecation, so taking it off works for me. I'd also agree that we need a concerted effort to either completely convert or we should leave it un-deprecated so feel free.
Let's move the junit4 stuff off to another discussion. Erick On Thu, Feb 25, 2010 at 1:27 PM, Shai Erera <ser...@gmail.com> wrote: > Erik, I'm totally with you on JUnit 4. I think the @Test annotation is > really not a big deal (it's actually very easy to migrate all the current > tests to JUnit 4 with the added import using some script. Even manually it > should be such a big deal. > > @Ignore is a perfect other advantage of JUnit4. I've found some tests which > were prefixed with _, i.e. _testXYZ just to disable them. Nobody knows about > it until he looks at the code (and pays attention). @Ignore would have been > better. > > And there are lots of other advantages, like the @Before and @After (not > only class). Another problem I've found in the tests is that not all > extended LuceneTestCase, and usually their setUp and tearDown > implementations were wrong - not calling super first/last. When I moved them > to extend LuceneTestCase they broke (I fixed them, don't worry). However, > that could never happen if the super's methods were tagged w/ @Before/After, > because JUnit would take care running them before/after their sub-classes' > @Before/After. So that's another win for JUnit4. > > And of course the @Before/AfterClass are really great ! > > So all in all, I'm a big fan of JUnit4, and if the discussion will start > again, I'll pay more attention to it and participate (I admit I didn't > follow it before). As long as it happens on the list and not on some IRC > channel (!?!?). > > But like Uwe said, that's slightly unrelated to that issue. Because that > deprecation alone produced > 500 warnings (probably even much more), I > un-deprecated it, and when we make a decision one way or the other, we > should simply remove it (in case that's the decision). Until then, let's get > rid of the unnecessary noise, agree? > > Shai > > > On Thu, Feb 25, 2010 at 7:15 PM, Uwe Schindler <u...@thetaphi.de> wrote: > >> 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 >> >> http://www.thetaphi.de >> >> eMail: u...@thetaphi.de >> >> >> >> *From:* Erick Erickson [mailto:erickerick...@gmail.com] >> *Sent:* Thursday, February 25, 2010 5:27 PM >> *To:* java-dev@lucene.apache.org >> *Subject:* Re: [jira] Commented: (LUCENE-2285) Code cleanup from all >> sorts of (trivial) warnings >> >> >> >> Junit4: >> >> >> >> 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.... >> >> >> >> FWIW >> >> Erick >> >> >> >> On Thu, Feb 25, 2010 at 10:54 AM, Uwe Schindler (JIRA) <j...@apache.org> >> wrote: >> >> >> [ >> https://issues.apache.org/jira/browse/LUCENE-2285?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&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: https://issues.apache.org/jira/browse/LUCENE-2285 >> > 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: java-dev-unsubscr...@lucene.apache.org >> For additional commands, e-mail: java-dev-h...@lucene.apache.org >> >> >> > >
