Mailing-List: contact dev-help@openjpa.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@openjpa.apache.org
Received-SPF: pass (athena.apache.org: domain of michael.d.dick@gmail.com
 designates 209.85.218.180 as permitted sender)
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=BZSCXcEKP+kz2dNJui7bximUkvhp+QhvnTnOrOxQchSefjC5v9RFucBwfzdaxA/MZy
         M9mCY2TE2FG0kiK6YbyVOEZT7ZlHHl0Er5dTWrhOPlchK9uclTLpnNolDIIUBnvAkjQg
         cU98CyS2Xm8zaQuRZAR6ONCnFfjii1slPe71Y=
MIME-Version: 1.0
In-Reply-To: <1239058171603-2595933.post@n2.nabble.com>
References: <72c1350f0904060830w5f77b33alfe7af79f8a290476@mail.gmail.com>
	 <1239038102827-2594325.post@n2.nabble.com>
	 <72c1350f0904061530pe57d764jcd11410aa2d204d6@mail.gmail.com>
	 <1239058171603-2595933.post@n2.nabble.com>
Date: Tue, 7 Apr 2009 07:07:04 -0500
Message-ID: <72c1350f0904070507r6c60971at9b8d3729c5314d42@mail.gmail.com>
Subject: Re: [DISCUSS] refactor @AllowFailure
From: Michael Dick <michael.d.dick@gmail.com>
To: dev@openjpa.apache.org
Content-Type: multipart/alternative; boundary=001636c5a57b6e62950466f5da1d

--001636c5a57b6e62950466f5da1d
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On Mon, Apr 6, 2009 at 5:49 PM, Pinaki Poddar <ppoddar@apache.org> wrote:

>
> > To put it bluntly if we don't care enough to fail the build, why do we
> care
> > enough to run the test?
>
> The intended usage of @AllowFailure is as follows:
> User X reports a failure accompanied by a test case. We want to verify the
> failure and accept X's contributed test case. So once we confirmed that the
> test case indeed causes a failure, we check-in the test case with
> @AllowFailure. The test being run is critical in such cases to know how it
> fails rather than completely excluding the test being executed at all.
>

> Another recent usage was when Albert checked-in a whole set of tests for
> the new locking  capabilities but actual functional support was added later.
>

How does running the test during every build help either of these scenarios?
It seems to me that a jUnit style @Ignore works just as well if not better.
When individual developers work on a fix for these problems they can easily
remove the @Ignore annotation (here an exclusion in pom.xml is better since
they can run the testcase in eclipse unmodified) do development work, when
the problem is fixed they commit supporting changes and enable the test.


> I was apprehensive of @AllowFailure being misused (I myself have been
> tempted) to annotate a previously passing test case to get a check-in
> through. The antidote for such tendencies should be
> a) audit all tests that are annotated with @AllowFailure periodically
> b) run the harness with a flag such that it does not recognize
> @AllowFailure annotation at all
>

I think the reverse is better. Only run tests annotated with AllowFailure
upon demand. Running tests that we know will fail has minimal benefit.

The other problem is that once a testcase has been annotated as AllowFailure
it may remain so for some time.  At the moment there are two groups of tests
annotated in this manner. The majority (approx 22  of 30) have been modified
in the past two months. The remainder haven't been modified (except for
whitespace conversions) for > 6 months.

 To summarize, we need few facilities
> 1. Ability to check-in a test that currently fails and tells us how exactly
> it fails
> 2. Ability to audit which tests are currently 'not passing' which must
> include tests that are ignored or excluded or allowed failure or whichever
> bypass we can devise in futrue.
> 3. Our harness must be capable of running 'all the tests' irrespective of
> whatever way they have been excluded.
>
> As long as these facilities are available, it is purely a matter of taste
> whether we use home-grown or not home-grown code to do it.
>

Not sure I agree. I think we should reuse code when applicable and utilize
de facto standards like jUnit, to make the project more accessable to new
developers. In addition utilizing standard annotations absolves us of some
of the responsiblity for writing tools to audit the tests. Surefire provides
a count of skipped test cases (not sure whether @Ignore would count as
skipped but there is likely a way to use this number). This number would be
included in any generated reports and is easier to track than running $ find
trunk/ -type f iname '*.java' | xargs fgrep '@AllowFailure' to find how many
tests we're running but not counting.

Regardless of the facility I think Don hit the two critical use cases.
Exclude tests based on platform, and exclude tests until such time as
they're passing.

-mike


Hi Pinaki,
>
> Maybe I should have said the @AllowFailure annotation ignores the results
> of
> a test, not the test itself. I'm not sure why that would be better than
> just
> excluding the test altogether though.
>
> Having testcases that might work (@AllowFailure) undermines the purpose of
> the test suite. The function under test may start working at any time and
> then stop working at any time, all the while we're blissfully unaware of
> what we've done.
>
> To put it bluntly if we don't care enough to fail the build, why do we care
> enough to run the test?
>
> Regarding homegrown solutions I see your point. In the case of a de facto
> standard like jUnit, I'd prefer to use their homegrown solution over ours
> in
> order to be more encouraging to new developers. That and we don't have to
> maintain the code ;-)
>
> -mike
>
> On Mon, Apr 6, 2009 at 12:15 PM, Pinaki Poddar <ppoddar@apache.org> wrote:
>
> >
> > The purpose of AllowFailure is *not* to ignore a test from running. But
> to
> > run the test and allow our test harness to continue even if the annotated
> > test fails. If a JUnit4 annotation provides us such functionality,
> perhaps
> > we can replace it. But remember that any solution is homegrown -- it is
> just
> > a different home than your own:)
> >
> >
> > Hi all,
> >
> > The AllowFailure annotation is very useful in that it allows specific
> test
> > methods to be ignored during a typical Maven build.
> >
> > The implementation, however is rather confusing as a "clean" build of
> > OpenJPA will typically contain several stack traces from exceptions.
> > Running
> > these optional tests that currently do not pass just consumes CPU cycles
> > that could be better spent elsewhere.
> >
> > I propose refactoring @AllowFailure to be called @OptionalTest (or just
> > @Optional) and updating the supporting methods in PersistenceTestCase so
> > that test methods (or classes) annotated with @OptionalTest are skipped
> > unless a jvm system property is true (ie -Dopenjpa.optional.tests=true).
> >
> > I think this will save everyone's CPU cycles without violating the intent
> > of
> > @AllowFailure. In addition with this change we could resurrect the
> changes
> > for OPENJPA-770 and we could clean up the root pom.xml a bit.
> >
> > Anyone else have strong opinions about @AllowFailure?
> >
> > -mike
> >
> >
> >
> >
> > -----
> > Pinaki Poddar                      http://ppoddar.blogspot.com/
> >
> > http://www.linkedin.com/in/pinakipoddar
> > OpenJPA PMC Member/Committer
> > JPA Expert Group Member
> > --
> > View this message in context:
> >
> http://n2.nabble.com/-DISCUSS--refactor-%40AllowFailure-tp2593705p2594325.html
> > Sent from the OpenJPA Developers mailing list archive at Nabble.com.
> >
> >
>
>
>
>
> -----
> Pinaki Poddar                      http://ppoddar.blogspot.com/
>
> http://www.linkedin.com/in/pinakipoddar
> OpenJPA PMC Member/Committer
> JPA Expert Group Member
> --
>
View this message in context:
> http://n2.nabble.com/-DISCUSS--refactor-%40AllowFailure-tp2593705p2595933.html
>
> Sent from the OpenJPA Developers mailing list archive at Nabble.com.
>
>
>

--001636c5a57b6e62950466f5da1d--