openjpa-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Dick <michael.d.d...@gmail.com>
Subject Re: [DISCUSS] refactor @AllowFailure
Date Thu, 09 Apr 2009 16:58:36 GMT
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 <michael.d.dick@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 <dwoods@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
>>>>>
>>>>>
>>>>
>>>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message