On Wed, Dec 9, 2009 at 2:07 AM, Adam Murdoch <[email protected]> wrote:
> > > John Murph wrote: > > The changes are on my github account (git://github.com/jmurph/gradle.git) > in branch test_listener. Adam, would you mind looking at it and giving > feedback? > > > This looks really good. Some comments: > > - TestListener: can we get rid of the distinction between an error and a > failure, and just have failures? > Done. > > - AbstractTestTask: I think we shouldn't make the broadcaster public, as > this is really an internal concern. Perhaps we should also add > addTestListener() and removeTestListener() methods on AbstractTestTask, plus > some javadoc. > If I make getTestListenerBroadcaster() protected, then the JUnitTestFrameworkInstanceTest class cannot access the method (for mocking purposes). I'm not sure how to work around this problem. I've added the addTestListener() and removeTestListener() methods, and beefed up the javadoc for much of my changes. > > - TestNGListenerAdapter: returns a status of FAILURE when a test failure, > but the exception is made available via getError() rather than getFailure(). > The method was renamed to getException() in part to address this confusion. > > - Maybe update the javadoc for Gradle.addListener() to mention that > TestListener is available? > Done. > > - I'd really like to see an integration test or tests which cover this, as > it seems like something that might be easy to break. Perhaps we could grow > the existing junit and testng int tests, so that we get some coverage with > failing and passing tests. > I added the java/testListener sample for that purpose, as well as to jumpstart documentation needs. Is having that sample insufficient for integration testing needs? I made sure to have passing and failing tests, but it's only testing JUnit as the TestNG support is currently disabled. -- John Murph Automated Logic Research Team
