[Junit-trackers] [ junit-Feature Requests-1539242 ] Weir exceptions are break error handling/report of Junit 4.1

[SourceForge.net <no...@so...>]

Feature Requests item #1539242, was opened at 2006-08-12 16:41
Message generated for change (Comment added) made by sf-robot
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=365278&aid=1539242&group_id=15278

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Closed
Resolution: None
Priority: 5
Private: No
Submitted By: TOF (dash-13)
Assigned to: Nobody/Anonymous (nobody)
Summary: Weir exceptions are break error handling/report of Junit 4.1

Initial Comment:
Because of a dumb fault of mine, I had a tough time
figuring out what went wrong with junit...

My fault: the getMessage() method of an Exception class
I wrote caused a NullPointerException.

This caused some pretty odd behavior, denying me any
hint, what might have went wrong:

Part A:

Using the plain command line with the build in
TextListner, I got:

JUnit version 4.1
EE.E.........E...E.E.................
Time: 441,976
There were 6 failures:
1) myTest[0](my.Package.MyTest)

... and exitus.


This, for once, is caused by 
org.junit.runner.notification.Failure.getTrace():
	public String getTrace() {
		StringWriter stringWriter= new StringWriter();
		PrintWriter writer= new PrintWriter(stringWriter);
		getException().printStackTrace(writer);
		StringBuffer buffer= stringWriter.getBuffer();
		return buffer.toString();
	}

where printStackTrace() delegates implicitly to
getMessage(). The resulting NPE is passed up to
JUnitCore.run() and never ever intercepted.

How I found this is Part B:

In absence of a yet stable TestListner in my favorite
IDE, and not yet aware of the NPE in getMessage(), I
decided to brew my own - only to discover, that it
quits reporting the very moment, it should have shown
the exception!

This is due to the following piece of code in RunNotifier:
Line 34:
	private abstract class SafeNotifier {
		void run() {
			for (Iterator<RunListener> all=
fListeners.iterator(); all.hasNext();) {
				try {
					notifyListener(all.next());
				} catch (Exception e) {
					all.remove(); // Remove the offending listener
first to avoid an infinite loop
					fireTestFailure(new
Failure(Description.TEST_MECHANISM, e));
				}
			}
		}
This could be called 'according to spec', because
elsewhere the java doc tells, that listners throwing
are removed, but using the cow bells example, this
means silencing the cow bell, when it should have been
toling like hell.

If still interested, I see several possible solutions
towards a more helpfull response to users:

a.) Exceptions stuffed into the Failure class shouldn't
be accessed directly. Failure could be used to wrap the
acces, which lends an opportunity to safeguard against
misbehaving exceptions as well as reporting details
regarding the nature of the problem.

b.) It might be OK to remove a corrupted TestListner to
prevent problems, but think it is unwise, if it's not
the TestListner, but rather the data it got. I'm also
not sure, if I'd like to have news about the failing
Listner disseminated into all other listners that
happen to be registered. Couldn't we use the parent
class RunListner as sort of proxy, say, implementing a
'protectedTestFailure()' which delegates to
testFailure,  retries with a modified Exception in case
of problems, and finally dumps to System.err, before
giving up?

c.) As a last resort and precaution: JUnitCore should
intercept and dump any exceptions that make it through.

Well, obviously, I should have tested my exception
class before starting to throw it around ;-)

Best Regards,
T. S.


----------------------------------------------------------------------

>Comment By: SourceForge Robot (sf-robot)
Date: 2009-12-01 02:20

Message:
This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

----------------------------------------------------------------------

Comment By: David Saff (dsaff)
Date: 2009-11-16 17:52

Message:
This tracker is being shut down.  Please move this item to
http://github.com/KentBeck/junit/issues

----------------------------------------------------------------------

Comment By: TOF (dash-13)
Date: 2006-09-30 19:30

Message:
Logged In: YES 
user_id=1557775

I allready moved on:

Patch 1542845 offers a remedy.

As I found out during the implementation, I was wrong on the
assumption, that the failure to report the exception was on
account of the framework not passing up uncaught throwables.
In fact, it does. But unfortunately, the build in
TextListner gets caught by the same snatch that silenced my
custom listner: it simply gets removed when trying to report
the poisoned exception, leaving the framework with no
listner to report to!

As in Patch 1542845 explained, this is fixed by:
 - treating poisoned exceptions as sugested in a.)
 - as a precaution, throwing a runtime exception, when the
last listner is removed, which addreses b. & c.) in a much
simpler fashion than originally expected.

The fix contains a testcase too.
So if you agree to the patch, this case might be closed.

Hope this helps,
T. S.

----------------------------------------------------------------------

Comment By: David Saff (dsaff)
Date: 2006-09-29 13:23

Message:
Logged In: YES 
user_id=325156

I totally agree with items a and c, and would like to
further discuss item b.  To help with housekeeping, can you
a new issue for each item (with a reference to this one)? 
Thanks!

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=365278&aid=1539242&group_id=15278