Submitting patches was RE: [nunitv3] Assume Implementation

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-----
> From: [email protected] 
> [mailto:[email protected]] On Behalf Of Kelly Anderson
> Sent: Tuesday, March 25, 2008 3:38 PM
> To: NUnitV3; [email protected]
> 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.
> 
> 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
> 
> --~--~---------~--~----~------------~-------~--~----~
> You received this message because you are subscribed to the 
> Google Groups "NUnitV3" group.
> To post to this group, send email to [email protected] 
> To unsubscribe from this group, send email to 
> [email protected]
> 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