Discussion:
Assume Implementation
Kelly Anderson
2008-03-25 23:13:33 UTC
Permalink
The attachment bounced from the nunit-developer listserv... don't know
if it got through to NunitV3 or not. Sorry if this is a multiple
posting. I sent the attachment to Charlie directly...

-Kelly

As promised, I have attached my implementation of Assume. I did this
with the absolute minimum possible changes to the other files, so the
implementation is not optimal yet. It does, however, work for regular
Assume. I did not bother yet to do Collections, Strings or File
Assumes to match the corresponding Asserts.... not sure if those are
useful or not.

These files were modified (slightly):

NUnitFramework.cs

Added:
public static readonly string AssumptionException =
"NUnit.Framework.AssumptionException";


NUnitTestMethod.cs

Added:

protected override bool IsAssumeException(Exception ex)
{
return ex.GetType().FullName ==
NUnitFramework.AssumptionException;
}

TestMethod.cs

Changed:
protected internal virtual void ProcessException(Exception
exception, TestCaseResult testResult)
{
if (!ExceptionExpected)
{
RecordException(exception, testResult);
return;
}

To:
protected internal virtual void ProcessException(Exception
exception, TestCaseResult testResult)
{
if (IsAssumeException(exception))
{
// TODO: Perhaps add text to some output stream.
testResult.Success();
return;
}

if (!ExceptionExpected)
{
RecordException(exception, testResult);
return;
}


nunit.framework.dll_VS2005.csproj
Added references to new files:
<Compile Include="Assume.cs" />
<Compile Include="AssumptionException.cs" />

Assume.cs - New File
AssumptionException.cs - New File

nunit.framework.tests_VS2005.csproj
Added reference to new file:
<Compile Include="AssumptionTest.cs" />

AssumptionTest.cs - Added

===================

Assume works just like Assert, except that it throws a different exception.

I would propose refactoring this code, pushing up a common base class
(Perhaps called AssBase? :-) that is inherited by both Assume and
Assert. Everything is moved up there, except that there is a new
virtual function called ThrowException which either throws
AssertException or AssumeException in either subclass. Wherever the
Exception is thrown in the base class, call ThrowException. Then I
think it's nicely done.

I didn't do that already because I wanted to make merging the code in
very easy for anyone wanting to experiment.

Charlie, please feel free to add this to the code base if you like it.
Obviously, I'd rather do the suggested refactoring first, of course.

The question of where to put the Assume message output is obviously up
for discussion. I did not implement anything relating to multiple
invocations of a test, including not passing the test if it always
fires Assume.

On a side note, this is only the second time I've modified code
written by someone else under test. What a pleasure!! Even though I
did have to resort to using the Debugger to figure out how the
exceptions were handled :-)

-Kelly

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
Charlie Poole
2008-03-26 01:34:35 UTC
Permalink
Hi All,

Since Kelly isn't the only one likely to be in the
situation of wanting to send in changes, I thought
I'd respond on both groups about the best form of
proposed patches to NUnit.

It's a bit rough to work through an entire new build
with 500+ files so as to get to the changed stuff.
Kelly realized this and sent me some explanations
of what you had changed to make it easier. That was
a GoodThing. :-) Another GoodThing was that he told
me what version of the source he was basing it on.
Sometimes people don't do that, which is confusing.

So I can work with this, it just isn't the BestWay
for the future.

Here's the BestWay for me:
1) Work from the CVS head or the head of a given
release branch if you are proposing a bug fix
for that release - for example the 2.4 branch
for fixes going into 2.4.x.
2) Send a single, multi-file patch for each logical
change that you are making - i.e. one in this case.
I'm familiar enough with the code that I can
usually figure out what's going on by reading the
patches, which saves time, so use a context format
by preference (diff -c) in many programs.

I realize that the above may be a bit foreign to many
Windows-only developers. Moving forward, I'll try to set
up some simple instructions for folks not familiar with
CVS - or whatever version control we later use.

Meanwhile, here's the SecondBestWay:

1) Work from the latest released source or at least
a very recent one.
2) Send only the changed or added files.

In either case, always specify where you got the
source that you based the changes on.

