You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@harmony.apache.org by Mark Hindess <ma...@googlemail.com> on 2010/05/26 11:59:18 UTC
test excludes (was [proposal] For future milestones, expect zero test failures)
In message <20...@d06av01.portsmouth.uk.ibm.com>,
Mark Hindess writes:
>
> In message <AA...@mail.gmail.com>,
> Nathan Beyer writes:
> >
> > For future milestone votes, I'd like to propose that we set the
> > expectation of zero test failures. Every time we do a milestone and I
> > run the build and tests I find myself looking at the test failures
> > (there are ALWAYS failures) and ask 'are these expected'? If the
> > failures are expected, they should be excluded and the tests should
> > just pass.
> >
> > If this proposal is workable, immediately after this upcoming release,
> > all 'expected' failures will be pulled into the exclusion lists.
>
> I was thinking this too. The problem for me is that:
>
> 1) Our test coverage isn't brilliant so we need a way to exclude
> individual tests not the tests for a whole class. We have too many
> excluded tests already[0].
>
> 2) The JDWP test framework seems to pass most of the time on the 5.0
> code base but seems to fail quite often on the 6.0 branch. I normally
> resort to doing 10 test runs and comparing the results of all runs. I
> typically see every test pass on at least one run. This leads me to
> believe that the improved jdwp in the 6.0 branch is merely exposing race
> conditions in the test framework. It would be great to fix the test
> framework so that these are avoided. If we tried to exclude tests that
> fail regularly on java6 jdwp we would probably end up removing most of
> the tests. ;-(
>
> I think solving 1 is really a pre-requisite to moving forward with your
> (excellent) goal. We should make that a priority after this release. I
> don't care if the solution is something fancy with junit 4 annotations
> or something simple like adding:
>
> if (Support_Excludes.isExcluded()) {
> return;
> }
>
> to problematic tests where the support class just looks up the
> class/method name of the caller in the exclude lists. (I actually
> quite like the idea of doing the exclude list processing lazily at
> run time rather than generating the list with ant.)
Last night I decided to try to implement this simple exclude list
mechanism in my build-improvement branch. I've checked in my changes at
https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
I've automatically added isExcluded() checks to all the test methods
in previously excluded tests (though this was automated and more
complicated than you might think, and definitely not 100% accurate
but should be close enough). The next job would be to remove/move
the isExcluded() checks to be as fine-grained as possible so as just
to exclude the failing tests (or individual asserts). As I commented
previously many of the excludes probably just need to be removed
completely.
New excludes can be added as before by adding tests names to the lists
but you also need to add an appropriate isExcluded()-if check to the
test code as well. Hopefully this will maximize coverage will still
allowing us to have only passing tests running by default.
If different exclude conditions apply to different platforms you can
use:
package.class#methodname
package.class+arbitrary_tag
package.class#methodname+arbitrary_tag
arbitrary_tag
to exclude tests - though I'd avoid the final one unless it is something
really broad like a VM not supporting concurrent (like the IBM v4 VMEs).
To see what is happening, you can run the tests with:
-Dhy.test.vmargs=-Dhy.excludes.verbose=true
(though this is too verbose I might make the load method less verbose
or use a hy.excludes.debug=true flag for that output).
To ignore excludes and run all tests use:
-Dhy.test.vmargs=-Dhy.excludes.ignore=true
Rather than using test-jre-vm-info in ant the vm name for the exclude
file selection is a trivial guess based on system properties. If you
are using a custom vm and exclude lists then you can use:
-Dhy.test.vmargs=-Dhy.excludes.vm=myvm
to set it explicitly.
I'm tempted to do away with the isExcluded() checks/lists are replace
them with more readable:
if (Support_Excludes.isRI()) {
or:
if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
etc. To make the Excludes more self-contained but that requires
looking at the excluded tests in more detail to understand the
requirements better and as I said I wanted to make this a simple
step from what we had today.
I'd like to merge this to trunk/java6 after the code freeze but I'd
like some feedback first. I don't necessarily see this as a final
solution but I just wanted to do something since this topic has been
discussed for far to long and I wanted something that we could move to
from where we are today without to much effort. On the other hand,
I'm not convinced that annotations are a good solution either since
they don't give you fine-grained control so every distinction has to be
represented by a separate method.
Comments very welcome.
Regards,
Mark.
Re: test excludes
Posted by Nathan Beyer <nd...@apache.org>.
On Fri, May 28, 2010 at 8:57 AM, Mark Hindess
<ma...@googlemail.com> wrote:
>
> In message <AA...@mail.gmail.com>,
> Nathan Beyer writes:
>>
>> On Thu, May 27, 2010 at 9:12 AM, Mark Hindess
>> <ma...@googlemail.com> wrote:
>> >
>> > In message <AA...@mail.gmail.com>,
>> > Nathan Beyer writes:
>> >>
>> >> On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
>> >> <ma...@googlemail.com> wrote:
>> >> >
>> >> >
>> >> > Last night I decided to try to implement this simple exclude list
>> >> > mechanism in my build-improvement branch. I've checked in my
>> >> > changes at
>> >> > https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
>> >>
>> >> I don't want to be rude,
>> >
>> > Not at all; I don't think you've ever been rude on this list.
>> >
>> >> but ... I brought this up a few months back and the response I
>> >> got was less than overwhelming. I stopped my work. How is this
>> >> different?
>> >>
>> >> http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=3Djunit+annotation
>> >
>> > I think the response you got was fairly positive. The only issue
>> > appears to be a request from Jesse for some way to externally
>> > influence the test selection process which I don't think was
>> > tackled.
>> >
>> > How far did you get? I'd be interested in playing with the
>> > annotations if you've written them. I'm not convinced annotations
>> > are enough but they probably will be part of the right longer term
>> > solution.
>>
>> What I have now is -
>> * Platform - an annotation that can mark a class and/or method with a
>> os, arch and vm value
>> * PlatformRule - a org.junit.rules.MethodRule implementation, which
>> is instantiated and stored as an instance variable in unit tests that
>> use the Platform annotation; when a test with this rule is run, if the
>> class or method is annotated with Platform, only the test methods that
>> match all qualifications will be run; methods not run will be logged
>> to standard error.
>
> It would be nice to be able to do things like:
>
> @Exclude() // for exclude.{common,intern} tests
> @Exclude(os=linux) // for tests that fail on any linux platform
>
> but to start with so long as we can have multiple @Exclude annotation
> this wont be an urgent problem to solve.
>
>> This was my experiment for using annotations and method rules. The
>> next step would be to define some annotations for the test
>> qualifications we wanted to express - Exclude(os, arch, vm), etc. And
>> then write method rules that would massage the run based on these
>> rules.
>
> I assume the Exclude annotation wont be a big leap from the Platform
> annotation. If you can provide this I can use a similar Perl one-liner
> to the one I used for the Support_Excludes changes in my branch to apply
> them to all methods in classes in the current exclude lists. We can
> then refine them after checking which methods really need to be excluded
> as we get time to go through them.
>
>> The interesting thing about method rules is that they can affect
>> the test runs without requiring any special runner, however, when a
>> method rule is used to not run a method, most runners will treat the
>> methods as run, even if the method rule doesn't actually execute the
>> 'statement'.
>
> Method rules would be good enough for now but having the top-level
> junit report showing more tests run even though they might have tested
> nothing is not ideal. So, I think longer term a runner will be
> necessary/useful. (I'd expect Jesse would override the runner with his
> own to get whatever external filtering he needs.)
Here's the issue -- the JUnit Ant Task doesn't provide much access to
the flexibility in JUnit4's APIs. Anything that gets into the realm of
a custom runner or customizing the run (filtering, etc) would require
a custom Ant task. If we're beholden to using the standard task, then
we're limited in what can be done. I think defining custom, semantic
annotations for the tests is the best approach for Harmony (both for
excludes, but also for includes -- i.e. run test for specific
platform), but to get them working we'll have to write, at a minimum,
a custom Ant task for running the tests.
-Nathan
>
> Regards,
> Mark.
>
>
>
Re: test excludes
Posted by Mark Hindess <ma...@googlemail.com>.
In message <AA...@mail.gmail.com>,
Nathan Beyer writes:
>
> On Thu, May 27, 2010 at 9:12 AM, Mark Hindess
> <ma...@googlemail.com> wrote:
> >
> > In message <AA...@mail.gmail.com>,
> > Nathan Beyer writes:
> >>
> >> On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
> >> <ma...@googlemail.com> wrote:
> >> >
> >> >
> >> > Last night I decided to try to implement this simple exclude list
> >> > mechanism in my build-improvement branch. I've checked in my
> >> > changes at
> >> > https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
> >>
> >> I don't want to be rude,
> >
> > Not at all; I don't think you've ever been rude on this list.
> >
> >> but ... I brought this up a few months back and the response I
> >> got was less than overwhelming. I stopped my work. How is this
> >> different?
> >>
> >> http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=3Djunit+annotation
> >
> > I think the response you got was fairly positive. The only issue
> > appears to be a request from Jesse for some way to externally
> > influence the test selection process which I don't think was
> > tackled.
> >
> > How far did you get? I'd be interested in playing with the
> > annotations if you've written them. I'm not convinced annotations
> > are enough but they probably will be part of the right longer term
> > solution.
>
> What I have now is -
> * Platform - an annotation that can mark a class and/or method with a
> os, arch and vm value
> * PlatformRule - a org.junit.rules.MethodRule implementation, which
> is instantiated and stored as an instance variable in unit tests that
> use the Platform annotation; when a test with this rule is run, if the
> class or method is annotated with Platform, only the test methods that
> match all qualifications will be run; methods not run will be logged
> to standard error.
It would be nice to be able to do things like:
@Exclude() // for exclude.{common,intern} tests
@Exclude(os=linux) // for tests that fail on any linux platform
but to start with so long as we can have multiple @Exclude annotation
this wont be an urgent problem to solve.
> This was my experiment for using annotations and method rules. The
> next step would be to define some annotations for the test
> qualifications we wanted to express - Exclude(os, arch, vm), etc. And
> then write method rules that would massage the run based on these
> rules.
I assume the Exclude annotation wont be a big leap from the Platform
annotation. If you can provide this I can use a similar Perl one-liner
to the one I used for the Support_Excludes changes in my branch to apply
them to all methods in classes in the current exclude lists. We can
then refine them after checking which methods really need to be excluded
as we get time to go through them.
> The interesting thing about method rules is that they can affect
> the test runs without requiring any special runner, however, when a
> method rule is used to not run a method, most runners will treat the
> methods as run, even if the method rule doesn't actually execute the
> 'statement'.
Method rules would be good enough for now but having the top-level
junit report showing more tests run even though they might have tested
nothing is not ideal. So, I think longer term a runner will be
necessary/useful. (I'd expect Jesse would override the runner with his
own to get whatever external filtering he needs.)
Regards,
Mark.
Re: test excludes
Posted by Nathan Beyer <nd...@apache.org>.
On Thu, May 27, 2010 at 9:12 AM, Mark Hindess
<ma...@googlemail.com> wrote:
>
> In message <AA...@mail.gmail.com>,
> Nathan Beyer writes:
>>
>> On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
>> <ma...@googlemail.com> wrote:
>> >
>> >
>> > Last night I decided to try to implement this simple exclude list
>> > mechanism in my build-improvement branch. I've checked in my changes at
>> > https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
>>
>> I don't want to be rude,
>
> Not at all; I don't think you've ever been rude on this list.
>
>> but ... I brought this up a few months back and the response I got was
>> less than overwhelming. I stopped my work.
>> How is this different?
>>
>> http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=junit+annotation
>
> I think the response you got was fairly positive. The only issue
> appears to be a request from Jesse for some way to externally influence
> the test selection process which I don't think was tackled.
>
> How far did you get? I'd be interested in playing with the annotations
> if you've written them. I'm not convinced annotations are enough but
> they probably will be part of the right longer term solution.
What I have now is -
* Platform - an annotation that can mark a class and/or method with a
os, arch and vm value
* PlatformRule - a org.junit.rules.MethodRule implementation, which is
instantiated and stored as an instance variable in unit tests that use
the Platform annotation; when a test with this rule is run, if the
class or method is annotated with Platform, only the test methods that
match all qualifications will be run; methods not run will be logged
to standard error.
This was my experiment for using annotations and method rules. The
next step would be to define some annotations for the test
qualifications we wanted to express - Exclude(os, arch, vm), etc. And
then write method rules that would massage the run based on these
rules.
The interesting thing about method rules is that they can affect the
test runs without requiring any special runner, however, when a method
rule is used to not run a method, most runners will treat the methods
as run, even if the method rule doesn't actually execute the
'statement'.
-Nathan
>
> I don't see what I have done as a long term solution - and tried to make
> it clear that I didn't. My motivation has been documented elsewhere in
> this thread - particularly, in my reply to Tim's equally blunt
> response! ;-) I also don't see how what I have proposed doing in the
> short term ... in particular ...
>
>> > [snip]
>> >
>> > I'm tempted to do away with the isExcluded() checks/lists are replace
>> > them with more readable:
>> >
>> > if (Support_Excludes.isRI()) {
>> >
>> > or:
>> >
>> > if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
>> >
>> > etc. To make the Excludes more self-contained but that requires
>> > looking at the excluded tests in more detail to understand the
>> > requirements better and as I said I wanted to make this a simple
>> > step from what we had today.
>
> ... prevents doing something better when a solution is decided upon.
> In fact, if the tests are modified like this it will make writing the
> correct annotations a lot quicker.
>
>> > I'd like to merge this to trunk/java6 after the code freeze but I'd
>> > like some feedback first. I don't necessarily see this as a final
>> > solution but I just wanted to do something since this topic has been
>> > discussed for far to long and I wanted something that we could move
>> > to from where we are today without to much effort. On the other
>> > hand, I'm not convinced that annotations are a good solution either
>> > since they don't give you fine-grained control so every distinction
>> > has to be represented by a separate method.
>
> In my response to Sebb, I mentioned a specific example of a single
> assert which fails on the RI. I'd like to think that we can find a
> better solution to this example than duplicating the method.
>
> In short, when you have some annotations I can use then I will certainly
> use them to mark up the tests. But I am impatient to get more tests
> running right now so I would like to continue to do something about that
> in the meantime.
>
> I've not done the merge from my branch anyway since it turns out I can
> get more tests running just by re-testing and un-excluding whole test so
> I am concentrating on that first. By the time I've done that perhaps
> your annotations will be ready and I can use those in trunk instead?
>
> Regards,
> Mark.
>
>
>
Re: test excludes
Posted by Mark Hindess <ma...@googlemail.com>.
In message <AA...@mail.gmail.com>,
Nathan Beyer writes:
>
> On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
> <ma...@googlemail.com> wrote:
> >
> >
> > Last night I decided to try to implement this simple exclude list
> > mechanism in my build-improvement branch. I've checked in my changes at
> > https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
>
> I don't want to be rude,
Not at all; I don't think you've ever been rude on this list.
> but ... I brought this up a few months back and the response I got was
> less than overwhelming. I stopped my work.
> How is this different?
>
> http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=junit+annotation
I think the response you got was fairly positive. The only issue
appears to be a request from Jesse for some way to externally influence
the test selection process which I don't think was tackled.
How far did you get? I'd be interested in playing with the annotations
if you've written them. I'm not convinced annotations are enough but
they probably will be part of the right longer term solution.
I don't see what I have done as a long term solution - and tried to make
it clear that I didn't. My motivation has been documented elsewhere in
this thread - particularly, in my reply to Tim's equally blunt
response! ;-) I also don't see how what I have proposed doing in the
short term ... in particular ...
> > [snip]
> >
> > I'm tempted to do away with the isExcluded() checks/lists are replace
> > them with more readable:
> >
> > if (Support_Excludes.isRI()) {
> >
> > or:
> >
> > if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
> >
> > etc. To make the Excludes more self-contained but that requires
> > looking at the excluded tests in more detail to understand the
> > requirements better and as I said I wanted to make this a simple
> > step from what we had today.
... prevents doing something better when a solution is decided upon.
In fact, if the tests are modified like this it will make writing the
correct annotations a lot quicker.
> > I'd like to merge this to trunk/java6 after the code freeze but I'd
> > like some feedback first. I don't necessarily see this as a final
> > solution but I just wanted to do something since this topic has been
> > discussed for far to long and I wanted something that we could move
> > to from where we are today without to much effort. On the other
> > hand, I'm not convinced that annotations are a good solution either
> > since they don't give you fine-grained control so every distinction
> > has to be represented by a separate method.
In my response to Sebb, I mentioned a specific example of a single
assert which fails on the RI. I'd like to think that we can find a
better solution to this example than duplicating the method.
In short, when you have some annotations I can use then I will certainly
use them to mark up the tests. But I am impatient to get more tests
running right now so I would like to continue to do something about that
in the meantime.
I've not done the merge from my branch anyway since it turns out I can
get more tests running just by re-testing and un-excluding whole test so
I am concentrating on that first. By the time I've done that perhaps
your annotations will be ready and I can use those in trunk instead?
Regards,
Mark.
Re: test excludes (was [proposal] For future milestones, expect zero
test failures)
Posted by Alexei Fedotov <al...@gmail.com>.
I wonder why the very clear thing like "zero test failures" always ends up
with test architecture discussion. :-)
--
With best regards / с наилучшими пожеланиями,
Alexei Fedotov / Алексей Федотов,
http://www.telecom-express.ru/
http://harmony.apache.org/
http://dataved.ru/ http://klsh.ru/
On Thu, May 27, 2010 at 5:22 AM, Nathan Beyer <nd...@apache.org> wrote:
> On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
> <ma...@googlemail.com> wrote:
> >
> > In message <20...@d06av01.portsmouth.uk.ibm.com>,
> > Mark Hindess writes:
> >>
> >> In message <
> AANLkTikh3nV56QB8HXSXfzLnI9jrtU-C2cRF1shElZVt@mail.gmail.com>,
> >> Nathan Beyer writes:
> >> >
> >> > For future milestone votes, I'd like to propose that we set the
> >> > expectation of zero test failures. Every time we do a milestone and I
> >> > run the build and tests I find myself looking at the test failures
> >> > (there are ALWAYS failures) and ask 'are these expected'? If the
> >> > failures are expected, they should be excluded and the tests should
> >> > just pass.
> >> >
> >> > If this proposal is workable, immediately after this upcoming release,
> >> > all 'expected' failures will be pulled into the exclusion lists.
> >>
> >> I was thinking this too. The problem for me is that:
> >>
> >> 1) Our test coverage isn't brilliant so we need a way to exclude
> >> individual tests not the tests for a whole class. We have too many
> >> excluded tests already[0].
> >>
> >> 2) The JDWP test framework seems to pass most of the time on the 5.0
> >> code base but seems to fail quite often on the 6.0 branch. I normally
> >> resort to doing 10 test runs and comparing the results of all runs. I
> >> typically see every test pass on at least one run. This leads me to
> >> believe that the improved jdwp in the 6.0 branch is merely exposing race
> >> conditions in the test framework. It would be great to fix the test
> >> framework so that these are avoided. If we tried to exclude tests that
> >> fail regularly on java6 jdwp we would probably end up removing most of
> >> the tests. ;-(
> >>
> >> I think solving 1 is really a pre-requisite to moving forward with your
> >> (excellent) goal. We should make that a priority after this release. I
> >> don't care if the solution is something fancy with junit 4 annotations
> >> or something simple like adding:
> >>
> >> if (Support_Excludes.isExcluded()) {
> >> return;
> >> }
> >>
> >> to problematic tests where the support class just looks up the
> >> class/method name of the caller in the exclude lists. (I actually
> >> quite like the idea of doing the exclude list processing lazily at
> >> run time rather than generating the list with ant.)
> >
> > Last night I decided to try to implement this simple exclude list
> > mechanism in my build-improvement branch. I've checked in my changes at
> >
> https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
>
> I don't want to be rude, but ... I brought this up a few months back
> and the response I got was less than overwhelming. I stopped my work.
> How is this different?
>
> http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=junit+annotation
>
> >
> > I've automatically added isExcluded() checks to all the test methods
> > in previously excluded tests (though this was automated and more
> > complicated than you might think, and definitely not 100% accurate
> > but should be close enough). The next job would be to remove/move
> > the isExcluded() checks to be as fine-grained as possible so as just
> > to exclude the failing tests (or individual asserts). As I commented
> > previously many of the excludes probably just need to be removed
> > completely.
> >
> > New excludes can be added as before by adding tests names to the lists
> > but you also need to add an appropriate isExcluded()-if check to the
> > test code as well. Hopefully this will maximize coverage will still
> > allowing us to have only passing tests running by default.
> >
> > If different exclude conditions apply to different platforms you can
> > use:
> >
> > package.class#methodname
> > package.class+arbitrary_tag
> > package.class#methodname+arbitrary_tag
> > arbitrary_tag
> >
> > to exclude tests - though I'd avoid the final one unless it is something
> > really broad like a VM not supporting concurrent (like the IBM v4 VMEs).
> >
> > To see what is happening, you can run the tests with:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.verbose=true
> >
> > (though this is too verbose I might make the load method less verbose
> > or use a hy.excludes.debug=true flag for that output).
> >
> > To ignore excludes and run all tests use:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.ignore=true
> >
> > Rather than using test-jre-vm-info in ant the vm name for the exclude
> > file selection is a trivial guess based on system properties. If you
> > are using a custom vm and exclude lists then you can use:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.vm=myvm
> >
> > to set it explicitly.
> >
> > I'm tempted to do away with the isExcluded() checks/lists are replace
> > them with more readable:
> >
> > if (Support_Excludes.isRI()) {
> >
> > or:
> >
> > if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
> >
> > etc. To make the Excludes more self-contained but that requires
> > looking at the excluded tests in more detail to understand the
> > requirements better and as I said I wanted to make this a simple
> > step from what we had today.
> >
> > I'd like to merge this to trunk/java6 after the code freeze but I'd
> > like some feedback first. I don't necessarily see this as a final
> > solution but I just wanted to do something since this topic has been
> > discussed for far to long and I wanted something that we could move to
> > from where we are today without to much effort. On the other hand,
> > I'm not convinced that annotations are a good solution either since
> > they don't give you fine-grained control so every distinction has to be
> > represented by a separate method.
> >
> > Comments very welcome.
> >
> > Regards,
> > Mark.
> >
> >
> >
>
Re: test excludes (was [proposal] For future milestones, expect zero
test failures)
Posted by Nathan Beyer <nd...@apache.org>.
On Wed, May 26, 2010 at 4:59 AM, Mark Hindess
<ma...@googlemail.com> wrote:
>
> In message <20...@d06av01.portsmouth.uk.ibm.com>,
> Mark Hindess writes:
>>
>> In message <AA...@mail.gmail.com>,
>> Nathan Beyer writes:
>> >
>> > For future milestone votes, I'd like to propose that we set the
>> > expectation of zero test failures. Every time we do a milestone and I
>> > run the build and tests I find myself looking at the test failures
>> > (there are ALWAYS failures) and ask 'are these expected'? If the
>> > failures are expected, they should be excluded and the tests should
>> > just pass.
>> >
>> > If this proposal is workable, immediately after this upcoming release,
>> > all 'expected' failures will be pulled into the exclusion lists.
>>
>> I was thinking this too. The problem for me is that:
>>
>> 1) Our test coverage isn't brilliant so we need a way to exclude
>> individual tests not the tests for a whole class. We have too many
>> excluded tests already[0].
>>
>> 2) The JDWP test framework seems to pass most of the time on the 5.0
>> code base but seems to fail quite often on the 6.0 branch. I normally
>> resort to doing 10 test runs and comparing the results of all runs. I
>> typically see every test pass on at least one run. This leads me to
>> believe that the improved jdwp in the 6.0 branch is merely exposing race
>> conditions in the test framework. It would be great to fix the test
>> framework so that these are avoided. If we tried to exclude tests that
>> fail regularly on java6 jdwp we would probably end up removing most of
>> the tests. ;-(
>>
>> I think solving 1 is really a pre-requisite to moving forward with your
>> (excellent) goal. We should make that a priority after this release. I
>> don't care if the solution is something fancy with junit 4 annotations
>> or something simple like adding:
>>
>> if (Support_Excludes.isExcluded()) {
>> return;
>> }
>>
>> to problematic tests where the support class just looks up the
>> class/method name of the caller in the exclude lists. (I actually
>> quite like the idea of doing the exclude list processing lazily at
>> run time rather than generating the list with ant.)
>
> Last night I decided to try to implement this simple exclude list
> mechanism in my build-improvement branch. I've checked in my changes at
> https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
I don't want to be rude, but ... I brought this up a few months back
and the response I got was less than overwhelming. I stopped my work.
How is this different?
http://harmony.markmail.org/message/4rhomwqjdr5uklln?q=junit+annotation
>
> I've automatically added isExcluded() checks to all the test methods
> in previously excluded tests (though this was automated and more
> complicated than you might think, and definitely not 100% accurate
> but should be close enough). The next job would be to remove/move
> the isExcluded() checks to be as fine-grained as possible so as just
> to exclude the failing tests (or individual asserts). As I commented
> previously many of the excludes probably just need to be removed
> completely.
>
> New excludes can be added as before by adding tests names to the lists
> but you also need to add an appropriate isExcluded()-if check to the
> test code as well. Hopefully this will maximize coverage will still
> allowing us to have only passing tests running by default.
>
> If different exclude conditions apply to different platforms you can
> use:
>
> package.class#methodname
> package.class+arbitrary_tag
> package.class#methodname+arbitrary_tag
> arbitrary_tag
>
> to exclude tests - though I'd avoid the final one unless it is something
> really broad like a VM not supporting concurrent (like the IBM v4 VMEs).
>
> To see what is happening, you can run the tests with:
>
> -Dhy.test.vmargs=-Dhy.excludes.verbose=true
>
> (though this is too verbose I might make the load method less verbose
> or use a hy.excludes.debug=true flag for that output).
>
> To ignore excludes and run all tests use:
>
> -Dhy.test.vmargs=-Dhy.excludes.ignore=true
>
> Rather than using test-jre-vm-info in ant the vm name for the exclude
> file selection is a trivial guess based on system properties. If you
> are using a custom vm and exclude lists then you can use:
>
> -Dhy.test.vmargs=-Dhy.excludes.vm=myvm
>
> to set it explicitly.
>
> I'm tempted to do away with the isExcluded() checks/lists are replace
> them with more readable:
>
> if (Support_Excludes.isRI()) {
>
> or:
>
> if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
>
> etc. To make the Excludes more self-contained but that requires
> looking at the excluded tests in more detail to understand the
> requirements better and as I said I wanted to make this a simple
> step from what we had today.
>
> I'd like to merge this to trunk/java6 after the code freeze but I'd
> like some feedback first. I don't necessarily see this as a final
> solution but I just wanted to do something since this topic has been
> discussed for far to long and I wanted something that we could move to
> from where we are today without to much effort. On the other hand,
> I'm not convinced that annotations are a good solution either since
> they don't give you fine-grained control so every distinction has to be
> represented by a separate method.
>
> Comments very welcome.
>
> Regards,
> Mark.
>
>
>
Re: test excludes
Posted by Mark Hindess <ma...@googlemail.com>.
In message <4B...@gmail.com>, Tim Ellison writes:
>
> Ah the good old unit testing discussion -- it must be release time again :-)
>
> (more below)
>
> On 26/May/2010 10:59, Mark Hindess wrote:
> > In message <20...@d06av01.portsmouth.uk.ibm.com>,
> > Mark Hindess writes:
> >> In message <AA...@mail.gmail.com>,
> >> Nathan Beyer writes:
> >>> For future milestone votes, I'd like to propose that we set the
> >>> expectation of zero test failures. Every time we do a milestone and I
> >>> run the build and tests I find myself looking at the test failures
> >>> (there are ALWAYS failures) and ask 'are these expected'? If the
> >>> failures are expected, they should be excluded and the tests should
> >>> just pass.
> >>>
> >>> If this proposal is workable, immediately after this upcoming release,
> >>> all 'expected' failures will be pulled into the exclusion lists.
> >> I was thinking this too. The problem for me is that:
> >>
> >> 1) Our test coverage isn't brilliant so we need a way to exclude
> >> individual tests not the tests for a whole class. We have too many
> >> excluded tests already[0].
>
> I agree that excluding whole classes is too broad.
See my reply to Sebb. I think excluding whole methods is going to be too
broad and will make it hard to maintain the tests.
> >> 2) The JDWP test framework seems to pass most of the time on the 5.0
> >> code base but seems to fail quite often on the 6.0 branch. I normally
> >> resort to doing 10 test runs and comparing the results of all runs. I
> >> typically see every test pass on at least one run. This leads me to
> >> believe that the improved jdwp in the 6.0 branch is merely exposing race
> >> conditions in the test framework. It would be great to fix the test
> >> framework so that these are avoided. If we tried to exclude tests that
> >> fail regularly on java6 jdwp we would probably end up removing most of
> >> the tests. ;-(
> >>
> >> I think solving 1 is really a pre-requisite to moving forward with your
> >> (excellent) goal. We should make that a priority after this release. I
> >> don't care if the solution is something fancy with junit 4 annotations
> >> or something simple like adding:
> >>
> >> if (Support_Excludes.isExcluded()) {
> >> return;
> >> }
> >>
> >> to problematic tests where the support class just looks up the
> >> class/method name of the caller in the exclude lists. (I actually
> >> quite like the idea of doing the exclude list processing lazily at
> >> run time rather than generating the list with ant.)
> >
> > Last night I decided to try to implement this simple exclude list
> > mechanism in my build-improvement branch. I've checked in my changes at
> > https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
>
> I've not reviewed the changes, but notice there were a lot of
> modifications required to the test cases themselves.
Only because I marked up each test method rather than testing them and
marking up only those that are really broken. This means we exclude the
same tests as before and can go through them as time allows marking up
the specific failures.
> > I've automatically added isExcluded() checks to all the test methods
> > in previously excluded tests (though this was automated and more
> > complicated than you might think, and definitely not 100% accurate
> > but should be close enough). The next job would be to remove/move
> > the isExcluded() checks to be as fine-grained as possible so as
> > just to exclude the failing tests (or individual asserts). As I
> > commented previously many of the excludes probably just need to be
> > removed completely.
> >
> > New excludes can be added as before by adding tests names to the
> > lists but you also need to add an appropriate isExcluded()-if
> > check to the test code as well. Hopefully this will maximize
> > coverage will still allowing us to have only passing tests running
> > by default.
>
> I apologize for having the 'same old discussion' on this topic, but I
> do believe that it is the role of the framework to decide which tests
> to run.
>
> Of course, right now there is only a simple selection based on test*()
> method name, and we have discussed previously the ways in which we
> can inject more information to help the framework decide if a test is
> valid or not. TestNG and JUnit4 have their own ideas about this too.
>
> Modifying the tests themselves to make them more granular would
> certainly help the framework separate out the tests we expect to
> pass from those we expect to fail -- having length tests written as
> a single test*() method doesn't really help since there is no way of
> knowing the dependencies throughout the test code.
Agreed. We definitely have tests that do too much in one method.
> Rather than go through the code and decide where to write the
> isExcluded() checks, can we split methods up to make them more readily
> exclude-able?
I don't think they are mutually exclusive. Having added isExcluded()
checks we know where we need to do work in order to move from what we
have today to using a framework - if we can agree on a framework and how
to use it. Evidence suggests that we can't decide and even if we could
it would take a quite a bit of effort to get any benefit.
As I said, I don't (necessarily) see this as an end result merely as a
way to move from where we are today to having enough information (in the
code) to help migrate if/when we do make a decision about where we want
to end up.
In the meantime, we get the almost immediate benefit of running more
tests. For example, I can change InetAddressTest.java (r948412) and get
25 more tests run on unix platforms than before. And it will be easier
to decide what needs to be done if/when we pick a framework.
> > If different exclude conditions apply to different platforms you can
> > use:
> >
> > package.class#methodname
> > package.class+arbitrary_tag
> > package.class#methodname+arbitrary_tag
> > arbitrary_tag
> >
> > to exclude tests - though I'd avoid the final one unless it is something
> > really broad like a VM not supporting concurrent (like the IBM v4 VMEs).
> >
> > To see what is happening, you can run the tests with:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.verbose=true
> >
> > (though this is too verbose I might make the load method less verbose
> > or use a hy.excludes.debug=true flag for that output).
> >
> > To ignore excludes and run all tests use:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.ignore=true
> >
> > Rather than using test-jre-vm-info in ant the vm name for the exclude
> > file selection is a trivial guess based on system properties. If you
> > are using a custom vm and exclude lists then you can use:
> >
> > -Dhy.test.vmargs=-Dhy.excludes.vm=myvm
> >
> > to set it explicitly.
> >
> > I'm tempted to do away with the isExcluded() checks/lists are replace
> > them with more readable:
> >
> > if (Support_Excludes.isRI()) {
> >
> > or:
> >
> > if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
> >
> > etc. To make the Excludes more self-contained but that requires
> > looking at the excluded tests in more detail to understand the
> > requirements better and as I said I wanted to make this a simple
> > step from what we had today.
> >
> > I'd like to merge this to trunk/java6 after the code freeze but I'd
> > like some feedback first. I don't necessarily see this as a final
> > solution but I just wanted to do something since this topic has been
> > discussed for far to long and I wanted something that we could move to
> > from where we are today without to much effort. On the other hand,
> > I'm not convinced that annotations are a good solution either since
> > they don't give you fine-grained control so every distinction has to be
> > represented by a separate method.
> >
> > Comments very welcome.
>
> My reaction is "yuk!" (well you did ask ;-) and I'd like to consider a
> more declarative approach, but you are writing the code and I don't have
> a concrete alternative to offer up. I have no /technical/ objection.
That's fine. I'm not saying I like it. I'm being pragmatic and trying
to do something which:
a) does no real harm
b) allows step-wise improvement
c) gets more tests running (quickly) and allows me (and anyone else wanting
to help) to easily see what needs to be worked on[1]
d) helps us identify what exclusion annotations we would need to use
a framework
> [ Perhaps it's simpler for me to just go fix the tests so they all pass
> then the question of how to do excludes right will become moot ;-) ]
Maybe. Please try! ;-)
You'll probably have trouble fixing the API tests to run on the RI though. ;-)
Regards,
Mark.
[0] find modules/*/src/test -name '*Test.java' -print|xargs fgrep -l Support_Excludes |xargs perl -ne '$c{$ARGV}++ if (/Support_Excludes\./); END { foreach (sort { $c{$b} <=> $c{$a} } keys %c) { printf "%6d %s\n", $c{$_}, $_ } }'
88 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLDocument_ReaderTest.java
84 modules/swing/src/test/api/java.injected/javax/swing/JTableTest.java
82 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicTreeUITest.java
80 modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/VetoableChangeSupportTest.java
75 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicTabbedPaneUITest.java
74 modules/swing/src/test/api/java.injected/javax/swing/text/html/StyleSheetTest.java
68 modules/swing/src/test/api/java.injected/javax/swing/JOptionPaneTest.java
62 modules/concurrent/src/test/java/ReentrantReadWriteLockTest.java
60 modules/concurrent/src/test/java/ScheduledExecutorTest.java
59 modules/awt/src/test/impl/boot/java/awt/geom/AffineTransformTest.java
56 modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/EventHandlerTest.java
55 modules/swing/src/test/api/java.injected/javax/swing/JInternalFrameTest.java
52 modules/beans/src/test/java/org/apache/harmony/beans/tests/java/beans/PersistenceDelegateTest.java
52 modules/swing/src/test/api/java.injected/javax/swing/JMenuTest.java
50 modules/concurrent/src/test/java/AbstractQueuedSynchronizerTest.java
49 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicInternalFrameUITest.java
47 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/DecimalFormatTest.java
47 modules/swing/src/test/api/java.injected/javax/swing/JListTest.java
47 modules/prefs/src/test/java/org/apache/harmony/prefs/tests/java/util/prefs/AbstractPreferencesTest.java
45 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicToolBarUITest.java
45 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/lang/ThreadTest.java
44 modules/concurrent/src/test/java/ReentrantLockTest.java
44 modules/swing/src/test/api/java.injected/javax/swing/SwingUtilitiesTest.java
44 modules/awt/src/test/impl/boot/java/awt/font/TextLayoutTest.java
41 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultEditorKit_Actions_MultithreadedTest.java
39 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicComboBoxUITest.java
39 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultCaretTest.java
38 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicOptionPaneUITest.java
36 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/ArrayListTest.java
35 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicInternalFrameTitlePaneTest.java
35 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponentTest.java
34 modules/awt/src/test/impl/boot/java/awt/geom/Line2DTest.java
33 modules/swing/src/test/api/java.injected/javax/swing/JEditorPaneTest.java
33 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicListUITest.java
33 modules/swing/src/test/api/java.injected/javax/swing/JFileChooserTest.java
32 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicComboPopupTest.java
32 modules/concurrent/src/test/java/ExecutorsTest.java
30 modules/lang-management/src/test/impl/java/org/apache/harmony/lang/management/ThreadInfoTest.java
30 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLDocument_InsertsTest.java
29 modules/swing/src/test/api/java.injected/javax/swing/JTextFieldTest.java
29 modules/awt/src/test/impl/boot/java/awt/geom/GeneralPathTest.java
28 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalIconFactoryTest.java
28 modules/swing/src/test/api/java.injected/javax/swing/JDialogTest.java
26 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_AccessibleJTextComponentTest.java
26 modules/swing/src/test/api/java.injected/javax/swing/text/html/StyleSheet_ResolvedRulesTest.java
26 modules/logging/src/test/java/org/apache/harmony/logging/tests/java/util/logging/LogManagerTest.java
25 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLEditorKitTest.java
25 modules/swing/src/test/api/java.injected/javax/swing/text/StringContentTest.java
25 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicSplitPaneUITest.java
25 modules/swing/src/test/api/java.injected/javax/swing/SpringLayoutTest.java
25 modules/swing/src/test/api/java.injected/javax/swing/border/TitledBorderTest.java
23 modules/swing/src/test/api/java.injected/javax/swing/JRootPaneTest.java
22 modules/swing/src/test/api/java.injected/javax/swing/text/StyleContextTest.java
22 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/DateFormatSymbolsTest.java
22 modules/swing/src/test/api/java.injected/javax/swing/JFrameTest.java
22 modules/swing/src/test/api/java.injected/javax/swing/text/FlowViewTest.java
22 modules/swing/src/test/api/java.injected/javax/swing/JViewportTest.java
22 modules/awt/src/test/impl/boot/java/awt/geom/CubicCurve2DTest.java
22 modules/swing/src/test/api/java.injected/javax/swing/text/CompositeView_VisualPositionTest.java
21 modules/swing/src/test/api/java.injected/javax/swing/JSpinnerTest.java
20 modules/swing/src/test/api/java.injected/javax/swing/event/AncestorEventTest.java
20 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/util/PropertiesTest.java
20 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/ManagementFactoryTest.java
20 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/SimpleDateFormatTest.java
20 modules/swing/src/test/api/java.injected/javax/swing/JSplitPaneTest.java
20 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLWriterTest.java
20 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLDocumentTest.java
20 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/net/MulticastSocketTest.java
19 modules/awt/src/test/impl/boot/java/awt/geom/Rectangle2DTest.java
19 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/MessageFormatTest.java
19 modules/swing/src/test/api/java.injected/javax/swing/text/UtilitiesTest.java
19 modules/swing/src/test/api/java.injected/javax/swing/text/html/InlineViewTest.java
19 modules/swing/src/test/api/java.injected/javax/swing/text/GapContentTest.java
18 modules/security/src/test/api/java/org/apache/harmony/security/tests/java/security/KeyStore2Test.java
18 modules/swing/src/test/api/java.injected/javax/swing/JWindowTest.java
18 modules/swing/src/test/api/java.injected/javax/swing/JFormattedTextFieldTest.java
18 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/ThreadMXBeanTest.java
17 modules/swing/src/test/api/java.injected/javax/swing/text/html/parser/DTDTest.java
17 modules/swing/src/test/api/java.injected/javax/swing/SortingFocusTraversalPolicyTest.java
17 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicFileChooserUITest.java
17 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/RuleBasedCollatorTest.java
17 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/MemoryPoolMXBeanTest.java
16 modules/awt/src/test/impl/boot/java/awt/geom/Arc2DTest.java
16 modules/awt/src/test/impl/boot/java/awt/geom/CubicCurve2DDoubleTest.java
16 modules/awt/src/test/impl/boot/java/awt/geom/CubicCurve2DFloatTest.java
15 modules/swing/src/test/api/java.injected/javax/swing/OverlayLayoutTest.java
15 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalToolBarUITest.java
15 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/RuntimeMXBeanTest.java
15 modules/archive/src/test/java/org/apache/harmony/archive/tests/java/util/jar/JarInputStreamTest.java
14 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicMenuUITest.java
14 modules/swing/src/test/api/java.injected/javax/swing/text/StringContentTest_CommonTest.java
14 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicMenuItemUITest.java
14 modules/swing/src/test/api/java.injected/javax/swing/border/EtchedBorderTest.java
14 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalFileChooserUITest.java
13 modules/swing/src/test/api/java.injected/javax/swing/border/LineBorderTest.java
13 modules/swing/src/test/api/java.injected/javax/swing/text/StyledEditorKitTest.java
13 modules/swing/src/test/api/java.injected/javax/swing/text/PlainViewI18N_ModelViewTest.java
13 modules/swing/src/test/api/java.injected/javax/swing/text/html/parser/ContentModelTest.java
13 modules/swing/src/test/api/java.injected/javax/swing/text/WrappedPlainView_SimpleTest.java
13 modules/swing/src/test/api/java.injected/javax/swing/RepaintManagerTest.java
12 modules/awt/src/test/impl/boot/java/awt/CardLayoutTest.java
12 modules/swing/src/test/api/java.injected/javax/swing/JDesktopPaneTest.java
12 modules/swing/src/test/api/java.injected/javax/swing/text/PlainView_ChangesTest.java
12 modules/instrument/src/test/java/org/apache/harmony/tests/java/lang/instrument/InstrumentTest.java
11 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicGraphicsUtilsTest.java
11 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalComboBoxUITest.java
11 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicProgressBarUITest.java
11 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicSplitPaneDividerTest.java
11 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultHighlighterTest.java
11 modules/awt/src/test/impl/boot/java/awt/GridLayoutTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/text/InternationalFormatterTest.java
10 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/lang/PackageTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/InputMapTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/text/html/FormViewTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/JCheckBoxMenuItemTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/text/html/FormView_FormInputElementTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/DefaultComboBoxModelTest.java
10 modules/swing/src/test/api/java.injected/javax/swing/text/PlainViewTest.java
10 modules/text/src/test/java/org/apache/harmony/text/tests/java/text/CollationElementIteratorTest.java
9 modules/swing/src/test/api/java.injected/javax/swing/LayoutFocusTraversalPolicyTest.java
9 modules/awt/src/test/impl/boot/java/awt/geom/AreaTest.java
9 modules/luni/src/test/impl/common/org/apache/harmony/luni/tests/internal/net/www/protocol/http/PersistenceTest.java
9 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_IMTest.java
9 modules/swing/src/test/api/java.injected/javax/swing/border/CompoundBorderTest.java
9 modules/swing/src/test/api/java.injected/javax/swing/Timer_MultithreadedTest.java
8 modules/swing/src/test/api/java.injected/javax/swing/text/AbstractDocument_AbstractElement_MASNoLockTest.java
8 modules/swing/src/test/api/java.injected/javax/swing/text/html/parser/DocumentParserTest.java
8 modules/swing/src/test/api/java.injected/javax/swing/DefaultCellEditorTest.java
8 modules/swing/src/test/api/java.injected/javax/swing/SpinnerDateModelTest.java
8 modules/swing/src/test/api/java.injected/javax/swing/text/html/StyleSheet_ViewAttributesTest.java
8 modules/awt/src/test/impl/boot/java/awt/font/LineBreakMeasurerTest.java
7 modules/awt/src/test/impl/boot/org/apache/harmony/awt/wtk/ShutdownWatchdogTest.java
7 modules/swing/src/test/api/java.injected/javax/swing/text/GlyphView_AttributesTest.java
7 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicTextAreaUITest.java
7 modules/swing/src/test/api/java.injected/javax/swing/text/PlainViewI18N_LineView_UpdateTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLEditorKit_HTMLTextActionTest.java
6 modules/awt/src/test/impl/boot/java/awt/font/TextMeasurerTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/text/View_VisualPosition_PartTest.java
6 modules/auth/src/test/java/common/org/apache/harmony/auth/tests/module/JndiLoginModuleTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalTreeUITest.java
6 modules/swing/src/test/api/java.injected/javax/swing/text/View_VisualPositionTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicColorChooserUITest.java
6 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicTextFieldUITest.java
6 modules/swing/src/test/api/java.injected/javax/swing/ProgressMonitorTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/text/WrappedPlainViewTest.java
6 modules/swing/src/test/api/java.injected/javax/swing/text/PasswordViewTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicDesktopPaneUIActionsTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/html/StyleSheet_ResolvedRulesClassTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/html/parser/TagElementTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_byAuxiliaryComponentTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicToolBarUI$DragWindowTest.java
5 modules/awt/src/test/impl/boot/java/awt/image/BufferedImageFilterTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/html/HTMLEditorKit_InsertHTMLTextActionTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/View_NextVisualPositionFromTest.java
5 modules/archive/src/test/java/org/apache/harmony/archive/tests/java/util/zip/GZIPInputStreamTest.java
5 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalRootPaneUITest.java
5 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultEditorKitRTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/html/ParagraphView_RequirementsTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/colorchooser/ColorChooserComponentFactoryTest.java
4 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/LoggingMXBeanTest.java
4 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/MemoryMXBeanTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicCheckBoxMenuItemUITest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/html/FormView_FormSelectElementTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicTextPaneUITest.java
4 modules/awt/src/test/impl/boot/java/awt/ToolkitRTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/SwingUtilitiesRTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/JEditorPane_AccessibleJEditorPaneHTMLTest.java
4 modules/awt/src/test/impl/boot/java/awt/ButtonRTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_MultithreadedTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/WrappedPlainViewRTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/ProgressMonitorInputStreamTest.java
4 modules/awt/src/test/impl/boot/org/apache/harmony/awt/tests/nativebridge/PointerPointerTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/GapContent_InternalTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultCaret_BidiTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicEditorPaneUITest.java
4 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalRootPaneUI$MetalRootLayoutTest.java
4 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/GarbageCollectorMXBeanTest.java
4 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicRootPaneUITest.java
4 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/ClassLoadingMXBeanTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalComboBoxIconTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/text/FieldViewTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicRadioButtonMenuItemUITest.java
3 modules/swing/src/test/api/java.injected/javax/swing/AccessibleJComponent_MultithreadedTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/JInternalFrame_MultithreadedTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_AccessibleJTextComponent_variousTextTest.java
3 modules/awt/src/test/impl/boot/java/awt/geom/RoundRectangle2DTest.java
3 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/net/ExcludedProxyTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/text/View_ForwardUpdateRTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/text/html/parser/DTD401Test.java
3 modules/lang-management/src/test/impl/java/org/apache/harmony/lang/management/MemoryPoolMXBeanImplTest.java
3 modules/swing/src/test/api/java.injected/javax/swing/JTextArea_MultithreadedTest.java
3 modules/awt/src/test/impl/boot/java/awt/WindowTest.java
3 modules/concurrent/src/test/java/ThreadLocalTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/JFileChooserRTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/text/TextAction_MultithreadedTest.java
2 modules/awt/src/test/impl/boot/java/awt/BasicStrokeTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultHighlighter_DefaultHighlightPainterTest.java
2 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/CompilationMXBeanTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/PopupFactoryTest.java
2 modules/awt/src/test/impl/boot/java/awt/ComponentTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/text/JTextComponent_IMLocationTest.java
2 modules/auth/src/test/java/common/org/apache/harmony/auth/tests/module/Krb5LoginModuleTest.java
2 modules/lang-management/src/test/api/java/org/apache/harmony/lang/management/tests/java/lang/management/MemoryManagerMXBeanTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/PopupTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/SortingFocusTraversalPolicyRTest.java
2 modules/awt/src/test/impl/unix/org/apache/harmony/awt/gl/linux/XGraphics2DRTest.java
2 modules/swing/src/test/api/java.injected/javax/swing/text/PlainViewI18N_LayoutTest.java
2 modules/rmi/src/test/api/java/org/apache/harmony/rmi/DGCTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/JFrameRTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/JRootPaneRTest.java
1 modules/awt/src/test/impl/boot/org/apache/harmony/awt/gl/MultiRectAreaLineCashTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/plaf/basic/BasicMenuUI_MultithreadedTest.java
1 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/net/InetAddressTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/plaf/metal/MetalRootPaneUIRTest.java
1 modules/awt/src/test/impl/boot/org/apache/harmony/awt/ListenerListTest.java
1 modules/jndi/src/test/java/org/apache/harmony/jndi/provider/ldap/LdapContextFactoryTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/JTextField_NotifyActionRTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/JOptionPaneRTest.java
1 modules/awt/src/test/impl/boot/java/awt/ChoiceTest.java
1 modules/awt/src/test/impl/boot/java/awt/ScrollPaneAdjustableTest.java
1 modules/luni/src/test/api/common/org/apache/harmony/luni/tests/java/lang/SecurityManager2Test.java
1 modules/print/src/test/api/java/common/javax/print/GetSupportedAttributeCategoriesTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/PopupFactoryRTest.java
1 modules/rmi/src/test/api/java/org/apache/harmony/rmi/tests/java/rmi/activation/StartupShutdownTest.java
1 modules/imageio/src/test/java/javax/imageio/ImageIOTest.java
1 modules/print/src/test/api/java/common/javax/print/GetDefaultAttributeValueTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/JFormattedTextField_CommitActionRTest.java
1 modules/print/src/test/api/java/common/javax/print/PrintTest.java
1 modules/swing/src/test/api/java.injected/org/apache/harmony/x/swing/plaf/resources/basic/BasicResourceBundleTest.java
1 modules/awt/src/test/impl/boot/java/awt/geom/Ellipse2DTest.java
1 modules/print/src/test/api/java/common/javax/print/GetSupportedDocFlavorsTest.java
1 modules/rmi/src/test/api/java/org/apache/harmony/rmi/tests/java/rmi/activation/DefaultParamTest.java
1 modules/print/src/test/api/java/common/javax/print/PrintJpegTest.java
1 modules/print/src/test/api/java/common/javax/print/PrintAutosenseTest.java
1 modules/swing/src/test/api/java.injected/javax/swing/text/DefaultCaret_MultithreadedTest.java
1 modules/print/src/test/api/java/common/javax/print/GetAttributeTest.java
1 modules/print/src/test/api/java/common/javax/print/GetAttributesTest.java
1 modules/print/src/test/api/java/common/javax/print/GetSupportedAttributeValuesTest.java
1 modules/print/src/test/api/java/common/javax/print/IsAttributeCategorySupportedTest.java
1 modules/print/src/test/api/java/common/javax/print/GetUnsupportedAttributesTest.java
1 modules/print/src/test/api/java/common/javax/print/LookupPrintServicesTest.java
1 modules/print/src/test/api/java/common/javax/print/IsAttributeValueSupportedTest.java
1 modules/print/src/test/api/java/common/javax/print/LookupDefaultPrintServiceTest.java
Re: test excludes
Posted by Tim Ellison <t....@gmail.com>.
Ah the good old unit testing discussion -- it must be release time again :-)
(more below)
On 26/May/2010 10:59, Mark Hindess wrote:
> In message <20...@d06av01.portsmouth.uk.ibm.com>,
> Mark Hindess writes:
>> In message <AA...@mail.gmail.com>,
>> Nathan Beyer writes:
>>> For future milestone votes, I'd like to propose that we set the
>>> expectation of zero test failures. Every time we do a milestone and I
>>> run the build and tests I find myself looking at the test failures
>>> (there are ALWAYS failures) and ask 'are these expected'? If the
>>> failures are expected, they should be excluded and the tests should
>>> just pass.
>>>
>>> If this proposal is workable, immediately after this upcoming release,
>>> all 'expected' failures will be pulled into the exclusion lists.
>> I was thinking this too. The problem for me is that:
>>
>> 1) Our test coverage isn't brilliant so we need a way to exclude
>> individual tests not the tests for a whole class. We have too many
>> excluded tests already[0].
I agree that excluding whole classes is too broad.
>> 2) The JDWP test framework seems to pass most of the time on the 5.0
>> code base but seems to fail quite often on the 6.0 branch. I normally
>> resort to doing 10 test runs and comparing the results of all runs. I
>> typically see every test pass on at least one run. This leads me to
>> believe that the improved jdwp in the 6.0 branch is merely exposing race
>> conditions in the test framework. It would be great to fix the test
>> framework so that these are avoided. If we tried to exclude tests that
>> fail regularly on java6 jdwp we would probably end up removing most of
>> the tests. ;-(
>>
>> I think solving 1 is really a pre-requisite to moving forward with your
>> (excellent) goal. We should make that a priority after this release. I
>> don't care if the solution is something fancy with junit 4 annotations
>> or something simple like adding:
>>
>> if (Support_Excludes.isExcluded()) {
>> return;
>> }
>>
>> to problematic tests where the support class just looks up the
>> class/method name of the caller in the exclude lists. (I actually
>> quite like the idea of doing the exclude list processing lazily at
>> run time rather than generating the list with ant.)
>
> Last night I decided to try to implement this simple exclude list
> mechanism in my build-improvement branch. I've checked in my changes at
> https://svn.apache.org/repos/asf/harmony/enhanced/java/branches/mrh@948377
I've not reviewed the changes, but notice there were a lot of
modifications required to the test cases themselves.
> I've automatically added isExcluded() checks to all the test methods
> in previously excluded tests (though this was automated and more
> complicated than you might think, and definitely not 100% accurate
> but should be close enough). The next job would be to remove/move
> the isExcluded() checks to be as fine-grained as possible so as just
> to exclude the failing tests (or individual asserts). As I commented
> previously many of the excludes probably just need to be removed
> completely.
>
> New excludes can be added as before by adding tests names to the lists
> but you also need to add an appropriate isExcluded()-if check to the
> test code as well. Hopefully this will maximize coverage will still
> allowing us to have only passing tests running by default.
I apologize for having the 'same old discussion' on this topic, but I do
believe that it is the role of the framework to decide which tests to run.
Of course, right now there is only a simple selection based on test*()
method name, and we have discussed previously the ways in which we can
inject more information to help the framework decide if a test is valid
or not. TestNG and JUnit4 have their own ideas about this too.
Modifying the tests themselves to make them more granular would
certainly help the framework separate out the tests we expect to pass
from those we expect to fail -- having length tests written as a single
test*() method doesn't really help since there is no way of knowing the
dependencies throughout the test code.
Rather than go through the code and decide where to write the
isExcluded() checks, can we split methods up to make them more readily
exclude-able?
> If different exclude conditions apply to different platforms you can
> use:
>
> package.class#methodname
> package.class+arbitrary_tag
> package.class#methodname+arbitrary_tag
> arbitrary_tag
>
> to exclude tests - though I'd avoid the final one unless it is something
> really broad like a VM not supporting concurrent (like the IBM v4 VMEs).
>
> To see what is happening, you can run the tests with:
>
> -Dhy.test.vmargs=-Dhy.excludes.verbose=true
>
> (though this is too verbose I might make the load method less verbose
> or use a hy.excludes.debug=true flag for that output).
>
> To ignore excludes and run all tests use:
>
> -Dhy.test.vmargs=-Dhy.excludes.ignore=true
>
> Rather than using test-jre-vm-info in ant the vm name for the exclude
> file selection is a trivial guess based on system properties. If you
> are using a custom vm and exclude lists then you can use:
>
> -Dhy.test.vmargs=-Dhy.excludes.vm=myvm
>
> to set it explicitly.
>
> I'm tempted to do away with the isExcluded() checks/lists are replace
> them with more readable:
>
> if (Support_Excludes.isRI()) {
>
> or:
>
> if (Support_Excludes.isLinux() && Support_Excludes.is64bit()) {
>
> etc. To make the Excludes more self-contained but that requires
> looking at the excluded tests in more detail to understand the
> requirements better and as I said I wanted to make this a simple
> step from what we had today.
>
> I'd like to merge this to trunk/java6 after the code freeze but I'd
> like some feedback first. I don't necessarily see this as a final
> solution but I just wanted to do something since this topic has been
> discussed for far to long and I wanted something that we could move to
> from where we are today without to much effort. On the other hand,
> I'm not convinced that annotations are a good solution either since
> they don't give you fine-grained control so every distinction has to be
> represented by a separate method.
>
> Comments very welcome.
My reaction is "yuk!" (well you did ask ;-) and I'd like to consider a
more declarative approach, but you are writing the code and I don't have
a concrete alternative to offer up. I have no /technical/ objection.
[ Perhaps it's simpler for me to just go fix the tests so they all pass
then the question of how to do excludes right will become moot ;-) ]
Regards,
Tim
Re: test excludes
Posted by Mark Hindess <ma...@googlemail.com>.
In message <AA...@mail.gmail.com>,
sebb writes:
>
> On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
> >
> > On the other hand, I'm not convinced that annotations are a good
> > solution either since they don't give you fine-grained control so
> > every distinction has to be represented by a separate method.
>
> That seems like a positive benefit to me.
Only sometimes I think.
> If a test method has several asserts which are independent, then the
> first fail may be masking later ones.
Part of me likes the idea of splitting independent tests into separate
methods, but I can't help thinking it will be a trade off as it will
lengthen my test cycles particularly the already slow report part.
Also, consider:
/**
* @tests java.util.EnumMap#entrySet()
*/
public void test_entrySet() {
...
entry = (Map.Entry) iter.next();
assertEquals("Wrong key", Size.Middle, entry.getKey()); //$NON-NLS-1$
assertTrue("Returned false for contained object", set.contains(entry));
//$NON-NLS-1$
enumSizeMap.put(Size.Middle, 3);
assertTrue("Returned false for contained object", set.contains(entry));
//$NON-NLS-1$
entry.setValue(2);
assertTrue("Returned false for contained object", set.contains(entry));
//$NON-NLS-1$
assertFalse("Returned true for uncontained object", set //$NON-NLS-1$
.remove(new Integer(1)));
iter.next();
//The following test case fails on RI.
assertEquals("Wrong key", Size.Middle, entry.getKey()); //$NON-NLS-1$
set.clear();
assertEquals("Wrong size", 0, set.size()); //$NON-NLS-1$
...
}
I'd propose putting:
if (Support_Excludes.isRI()) {
...
}
around the asserts that fail on the RI. You could create a new test
method but you'd end up repeating the code to build up to it so I don't
think it make sense to do that.
You are undoubtedly correct that we don't have the balance right though
(that particularly test is enormous and would be better split in to
helpful named methods distinguishing the "topics").
Regards,
Mark.
Re: test excludes
Posted by sebb <se...@gmail.com>.
On 26/05/2010, Tim Ellison <t....@gmail.com> wrote:
> On 26/May/2010 12:26, sebb wrote:
> > On 26/05/2010, Tim Ellison <t....@gmail.com> wrote:
> >> On 26/May/2010 11:20, sebb wrote:
> >> > On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
> >> >
> >> >> On the other hand,
> >> >> I'm not convinced that annotations are a good solution either since
> >> >> they don't give you fine-grained control so every distinction has to be
> >> >> represented by a separate method.
> >> >
> >> > That seems like a positive benefit to me. If a test method has several
> >> > asserts which are independent, then the first fail may be masking
> >> > later ones.
> >>
> >> So how do you know which of the asserts are independent? Letting them
> >> all fall through would seem wrong, since you would likely need to fix
> >> the first assertion failure and retest for the later dependent
> >> assertions to be meaningful.
> >
> > If a subsequent test cannot possibly work unless the current assertion
> > succeeds, then the subsequent test is not independent.
> >
> > e.g.
> >
> > Fetch item from Map
> > assertNonNull(item) // must succeed else next assert is bound to fail
> > assertEquals("abc",item.getName())
> >
> > This may mean more shorter tests (and possibly some duplication of
> > setup) but it does allow for better control over expected failures.
> >
> > [Note that assertions in JUnit setUp() methods work fine]
>
>
> Yes, I understand the definition. My question is how do you
> (practically) find them in the 26,500 JUnit tests that we are currently
> running.
>
> You're not expecting me to read though the code and decide which are
> independent and which are not, are you?!
Not for every test.
But the ones that fail need to be investigated anyway, and at that
point the opportunity should be taken to decompose them.
If necessary, log a message to say that a particular test may fail on
some hosts.
If the test always fails on a particular host, then conditionally skip
that test, but log a message to say it has been skipped.
Suppressing the test so that no output appears seems like a bad idea to me.
>
> >> Without being able to distinguish I think we'd just get lots more
> >> failures listed but no way of knowing which ones are false positives.
> >>
> >> Here's somebody's solution to making the assertions non-fatal.
> >> http://www.gnufoo.org/junit/index.html#failures
> >
> > Which looks fine, except that non-fatal assertions only help for
> > *independent* tests.
> >
> > Otherwise they generate more output than necessary - e.g. my example
> > above would generate at least two failures where one is sufficient.
> >
> > Independent tests need independent test methods. Not always easy to do
> > in all cases
>
>
> Agreed.
>
> Regards,
>
> Tim
>
Re: test excludes
Posted by Tim Ellison <t....@gmail.com>.
On 26/May/2010 12:26, sebb wrote:
> On 26/05/2010, Tim Ellison <t....@gmail.com> wrote:
>> On 26/May/2010 11:20, sebb wrote:
>> > On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
>> >
>> >> On the other hand,
>> >> I'm not convinced that annotations are a good solution either since
>> >> they don't give you fine-grained control so every distinction has to be
>> >> represented by a separate method.
>> >
>> > That seems like a positive benefit to me. If a test method has several
>> > asserts which are independent, then the first fail may be masking
>> > later ones.
>>
>> So how do you know which of the asserts are independent? Letting them
>> all fall through would seem wrong, since you would likely need to fix
>> the first assertion failure and retest for the later dependent
>> assertions to be meaningful.
>
> If a subsequent test cannot possibly work unless the current assertion
> succeeds, then the subsequent test is not independent.
>
> e.g.
>
> Fetch item from Map
> assertNonNull(item) // must succeed else next assert is bound to fail
> assertEquals("abc",item.getName())
>
> This may mean more shorter tests (and possibly some duplication of
> setup) but it does allow for better control over expected failures.
>
> [Note that assertions in JUnit setUp() methods work fine]
Yes, I understand the definition. My question is how do you
(practically) find them in the 26,500 JUnit tests that we are currently
running.
You're not expecting me to read though the code and decide which are
independent and which are not, are you?!
>> Without being able to distinguish I think we'd just get lots more
>> failures listed but no way of knowing which ones are false positives.
>>
>> Here's somebody's solution to making the assertions non-fatal.
>> http://www.gnufoo.org/junit/index.html#failures
>
> Which looks fine, except that non-fatal assertions only help for
> *independent* tests.
>
> Otherwise they generate more output than necessary - e.g. my example
> above would generate at least two failures where one is sufficient.
>
> Independent tests need independent test methods. Not always easy to do
> in all cases
Agreed.
Regards,
Tim
Re: test excludes
Posted by sebb <se...@gmail.com>.
On 26/05/2010, Tim Ellison <t....@gmail.com> wrote:
> On 26/May/2010 11:20, sebb wrote:
> > On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
> >
> >> On the other hand,
> >> I'm not convinced that annotations are a good solution either since
> >> they don't give you fine-grained control so every distinction has to be
> >> represented by a separate method.
> >
> > That seems like a positive benefit to me. If a test method has several
> > asserts which are independent, then the first fail may be masking
> > later ones.
>
> So how do you know which of the asserts are independent? Letting them
> all fall through would seem wrong, since you would likely need to fix
> the first assertion failure and retest for the later dependent
> assertions to be meaningful.
If a subsequent test cannot possibly work unless the current assertion
succeeds, then the subsequent test is not independent.
e.g.
Fetch item from Map
assertNonNull(item) // must succeed else next assert is bound to fail
assertEquals("abc",item.getName())
This may mean more shorter tests (and possibly some duplication of
setup) but it does allow for better control over expected failures.
[Note that assertions in JUnit setUp() methods work fine]
> Without being able to distinguish I think we'd just get lots more
> failures listed but no way of knowing which ones are false positives.
>
> Here's somebody's solution to making the assertions non-fatal.
> http://www.gnufoo.org/junit/index.html#failures
Which looks fine, except that non-fatal assertions only help for
*independent* tests.
Otherwise they generate more output than necessary - e.g. my example
above would generate at least two failures where one is sufficient.
Independent tests need independent test methods. Not always easy to do
in all cases
> Regards,
>
> Tim
>
Re: test excludes
Posted by Tim Ellison <t....@gmail.com>.
On 26/May/2010 11:20, sebb wrote:
> On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
>
>> On the other hand,
>> I'm not convinced that annotations are a good solution either since
>> they don't give you fine-grained control so every distinction has to be
>> represented by a separate method.
>
> That seems like a positive benefit to me. If a test method has several
> asserts which are independent, then the first fail may be masking
> later ones.
So how do you know which of the asserts are independent? Letting them
all fall through would seem wrong, since you would likely need to fix
the first assertion failure and retest for the later dependent
assertions to be meaningful.
Without being able to distinguish I think we'd just get lots more
failures listed but no way of knowing which ones are false positives.
Here's somebody's solution to making the assertions non-fatal.
http://www.gnufoo.org/junit/index.html#failures
Regards,
Tim
Re: test excludes (was [proposal] For future milestones, expect zero
test failures)
Posted by sebb <se...@gmail.com>.
On 26/05/2010, Mark Hindess <ma...@googlemail.com> wrote:
> On the other hand,
> I'm not convinced that annotations are a good solution either since
> they don't give you fine-grained control so every distinction has to be
> represented by a separate method.
That seems like a positive benefit to me. If a test method has several
asserts which are independent, then the first fail may be masking
later ones.