Return-Path: Delivered-To: apmail-lucene-java-dev-archive@www.apache.org Received: (qmail 43223 invoked from network); 26 Feb 2010 04:16:06 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.3) by minotaur.apache.org with SMTP; 26 Feb 2010 04:16:06 -0000 Received: (qmail 4193 invoked by uid 500); 26 Feb 2010 04:16:05 -0000 Delivered-To: apmail-lucene-java-dev-archive@lucene.apache.org Received: (qmail 4148 invoked by uid 500); 26 Feb 2010 04:16:05 -0000 Mailing-List: contact java-dev-help@lucene.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: java-dev@lucene.apache.org Delivered-To: mailing list java-dev@lucene.apache.org Received: (qmail 4141 invoked by uid 99); 26 Feb 2010 04:16:05 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 26 Feb 2010 04:16:05 +0000 X-ASF-Spam-Status: No, hits=2.2 required=10.0 tests=HTML_MESSAGE,SPF_PASS X-Spam-Check-By: apache.org Received-SPF: pass (athena.apache.org: domain of serera@gmail.com designates 74.125.82.176 as permitted sender) Received: from [74.125.82.176] (HELO mail-wy0-f176.google.com) (74.125.82.176) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 26 Feb 2010 04:15:58 +0000 Received: by wyf22 with SMTP id 22so2176075wyf.35 for ; Thu, 25 Feb 2010 20:15:37 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:content-type; bh=UTBHM+Ac58mhtVF1Nr6JdEpleaMit3UI5E5GYMzbHrw=; b=BtR/f88/aQO7yftCh+KZzx5ykLqekmX7Lyf+IluE5SGx3ts6LhpxqhYfG1qcxQqz45 M3pPd0T06RHx1fUnEhAhN/kg29qjxkyRIIZqrx2U5Cjj8nkYzRvrFpt6uOQdpdtSuS9E Fu7sOyah2s65k16oFkLHARva8t88KHQ6hwaoc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; b=PRVqcSK+TKP5UipK8KnfJePUXPuhVV3H1IsIvAeODUhjIISfgsqoqdQ+98dqzE92Cn O+a4c2OJg2g+78TkVkFSKNMxvPQwnXXVHI/24x6/LhbaCObAHGcVYdkI+TfCSVgGMCk5 xcHNVSU80EhkUFAKTQquj9P7oMHxfjheBLSvg= MIME-Version: 1.0 Received: by 10.216.91.82 with SMTP id g60mr858381wef.98.1267157737503; Thu, 25 Feb 2010 20:15:37 -0800 (PST) In-Reply-To: <359a92831002251857j5a7415a6p651ed08f55d73c8e@mail.gmail.com> References: <359a92831002251752p6b04b488q2a2af8279fd530ef@mail.gmail.com> <8f0ad1f31002251806v244a9d40v8a595dabfbd9521a@mail.gmail.com> <359a92831002251857j5a7415a6p651ed08f55d73c8e@mail.gmail.com> Date: Fri, 26 Feb 2010 06:15:37 +0200 Message-ID: <786fde51002252015h310d1955w2bd2c6d18dd2174f@mail.gmail.com> Subject: Re: Uwe's question From: Shai Erera To: java-dev@lucene.apache.org Content-Type: multipart/alternative; boundary=0016e6d7858ed17c730480792624 --0016e6d7858ed17c730480792624 Content-Type: text/plain; charset=ISO-8859-1 Ok this seems a discussion related to JUnit 4, so I'll port what I've said about it from the other thread (doing the code cleanup): {quote} 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 shouldn't 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 them 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 ! {quote} I think the @Before/After annotations can be a real win for our tests. My two cents, Shai On Fri, Feb 26, 2010 at 4:57 AM, Erick Erickson wrote: > Well, "Things got busy (tm)". Uwe's point if valid; unless there's > demonstrable gain, moving things to Junit4 "just for fun" is wasted motion, > indeed dangerous. I was focusing on LocalizedTestCase to understand the > place of runBare etc. in the scheme of things since when I created > LuceneTestCaseJ4 that was something I wanted to figure out to make it a > replacement for LuceneTestCase. > > I can't point to a compelling reason to shake up the code, the only > improvement it would have is having a demonstration of using the Junit4 > @RunWith annotation for future reference. > > So, I've no compelling reason to push that patch forward. If y'all think > it's worth it I'll be happy to crank that patch back up again, it'll take a > few days though. It does affect a several files, and if the main value here > is an exemplar of the @RunWith annotation, perhaps there's a better place to > put that in. > > Erick > > > On Thu, Feb 25, 2010 at 9:06 PM, Robert Muir wrote: > >> >> >> >>> LocalizedTestCase called runBare in LuceneTestCase which reported the >>> seed value if an exception was thrown. I couldn't find a good way to access >>> runBare or analogs in Junit4, but the interceptor pattern worked as well. >>> The interceptor is called by the Junit framework on test events, so there >>> aren't references to it in the Lucene test code. There are other places that >>> call runBare, so I assumed that if anyone wanted to use Junit4 with those >>> classes it would be a good thing to allow. >>> >> >> I didn't forget about your patch Erick, in my opinion there is nothing >> wrong with it. I hope its not discouraging you, the problem is a few of us >> have spent countless hours trying to debug this hard-to-reproduce Thai test >> failure problem. >> >> It failed in the existing tests, too, with Junit 3 on hudson (one time!). >> At this point, i start to wonder if it could be related to stuff like this: >> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6683975 >> >> I don't think we should let this stop progress with the tests, if you >> think we should move LocalizedTestCase to junit 4 lets do it. >> >> -- >> Robert Muir >> rcmuir@gmail.com >> > > --0016e6d7858ed17c730480792624 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable
Ok this seems a discussion related to JUnit 4, so I'll= port what I've said about it from the other thread (doing the code cle= anup):