Charlie
-----Original Message-----
Sent: Tuesday, March 25, 2008 3:38 PM
Subject: [nunitv3] Assume Implementation
As promised, I have attached my implementation of Assume. I
did this with the absolute minimum possible changes to the
other files, so the implementation is not optimal yet. It
does, however, work for regular Assume. I did not bother yet
to do Collections, Strings or File Assumes to match the
corresponding Asserts.... not sure if those are useful or not.
NUnitFramework.cs
public static readonly string AssumptionException =
"NUnit.Framework.AssumptionException";
NUnitTestMethod.cs
protected override bool IsAssumeException(Exception ex)
{
return ex.GetType().FullName ==
NUnitFramework.AssumptionException;
}
TestMethod.cs
protected internal virtual void
ProcessException(Exception exception, TestCaseResult testResult)
{
if (!ExceptionExpected)
{
RecordException(exception, testResult);
return;
}
protected internal virtual void
ProcessException(Exception exception, TestCaseResult testResult)
{
if (IsAssumeException(exception))
{
// TODO: Perhaps add text to
some output stream.
testResult.Success();
return;
}
if (!ExceptionExpected)
{
RecordException(exception, testResult);
return;
}
nunit.framework.dll_VS2005.csproj
<Compile Include="Assume.cs" />
<Compile Include="AssumptionException.cs" />
Assume.cs - New File
AssumptionException.cs - New File
nunit.framework.tests_VS2005.csproj
<Compile Include="AssumptionTest.cs" />
AssumptionTest.cs - Added
===================
Assume works just like Assert, except that it throws a
different exception.
I would propose refactoring this code, pushing up a common
base class (Perhaps called AssBase? :-) that is inherited by
both Assume and Assert. Everything is moved up there, except
that there is a new virtual function called ThrowException
which either throws AssertException or AssumeException in
either subclass. Wherever the Exception is thrown in the base
class, call ThrowException. Then I think it's nicely done.
I didn't do that already because I wanted to make merging the
code in very easy for anyone wanting to experiment.
Charlie, please feel free to add this to the code base if you like it.
Obviously, I'd rather do the suggested refactoring first, of course.
The question of where to put the Assume message output is
obviously up for discussion. I did not implement anything
relating to multiple invocations of a test, including not
passing the test if it always fires Assume.
On a side note, this is only the second time I've modified
code written by someone else under test. What a pleasure!!
Even though I did have to resort to using the Debugger to
figure out how the exceptions were handled :-)
-Kelly
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "NUnitV3" group.
To unsubscribe from this group, send email to
For more options, visit this group at
http://groups.google.com/group/nunitv3?hl=en
-~----------~----~----~----~------~----~------~--~---
-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
Charlie Poole
2008-04-08 16:08:56 UTC
Permalink
Hi Kelly,

Responding to your proposal - now that you
reminded me of it on the developer list.
Post by Kelly Anderson
As promised, I have attached my implementation of Assume. I
did this with the absolute minimum possible changes to the
other files, so the implementation is not optimal yet. It
does, however, work for regular Assume. I did not bother yet
to do Collections, Strings or File Assumes to match the
corresponding Asserts.... not sure if those are useful or not.
My thought is that you should stick with Assume.That for this
new feature. There is nothing you can do with the other Asserts
that can't be done with Assert.That and Assume.That will inherit
the same superset of functionality.

<snipped code changes/>
Post by Kelly Anderson
Assume works just like Assert, except that it throws a
different exception.
Which, in you code causes the test to succeed. That's an
implementation detail I find troubling. I believe it needs
to do something else to justify itself.
Post by Kelly Anderson
I would propose refactoring this code, pushing up a common
base class (Perhaps called AssBase? :-) that is inherited by
both Assume and Assert. Everything is moved up there, except
that there is a new virtual function called ThrowException
which either throws AssertException or AssumeException in
either subclass. Wherever the Exception is thrown in the base
class, call ThrowException. Then I think it's nicely done.
I plan to put the key Assert components (That, Fail, Ignore)
into a base class. The "classic" asserts would inherit from that.
However, I don't think Assume.Fail or Assume.Ignore make much
sense, so Assume may need to remain a separate class. Anyway,
I prefer to create duplication in code before removing it -
as opposed to just seeing it in my head as many folks do.
Post by Kelly Anderson
I didn't do that already because I wanted to make merging the
code in very easy for anyone wanting to experiment.
Good idea. It would have made it pretty impossible for me
to review as well.
Post by Kelly Anderson
Charlie, please feel free to add this to the code base if you like it.
Obviously, I'd rather do the suggested refactoring first, of course.
The question of where to put the Assume message output is
obviously up for discussion. I did not implement anything
relating to multiple invocations of a test, including not
passing the test if it always fires Assume.
A test that is run many times, with a sort of vote being taken
on the results, is a candidate for a new type of test. Tests
in NUnit are responsible evaluating their results - not the
runner. So any logic like this will have to live in some
special kind of test. The basic NUnit testcase passes or
fails based on a single result.
Post by Kelly Anderson
On a side note, this is only the second time I've modified
code written by someone else under test. What a pleasure!!
Even though I did have to resort to using the Debugger to
figure out how the exceptions were handled :-)
Cool!

Charlie
Post by Kelly Anderson
-Kelly
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "NUnitV3" group.
To unsubscribe from this group, send email to
For more options, visit this group at
http://groups.google.com/group/nunitv3?hl=en
-~----------~----~----~----~------~----~------~--~---
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Register now and save $200. Hurry, offer ends at 11:59 p.m.,
Monday, April 7! Use priority code J8TLD2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
Loading...