Re: [Cppunit-devel] Assertion, ASSERT_EQUAL & co...

[Baptiste L. <gai...@fr...>]

Quoting "Steve M. Robbins" <ste...@vi...>:

> Hey Baptiste,
> 
> 
> On Fri, Oct 05, 2001 at 02:30:36PM +0200, Baptiste Lepilleur wrote:
>  
> >     So, the path I wish to take is:
> >     1) propose a reasonable default ASSERT_EQUAL (to discuss later)
> >     2) provide all the tools for user to easily implement a new assert
> macro
> 
> This sounds promising!
> 
> 
> >     Here is the list of what a ASSERT macro typically do:
> >     a) capture filename and line where the macro was expanded for easy
> 
> > localisation of failure
> >     b) throw an Exception if failure occurs. The Exception convey a
> message 
> > describing the failure, and the location of the failure captured in
> (a).
> > 
> >     That means CppUnit should help the user with both step. At the
> current 
> > time, it means relying on TestAssert::assertImplemention and a lot of
> passing 
> > filename and line around as parameter.
> > 
> >     First I apply IntroduceParameterObject and create a SourceLine
> class that 
> > contains the filename and the line, and a nice default constructor
> when those 
> > are not available. Refactor all CppUnit to use SourceLine (Exception,
> 
> > TestAssert)
> 
> I like this idea.

  So do I! The thing hit me when I was writting the assertion for XML equality. 
I wondered about capturing the location, and finally decided against it because 
that would lead to a lot of code duplication. And I just though that just about 
everyone had the same problem, and probably wrote a lot of code that qualify 
as "duplication"...

> >     Introduce helper methods in TestAssert to factor out code that
> throw 
> > exception for failure.
> 
> OK, but anything that is called an "assertion" should involve testing
> a boolean value.  A function that unconditionally throws an exception
> is not an assertion of anything, it is merely throwing an exception;
> such a function should not live in TestAssert.  It could live in the
> Exception class, perhaps.

  Like you pointed out, TestAssert is probably not a good name. Those helper 
method help to factor out the could produced by when writing asssertion. That 
is:
  - building the failure message
  - constructing the exception object to carry the message and failure location
  - throwing the exception object
  - (optionaly) test an expression to conditionnaly throw the exception object

  Why make the condition check optional ? Because otherwise, you must build the 
string even if not used. Computing those string can be costly, especially if 
you do more than expected/was, by adding another statement pin pointing where 
the difference is. Computing that difference can take some time.

  That helper would be some kind of facade. You don't need to bother yourself 
with knowing how the exception classes works to use them (and you don't need to 
include them). Manipulating CppUnit Exception is only for advanced usage 
(subclassing exception to carry additional info (screen capture, timing...)).

> >     (That last one is not well refined yet, but it would be something
> like:
> > 
> >     void fail( std::string message, 
> >                SourceLine location = SourceLine() );
> 
> This sounds like "throw exception unconditionally" ?
  Yes. Would usually be used after computing a message when you already are in 
the "fail" branch.

> 
> 
> >     void failIf( bool shouldFail, 
> >                  std::string message, 
> >                  SourceLine location = SourceLine() );
> 
> This sounds like the replacement for the current
> "assertImplementation"?

  Yes.

> If so, why not call it simply "assert"?

  The "C" assert macro from hell is still around...

> >     void failNotEqual( std::string expected, 
> >                        std::string actual, 
> >                        SourceLine location = SourceLine(),
> >                        std::string additionalMessage ="" );
> >
> >     void failNotEqual( bool shouldFail,
> >                        std::string expected, 
> >                        std::string actual, 
> >                        SourceLine location = SourceLine(),
> >                        std::string additionalMessage ="" );
> 
> I don't get these two.  It seems like replacements for the current
> "assertEquals", except that you've simply moved the equality testing
> out of the assertion_traits and made it a parameter (of the second
> function).

  Somewhat. Those build a NotEqualException (which store both the 
expected/actual value), and throw it if needed. The user don't have to bother 
himself knowing that a NotEqualException is thrown, only that an exception will 
be thrown.

> If that is the aim, then what is being provided to the library
> user?  It is construction of the exception message.  So, how about
> thinking up common message constructors that one can use with the
> standard TestAssert::assert():

  Noooo!!! Like I pointed out, it constructs and throw the RIGHT exception 
type. For example, TextTestResult detect that it is an NotEqualException, and 
outputs the actual and expected value on two distinct lines, and aligned for 
easy comparison. Advanced TestRunner could even make a diff between the two 
values and point out where the difference is! (Could have really used that 
feature for XML equality. That's how I ended up adding the extra statement on 
failure that pointed out the difference).

[...]
> I really like the idea of providing small building blocks for writing
> custom assertions.  I think the building blocks should be very small
> and very generic.  Having five nearly-the-same-but-slightly-different
> methods leads to great confusion, at least in my mind.  I think we
> should consider hard what building blocks the library should provide.
> 
> I also wonder whether "TestAssert" is the best namespace for it.

  Probably not. How about AssertionHelper or FailureReporter or Asserter or ? 
The main responsability of those functions being to construct and throw the 
exception. The condition check is only provided because to factor the "if" out 
of the user code.

> -Steve
> 
> P.S.  Also, think about backwards compatibility.  I know we are still
> "pre-alpha" so I don't mind breaking it, but if it doesn't cost too
> much, I'm in favour of being compatible.  If nothing else, make notes
> about what is getting broken.

  Well, most of the interface to assertion is through macros. Their interface 
won't change. Current methods could forward toward the new one (at least for 
one or two releases).

  For now, I'll put the new code in Asserter. We'll rename when we got the 
right name.

  I wrote down the current list of compatiblity break in NEWS. Let me know if 
it need to be more detailed. I don't expect compatibility break for this, just 
some "deprecated" methods.

  Also, another important point I noticed when refactoring the TestResult and 
TextTestResult. I've splitted the output into a lot of small methods (some can 
still be split), is that it incredibly enhance the reuse. Make all those little 
print methods virtual and it becomes very easy to customize your output.

  Baptiste.

---
Baptiste Lepilleur <gai...@fr...>
http://gaiacrtn.free.fr/index.html
Language: English, French