Re: NUnit 2.5: What's In It Now

On Wed, Apr 9, 2008 at 11:20 AM, Charlie Poole
<[email protected]> wrote:
> Hi Kelly,
>
>  > >  Actually, I don't find it more expressive - which is
>  > probably why  we
>  > > aren't agreeing. To me, it looks as if it ought to do
>  > something  more
>  > > than just return, so I waste energy trying to figure it out.
>  > >  Then I learn it's just a way for the test to succeed. I guess
>  > > expressiveness is a matter of opinion.
>  >
>  > That's really interesting. Do you find Theory to be more
>  > expressive? I thought that part of what made Theory more
>  > expressive was Assume. I guess it might be a matter of what
>  > makes sense to you... for example I don't find this code:
>
>  My point was that Assume as a part of Theory does NOT merely
>  return success. It's not the word that I find unexpressive,
>  it's that it doesn't seem to express what you are doing.
>
>  It's like a method name, which expresses something, but
>  not what the method really does. Usually, we just change
>  the name of the method. In this case, I think we want
>  Assume to do something other than what you're having
>  it do. I don't want to implement it and then have to
>  change the semantics at a later time.

I guess that makes sense. I'm just not sure I can think of what it
would do differently, other than how the framework deals with it when
the exception fires. There is no way to return from the middle of a
function other than an exception (that I can think of) so I don't know
how else you would implement something like assume in the middle of
the code. You could somehow do it before the method is called, but
that separates the assumption from the assertions, which I find less
expressive than having them close together in the code.

The proximity of the Assumes to the Asserts seems fundamental to me in
terms of expressiveness. Does that make sense to you in the context of
Theory?

>  > [Test]
>  > public void x()
>  > {
>  >  Assert.Throws<ArgumentException>(new
>  > TestSnippet(MethodThatThrows) ); }
>  >
>  > To be more expressive than:
>  >
>  > [Test]
>  > [ExpectedException(typeof(ArgumentException))]
>  > {
>  >   MethodThatThrows();
>  > }
>  >
>  > But I guess maybe that's just me. It may also be that I'm not
>  > yet used to the delegate syntax, and I may grow to like it as
>  > I have with Assert.That()...
>
>  This is on a different plain AFAIC. It's a well-tested syntax,
>  used in other apps and has been requested for NUnit for years.
>  I'm not expressing a preference here, I;m responding to a request.

Ok.

>  And anyway, you're using the .NET 1.1 syntax. It looks lots
>  better as
>   Assert.Throws<ArgumentException>( MethodThatThrows );
>
>  Which is why I turned down the requests till now. :-)

Still looks a bit funny, but I suppose I could get used to it.

>  > Of course, there is also the possibility that you might grow
>  > to like Assume.That, if you used it as much as I used Assert.That...
>
>  I like it! I like it! I don't like it as a synonym for Succeed.

Sigh.

>  > >  > See my post to Gary a minute ago. In the Framework the Assert  >
>  > > exception is handled, I want to handle the Assume exception  > in a
>  > > similar way.
>  > >
>  > >  They are handled by NUnitTestCase, which is not part of
>  > the  core of
>  > > NUnit - it's just another extension like you might write.
>  >
>  > But to write that extension would mean that Assume would only
>  > work within [KellyTests] not [Test]... and I'd really like it
>  > to work for any other sort of test as well... Maybe that's
>  > asking too much of the extensibility framework.
>
>  If we had the ablility to extend extensions, then it could
>  work for an NUnitTestMethod. As I made clear at the start of
>  this, we cannot have extensions on extensions without the
>  major rework of the addin architecture planned for 3.0.

Ok, just my previous misunderstanding of the architecture, which I
THINK is now straightened out.

>  I could be wrong, but I think you are having difficulty
>  accepting that NUnitTestMethod is itself an extension. If
>  so, consider what should happen if you use Assume.That in
>  a csunit test method.

Ok I get it now.

>  > >  I don't understand that.
>  >
>  > With a Theory Explorer, you can't predict what the parameters
>  > might be, therefore, you would have to have Assume. If you just used
>  > if(condition) return; then you wouldn't have sure knowledge
>  > that the Theory Explorer had generated cases that reached the
>  > code to be truly explored, that is, the code after the
>  > Assume.That's... This could lead to bad theories. For
>  > example, if I wrote this theory...
>
>  In my view, that's not Theory Explorer, it's Theory. I believe
>  that the base implementation of Theory should provide for mixing
>  up available parameters. The notion of a spearate program to
>  suggest additional parameters (Theory Explorer) is an important
>  idea and one we may see implemented some day. But a supplier
>  of parameters for Theory is not going to be deterministic,
>  at least in my view, the way a TestCase is.

I think it's important when RUNNING a Theory for the run to be
repeatable, thus deterministic (if my definition of deterministic is
the same as yours). When you go exploring the space a Theory covers,
then it's important to be non-deterministic (but report the case(s)
where the Theory fails so that new cases can be added to the Theory
data source.)

>  So yes, I believe something performing the function you assign
>  to Assume.That is needed in a Theory. I don't believe it has
>  to be Assume.That, but it is definitely a likely candidate.
>  Since it is likely that Assume.That will be used with Theory,
>  and that it will have a different meaning than what you have
>  given it, I don't want to start using it with your meaning
>  right now.