{quote}
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 shouldn't 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 them 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 !
{quote}
<= br>I think the @Before/After annotations can be a real win for our tests.
My two cents,
Shai

On Fri, Feb 2= 6, 2010 at 4:57 AM, Erick Erickson <erickerickson@gmail.com> wrote:
Well, "Thing= s got busy (tm)". Uwe's point if valid; unless there's demonst= rable gain, moving things to Junit4 "just for fun" is wasted moti= on, indeed dangerous. I was focusing on LocalizedTestCase to understand the= place of runBare etc. in the scheme of things since when I created LuceneT= estCaseJ4 that was something I wanted to figure out to make it a replacemen= t for LuceneTestCase.

I can't point to a compelling reason to shake up the code, the only= improvement it would have is having a demonstration of using the Junit4 @R= unWith annotation for future reference.

So, I've no compelling r= eason to push that patch forward. If y'all think it's worth it I= 9;ll be happy to crank that patch back up again, it'll take a few days = though. It does affect a several files, and if the main value here is an ex= emplar of the @RunWith annotation, perhaps there's a better place to pu= t that in.

Erick


On Thu, Feb 25, 2010 at 9:06 PM, Robert Muir &l= t;rcmuir@gmail.com> wrote:



LocalizedTestCase called runBare in LuceneTestCase which reported the seed = value if an exception was thrown. I couldn't find a good way to access = runBare or analogs in Junit4, but the interceptor pattern worked as well. T= he interceptor is called by the Junit framework on test events, so there ar= en't references to it in the Lucene test code. There are other places t= hat call runBare, so I assumed that if anyone wanted to use Junit4 with tho= se classes it would be a good thing to allow.

I didn't forget about your patch Erick, in my op= inion there is nothing wrong with it. I hope its not discouraging you, the = problem is a few of us have spent countless hours trying to debug this hard= -to-reproduce Thai test failure problem.

It failed in the existing tests, too, with Junit 3 on hudson (one time!= ). At this point, i start to wonder if it could be related to stuff like th= is: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=3D66= 83975

I don't think we should let this stop progress with the tests, if y= ou think we should move LocalizedTestCase to junit 4 lets do it.

--
Robert Muir
rcmuir@gmail.com


--0016e6d7858ed17c730480792624--