SimpleTestRunner miss handles point of exception

Bug #577156 reported by Lucas Caballero
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
NUnit Framework
Triaged
High
Unassigned

Bug Description

[This bug is now tracked at https://github.com/nunit/nunit-framework/issues/4]

The SimpleTestRunner performs a an odd skip of Exception processing when faced with a fully un-implemented EventListener. What happens is that my RunStarted throws an Exception and this gets caught end then RunFinished is called which then throws an exception -- the stack trace makes it look like the RunFinished was the culprit when in fact the RunStart began the exceptional behavior. This occurs mainly because the catch( Exception exception ) catches everything. Perhaps what it should do is catch NUnit exceptions and throw all other exceptions -- I'm not familiar enough with the architecture around Exception testing, Assert Exceptions, etc.

So here is the culprit code in SimpleTestRunner, Line 141, of release 2.5.5.101112.

public virtual TestResult Run( EventListener listener, ITestFilter filter )
{
 try
 {
        log.Debug("Starting test run");

  // Take note of the fact that we are running
  this.runThread = Thread.CurrentThread;

  listener.RunStarted( this.Test.TestName.FullName, test.CountTestCases( filter ) );

  testResult = test.Run( listener, filter );

  // Signal that we are done
  listener.RunFinished( testResult );
        log.Debug("Test run complete");

  // Return result array
  return testResult;
 }
 catch( Exception exception )
 {
                // RunStart actually threw the exception. so RunFinish doesn't make sense.

                // RunFinish then throws an exception when really the first exception should be handled first.
  // Signal that we finished with an exception
  listener.RunFinished( exception );
  // Rethrow - should we do this?
  throw;
 }
 finally
 {
  runThread = null;
 }
}

Cheers,
L-

Tags: github
Changed in nunitv2:
status: New → Triaged
importance: Undecided → High
milestone: none → 2.5.6
Changed in nunitv2:
assignee: nobody → Charlie Poole (charlie.poole)
Changed in nunit-3.0:
status: New → Triaged
importance: Undecided → High
Revision history for this message
Charlie Poole (charlie.poole) wrote :

The problem is subtle. If one of the RunStart event handlers throws an exception, some handlers may have already successully processed the event while others will never receive it at all. It's not possible to discriminate among the handlers without taking over the event routing from .NET and we don't want to do that.

Safest thing seems to be that we should not send RunFinished to any handlers unless all RunStarted events processed successfully.

Revision history for this message
Charlie Poole (charlie.poole) wrote :

OK, I tried suppressing RunFinished and now I see why I did it the way I did.

If no RunFinished is issued, then the GUI never realiizes that the run is finished and
NUnit just appears to hang.

Back to the drawing board!

Revision history for this message
Charlie Poole (charlie.poole) wrote :

Postponing this to another release. It's a true can of worms.

Essentially, we should probably have some sort of AddinException that wraps exceptions like this and treat them specially. We may simply have to remove addins once they throw an exception.

Changed in nunitv2:
milestone: 2.5.6 → none
tags: added: framework
Revision history for this message
Charlie Poole (charlie.poole) wrote :

Moving entirely to the 3.0 release

Changed in nunitv2:
status: Triaged → Won't Fix
Changed in nunit-3.0:
milestone: none → 2.9.6
Revision history for this message
Charlie Poole (charlie.poole) wrote :

I removed this from the 2.9.6 milestone since it's not clear whether custom EventListeners will be used in NUnit 3.0. We may end up providing this functionality without use of addins, in which case the problem will be resolved in a different way.

Changed in nunit-3.0:
milestone: 2.9.6 → none
no longer affects: nunitv2
description: updated
tags: added: github
removed: framework
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.