Ok, that makes a certain amount of sense, I just don't see the meaning
as being quite as significant as you are indicating. This is of course
just opinion when you get to that point.

>  > private bool IsEven(int number)
>  > {
>  >  return true; // wrong
>  > }
>  >
>  > [Theory]
>  > public void AllEvenNumbersShouldDivideToFullNumbers(int evenNumber) {
>  >   if (!IsEven(evenNumber))
>  >      return;
>  >   Assert.That(evenNumber/2, Is.EqualTo( (int) ( (double)
>  > evenNumber)/2.0 ) ); }
>  >
>  > You see that I messed up the implementation of IsEven, but I
>  > would never know it because the Theory would always pass.
>  > However, if I wrote it with Assume like so...
>  >
>  > [Theory]
>  > public void AllEvenNumbersShouldDivideToFullNumbers(int evenNumber) {
>  >   Assume.That (IsEven(evenNumber));
>  >   Assert.That(evenNumber/2, Is.EqualTo( (int) ( (double)
>  > evenNumber)/2.0)); }
>  >
>  > Then the framework would tell me that IsEven was implemented
>  > incorrectly, because the test would never complete
>  > successfully as the Assume would fire every time.
>
>  That's not what your Assume.That does, however. We're not
>  (I'm not) talking about the general design of Theory here,
>  but about your particular implementation which equates
>  to the use of return.

My implementation throws AssumeException, which is not equivalent to
return. My implementation of the catch of AssumeException in the test
case makes it equivalent, but I'm kind of separating Assume.That from
the handling of the exception. If you're talking about how the
exception is handled, then point taken. But I believe throwing the
exception has the right implementation for Theory...

>  > That seems like a technical argument, rather than an argument
>  > of expressiveness.
>
>  Again - YOUR Assert.That expresses something it doesn't
>  deliver. Giving users a method that seems to do something
>  but doesn't is not such a good idea.
>
>  Assert.Succeed() has been requested and I'm inclined to
>  implement it, since it's trivial. Would you be equally
>  happy using Assert.Succeed in place of your non-Theory
>  uses of Assume.That?

As long as I could use a Constraint with Assert.Succeed... but then it
reads funny
Assert.Succeed(Is.EqualTo(f)); ???

If you just do Assert.Succeed() that doesn't give you the same level
of expressiveness. If you did

Succeed.If(Is.EqualTo(f));

That would seem grammatically and syntactically similar to what I'm proposing.

>  > >  ProcessException is in nunit.core, in TestMethod or TestCase -  I
>  > > forget which at the moment. But it's not truly "core" -
>  > that  is it's
>  > > not part of the stuff we extend. Rather, it is itself  an
>  > extension.
>  > > When you write your own extension, with your own  test
>  > case, then you
>  > > can handle any exception you like in any  way you like.
>  >
>  > It's in TestMethod. Ok, then I just misunderstood the
>  > architecture. I would just have to subclass TestCase, and
>  > write my own kind of ProcessException...
>
>  Or take a look at what methods are virtual.
>
>  Ideally, I'd like to see extensions subclass Test, but it's
>  hard to pass up all the pre-implemented stuff. I've been
>  guilty of falling into the inheritance trap myself. In
>  3.0, I want to look at the versioning issue for addins.

More interfaces, less inheritance! :-)

>  > >  In principle, this is no different from the exceptions
>  > that  signal a
>  > > failure. There is no place in NUnit that recognizes  those
>  > exceptions
>  > > for all tests. NUnitTestMethod recogizes the  particular exception
>  > > that it knows about. CsUnitTestMethod -  to take one example -
>  > > recogizes a completely different exception.
>  >
>  > Ok, I think I "get it" now. I really would like for all kinds
>  > of TestCase subclasses to be able to handle Assume, just like
>  > they all can handle Assert (I suppose that they all just
>  > choose to handle Assert at this point, if I'm
>  > understanding)... but that is probably not going to happen.
>
>  Bear in mind that TestCase and TestMethod were not written to
>  be inherited from by Addins. It's just an expedient thing we
>  are all doing as a quick way to get work done. I'm hoping
>  to see us get away from this as the addin thing matures.

I agree. More interfaces could help.

>  Even so, NOT all TestCase-derived clases handle the same Assert.
>  For example, if you use an NUnit Assert in a csUnit test class,
>  it comes up as an unexpected exception, not a failure. That's
>  because CsUnitTestMethod doesn't understand NUnit's exception.

Ok, I get it now :-)

>  This isn't a bug: it's the way it has to work. Different
>  frameworks handle different exceptions. The different
>  exception types are not what is understood in common among
>  all tests under NUnit: it's the TestResults that are
>  created by the individual test types.

I thought they were orthogonal concerns before.

>  In an earlier version of NUnit's extensibility, there was
>  a concept that each test type fell into under a "TestFramework"
>  and could tell NUnit under which frameworks it should be
>  active. That was too complicated and I took it out, although
>  there is still some code left over that allows you to list
>  frameworks that have been loaded. It could be that we will
>  want to reconsider this decision in the future, so we could
>  say AssumeException is recognized by all test types that
>  are under the NUnitTestFramework. Right now, that's not
>  possible.

I agree that it is not currently possible. The question is whether it
is eventually desirable, and that's a separate question.

I assume a good design might have exception types orthogonal to test
types, but there may be cases I haven't considered, so I'm not ready
to assert my assumption. :-)

-Kelly

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone