You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@openjpa.apache.org by Michael Dick <mi...@gmail.com> on 2009/04/06 17:30:14 UTC
[DISCUSS] refactor @AllowFailure
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
Re: [DISCUSS] refactor @AllowFailure
Posted by Michael Dick <mi...@gmail.com>.
On Mon, Apr 6, 2009 at 5:49 PM, Pinaki Poddar <pp...@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 <pp...@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.
>
>
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Pinaki Poddar <pp...@apache.org>.
> 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.
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
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.
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 <pp...@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.
Re: [DISCUSS] refactor @AllowFailure
Posted by Michael Dick <mi...@gmail.com>.
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 <pp...@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.
>
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Pinaki Poddar <pp...@apache.org>.
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.
Re: [DISCUSS] refactor @AllowFailure
Posted by Miłosz Tylenda <mt...@o2.pl>.
+1 for giving JUnit 4 another try.
Milosz
> Edit, Assume is not an annotation, it is a method, sorry for the typo.
>
> -mike
>
> On Thu, Apr 9, 2009 at 11:46 AM, Michael Dick wrote:
>
> > Hi Donald,
> >
> > I think @Ignore is pretty much what I'm looking for for these excluded
> > tests. Whether we add a new annotation or rename @AllowFailure doesn't
> > matter much in the grand scheme of things. I think it's better to not run
> > the tests that we don't expect to work rather than running them and
> > displaying an error message (wastes CPU time, the developers' time
> > interpreting results, gives new developers a bad view of the project, etc.).
> >
> >
> > In addition we should be consistent about how we detect a testcase that
> > will or will not execute on a specific database. The @DatabasePlatform
> > annotation is a good first step in this direction, but it's a bit brittle -
> > if we have multiple JDBC drivers on the classpath it doesn't work as one
> > would expect. Moving @DatabasePlatform to SingleEM/SingleEMF TestCase is
> > another good improvement (checking the openjpa config at this time).
> >
> > I think a lot of our needs can be solved by upgrading to jUnit 4 and using
> > @Assume and @Ignore (in addition @BeforeClass and @AfterClass will be nice).
> > If we can't get to the bottom of the memory issues with jUnit 4 or if it
> > looks like it's likely to take a while I'd be in favor of adding our own
> > @Ignore.
> >
> > -mike
> >
> >
> > On Tue, Apr 7, 2009 at 10:50 AM, Donald Woods wrote:
> >
> >> Reviving the sub-discussion about @Ignore in OPENJPA-998...
> >>
> >> Currently, there are junit tests that use simple if() checks to skip
> >> running tests on certain DBs or when certain DBDictionary properties are not
> >> enabled. If we had a standard mechanism like @Ignore, we could use the
> >> ClassSelector.java (or Eclipse search, or ...) to easily discover which
> >> tests are not being run against all supported DBs.
> >>
> >>
> >> -Donald
> >>
> >>
> >>
> >> Donald Woods wrote:
> >>
> >>> Mike, after thinking about this some more, I guess there are 2 different
> >>> test needs here -
> >>> 1) allow some tests to be skipped due to a boolean condition (like if the
> >>> test is running on DB2), which OPENJPA-998 provides via the @Ignore
> >>> annotation
> >>> 2) convert the surefire excludes list in pom.xml into annotations, via an
> >>> @Optional or similar system property, as you suggest below and could use
> >>> OPENJPA-949 for the JIRA work.
> >>>
> >>> I support renaming @AllowFailure to something like @Optional which uses a
> >>> system property as the default activator. Besides the default all/true or
> >>> none/false conditions, we could also support a package/classname value like
> >>> -
> >>> -Dopenjpa.optional.tests=org.apache.openjpa.persistence.query.*
> >>> to allow running a subset of the normally excluded tests.
> >>>
> >>> I would also check the @Optional annotation before the @Ignore
> >>> annotation, so these non-normal tests will be skipped before the conditional
> >>> code (usually in setup()) is run for the @Ignore.
> >>>
> >>>
> >>> -Donald
> >>>
> >>>
> >>> Donald Woods wrote:
> >>>
> >>>> I would rather see us use a Junit v4 annotation like @Ignore as provided
> >>>> via OPENJPA-998 (which just needs someone to review and commit it...)
> >>>>
> >>>> /**
> >>>> * Signals to the harness to ignore the annotated test.
> >>>> *
> >>>> */
> >>>> @Target({TYPE, METHOD})
> >>>> @Retention(RUNTIME)
> >>>> public @interface Ignore {
> >>>> boolean value() default true;
> >>>> String message() default "";
> >>>> }
> >>>>
> >>>>
> >>>> After the above is added, the @AllowFailure support can be removed and
> >>>> the ClassSelector.java updated to look for @Ignore by default.
> >>>>
> >>>>
> >>>> -Donald
> >>>>
> >>>>
> >>>> Michael Dick wrote:
> >>>>
> >>>>> 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
> >>>>>
> >>>>>
> >>>>
> >>>
> >
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Michael Dick <mi...@gmail.com>.
Edit, Assume is not an annotation, it is a method, sorry for the typo.
-mike
On Thu, Apr 9, 2009 at 11:46 AM, Michael Dick <mi...@gmail.com>wrote:
> Hi Donald,
>
> I think @Ignore is pretty much what I'm looking for for these excluded
> tests. Whether we add a new annotation or rename @AllowFailure doesn't
> matter much in the grand scheme of things. I think it's better to not run
> the tests that we don't expect to work rather than running them and
> displaying an error message (wastes CPU time, the developers' time
> interpreting results, gives new developers a bad view of the project, etc.).
>
>
> In addition we should be consistent about how we detect a testcase that
> will or will not execute on a specific database. The @DatabasePlatform
> annotation is a good first step in this direction, but it's a bit brittle -
> if we have multiple JDBC drivers on the classpath it doesn't work as one
> would expect. Moving @DatabasePlatform to SingleEM/SingleEMF TestCase is
> another good improvement (checking the openjpa config at this time).
>
> I think a lot of our needs can be solved by upgrading to jUnit 4 and using
> @Assume and @Ignore (in addition @BeforeClass and @AfterClass will be nice).
> If we can't get to the bottom of the memory issues with jUnit 4 or if it
> looks like it's likely to take a while I'd be in favor of adding our own
> @Ignore.
>
> -mike
>
>
> On Tue, Apr 7, 2009 at 10:50 AM, Donald Woods <dw...@apache.org> wrote:
>
>> Reviving the sub-discussion about @Ignore in OPENJPA-998...
>>
>> Currently, there are junit tests that use simple if() checks to skip
>> running tests on certain DBs or when certain DBDictionary properties are not
>> enabled. If we had a standard mechanism like @Ignore, we could use the
>> ClassSelector.java (or Eclipse search, or ...) to easily discover which
>> tests are not being run against all supported DBs.
>>
>>
>> -Donald
>>
>>
>>
>> Donald Woods wrote:
>>
>>> Mike, after thinking about this some more, I guess there are 2 different
>>> test needs here -
>>> 1) allow some tests to be skipped due to a boolean condition (like if the
>>> test is running on DB2), which OPENJPA-998 provides via the @Ignore
>>> annotation
>>> 2) convert the surefire excludes list in pom.xml into annotations, via an
>>> @Optional or similar system property, as you suggest below and could use
>>> OPENJPA-949 for the JIRA work.
>>>
>>> I support renaming @AllowFailure to something like @Optional which uses a
>>> system property as the default activator. Besides the default all/true or
>>> none/false conditions, we could also support a package/classname value like
>>> -
>>> -Dopenjpa.optional.tests=org.apache.openjpa.persistence.query.*
>>> to allow running a subset of the normally excluded tests.
>>>
>>> I would also check the @Optional annotation before the @Ignore
>>> annotation, so these non-normal tests will be skipped before the conditional
>>> code (usually in setup()) is run for the @Ignore.
>>>
>>>
>>> -Donald
>>>
>>>
>>> Donald Woods wrote:
>>>
>>>> I would rather see us use a Junit v4 annotation like @Ignore as provided
>>>> via OPENJPA-998 (which just needs someone to review and commit it...)
>>>>
>>>> /**
>>>> * Signals to the harness to ignore the annotated test.
>>>> *
>>>> */
>>>> @Target({TYPE, METHOD})
>>>> @Retention(RUNTIME)
>>>> public @interface Ignore {
>>>> boolean value() default true;
>>>> String message() default "";
>>>> }
>>>>
>>>>
>>>> After the above is added, the @AllowFailure support can be removed and
>>>> the ClassSelector.java updated to look for @Ignore by default.
>>>>
>>>>
>>>> -Donald
>>>>
>>>>
>>>> Michael Dick wrote:
>>>>
>>>>> 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
>>>>>
>>>>>
>>>>
>>>
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Michael Dick <mi...@gmail.com>.
Hi Donald,
I think @Ignore is pretty much what I'm looking for for these excluded
tests. Whether we add a new annotation or rename @AllowFailure doesn't
matter much in the grand scheme of things. I think it's better to not run
the tests that we don't expect to work rather than running them and
displaying an error message (wastes CPU time, the developers' time
interpreting results, gives new developers a bad view of the project, etc.).
In addition we should be consistent about how we detect a testcase that will
or will not execute on a specific database. The @DatabasePlatform annotation
is a good first step in this direction, but it's a bit brittle - if we have
multiple JDBC drivers on the classpath it doesn't work as one would expect.
Moving @DatabasePlatform to SingleEM/SingleEMF TestCase is another good
improvement (checking the openjpa config at this time).
I think a lot of our needs can be solved by upgrading to jUnit 4 and using
@Assume and @Ignore (in addition @BeforeClass and @AfterClass will be nice).
If we can't get to the bottom of the memory issues with jUnit 4 or if it
looks like it's likely to take a while I'd be in favor of adding our own
@Ignore.
-mike
On Tue, Apr 7, 2009 at 10:50 AM, Donald Woods <dw...@apache.org> wrote:
> Reviving the sub-discussion about @Ignore in OPENJPA-998...
>
> Currently, there are junit tests that use simple if() checks to skip
> running tests on certain DBs or when certain DBDictionary properties are not
> enabled. If we had a standard mechanism like @Ignore, we could use the
> ClassSelector.java (or Eclipse search, or ...) to easily discover which
> tests are not being run against all supported DBs.
>
>
> -Donald
>
>
>
> Donald Woods wrote:
>
>> Mike, after thinking about this some more, I guess there are 2 different
>> test needs here -
>> 1) allow some tests to be skipped due to a boolean condition (like if the
>> test is running on DB2), which OPENJPA-998 provides via the @Ignore
>> annotation
>> 2) convert the surefire excludes list in pom.xml into annotations, via an
>> @Optional or similar system property, as you suggest below and could use
>> OPENJPA-949 for the JIRA work.
>>
>> I support renaming @AllowFailure to something like @Optional which uses a
>> system property as the default activator. Besides the default all/true or
>> none/false conditions, we could also support a package/classname value like
>> -
>> -Dopenjpa.optional.tests=org.apache.openjpa.persistence.query.*
>> to allow running a subset of the normally excluded tests.
>>
>> I would also check the @Optional annotation before the @Ignore annotation,
>> so these non-normal tests will be skipped before the conditional code
>> (usually in setup()) is run for the @Ignore.
>>
>>
>> -Donald
>>
>>
>> Donald Woods wrote:
>>
>>> I would rather see us use a Junit v4 annotation like @Ignore as provided
>>> via OPENJPA-998 (which just needs someone to review and commit it...)
>>>
>>> /**
>>> * Signals to the harness to ignore the annotated test.
>>> *
>>> */
>>> @Target({TYPE, METHOD})
>>> @Retention(RUNTIME)
>>> public @interface Ignore {
>>> boolean value() default true;
>>> String message() default "";
>>> }
>>>
>>>
>>> After the above is added, the @AllowFailure support can be removed and
>>> the ClassSelector.java updated to look for @Ignore by default.
>>>
>>>
>>> -Donald
>>>
>>>
>>> Michael Dick wrote:
>>>
>>>> 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
>>>>
>>>>
>>>
>>
Re: [DISCUSS] refactor @AllowFailure
Posted by Donald Woods <dw...@apache.org>.
Reviving the sub-discussion about @Ignore in OPENJPA-998...
Currently, there are junit tests that use simple if() checks to skip
running tests on certain DBs or when certain DBDictionary properties are
not enabled. If we had a standard mechanism like @Ignore, we could use
the ClassSelector.java (or Eclipse search, or ...) to easily discover
which tests are not being run against all supported DBs.
-Donald
Donald Woods wrote:
> Mike, after thinking about this some more, I guess there are 2 different
> test needs here -
> 1) allow some tests to be skipped due to a boolean condition (like if
> the test is running on DB2), which OPENJPA-998 provides via the @Ignore
> annotation
> 2) convert the surefire excludes list in pom.xml into annotations, via
> an @Optional or similar system property, as you suggest below and could
> use OPENJPA-949 for the JIRA work.
>
> I support renaming @AllowFailure to something like @Optional which uses
> a system property as the default activator. Besides the default
> all/true or none/false conditions, we could also support a
> package/classname value like -
> -Dopenjpa.optional.tests=org.apache.openjpa.persistence.query.*
> to allow running a subset of the normally excluded tests.
>
> I would also check the @Optional annotation before the @Ignore
> annotation, so these non-normal tests will be skipped before the
> conditional code (usually in setup()) is run for the @Ignore.
>
>
> -Donald
>
>
> Donald Woods wrote:
>> I would rather see us use a Junit v4 annotation like @Ignore as
>> provided via OPENJPA-998 (which just needs someone to review and
>> commit it...)
>>
>> /**
>> * Signals to the harness to ignore the annotated test.
>> *
>> */
>> @Target({TYPE, METHOD})
>> @Retention(RUNTIME)
>> public @interface Ignore {
>> boolean value() default true;
>> String message() default "";
>> }
>>
>>
>> After the above is added, the @AllowFailure support can be removed and
>> the ClassSelector.java updated to look for @Ignore by default.
>>
>>
>> -Donald
>>
>>
>> Michael Dick wrote:
>>> 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
>>>
>>
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Donald Woods <dw...@apache.org>.
Mike, after thinking about this some more, I guess there are 2 different
test needs here -
1) allow some tests to be skipped due to a boolean condition (like if
the test is running on DB2), which OPENJPA-998 provides via the @Ignore
annotation
2) convert the surefire excludes list in pom.xml into annotations, via
an @Optional or similar system property, as you suggest below and could
use OPENJPA-949 for the JIRA work.
I support renaming @AllowFailure to something like @Optional which uses
a system property as the default activator. Besides the default
all/true or none/false conditions, we could also support a
package/classname value like -
-Dopenjpa.optional.tests=org.apache.openjpa.persistence.query.*
to allow running a subset of the normally excluded tests.
I would also check the @Optional annotation before the @Ignore
annotation, so these non-normal tests will be skipped before the
conditional code (usually in setup()) is run for the @Ignore.
-Donald
Donald Woods wrote:
> I would rather see us use a Junit v4 annotation like @Ignore as provided
> via OPENJPA-998 (which just needs someone to review and commit it...)
>
> /**
> * Signals to the harness to ignore the annotated test.
> *
> */
> @Target({TYPE, METHOD})
> @Retention(RUNTIME)
> public @interface Ignore {
> boolean value() default true;
> String message() default "";
> }
>
>
> After the above is added, the @AllowFailure support can be removed and
> the ClassSelector.java updated to look for @Ignore by default.
>
>
> -Donald
>
>
> Michael Dick wrote:
>> 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
>>
>
Re: [DISCUSS] refactor @AllowFailure
Posted by Michael Dick <mi...@gmail.com>.
Sorry Donald, I missed that issue.
I'd prefer the jUnit 4 annotation over any homegrown solution. The catch
with jUnit 4 is that (at least in my experience) it quadruples the memory
requirement to run the tests as is. Setting test.jvm.arguments=-Xmx2G
probably isn't viable for most devs (tried with 768M, 1G, and 2G, only the
latter worked for me).
Regarding @Ignore, I think it should replace @AllowFailure instead of adding
two annotations that do nearly the same thing. Will take a closer look
tonight, time permitting.
-mike
On Mon, Apr 6, 2009 at 11:23 AM, Donald Woods <dw...@apache.org> wrote:
> I would rather see us use a Junit v4 annotation like @Ignore as provided
> via OPENJPA-998 (which just needs someone to review and commit it...)
>
> /**
> * Signals to the harness to ignore the annotated test.
> *
> */
> @Target({TYPE, METHOD})
> @Retention(RUNTIME)f
> public @interface Ignore {r
> boolean value() default true;
> String message() default "";
> }
>
>
> After the above is added, the @AllowFailure support can be removed and the
> ClassSelector.java updated to look for @Ignore by default.
>
>
> -Donald
>
>
>
> Michael Dick wrote:
>
>> 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
>>
>>
Re: [DISCUSS] refactor @AllowFailure
Posted by Donald Woods <dw...@apache.org>.
I would rather see us use a Junit v4 annotation like @Ignore as provided
via OPENJPA-998 (which just needs someone to review and commit it...)
/**
* Signals to the harness to ignore the annotated test.
*
*/
@Target({TYPE, METHOD})
@Retention(RUNTIME)
public @interface Ignore {
boolean value() default true;
String message() default "";
}
After the above is added, the @AllowFailure support can be removed and
the ClassSelector.java updated to look for @Ignore by default.
-Donald
Michael Dick wrote:
> 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
>